Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates container_cluster to set enable_legacy_abac to false by default #1281

Merged

Conversation

mrparkers
Copy link
Contributor

Discussion in #1116.

GKE disables legacy ABAC by default in 1.8, and the current default GKE version (as of opening this PR) is 1.8.8. This means that creating a GKE cluster using the cloud console and leaving all values as default will result legacy ABAC being disabled, but creating a GKE cluster using Terraform and leaving all values as default will keep it enabled. This PR disables legacy ABAC by default so the default behavior is more consistent with the cloud console.

Test:

TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccContainerCluster_withDefaultLegacyAbac -timeout 120m
?   	github.com/terraform-providers/terraform-provider-google	[no test files]
=== RUN   TestAccContainerCluster_withDefaultLegacyAbac
--- PASS: TestAccContainerCluster_withDefaultLegacyAbac (243.16s)

@rosbo rosbo requested a review from danawillow April 2, 2018 18:28
Copy link
Contributor

@danawillow danawillow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @mrparkers! Just one quick suggestion.

Config: testAccContainerCluster_defaultLegacyAbac(acctest.RandString(10)),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("google_container_cluster.default_legacy_abac", "enable_legacy_abac", "false"),
),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind adding an import step here as well? (Take a look at the other tests in this file for examples)

@mrparkers
Copy link
Contributor Author

@danawillow I addressed your comment.

TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccContainerCluster_withDefaultLegacyAbac -timeout 120m
?   	github.com/terraform-providers/terraform-provider-google	[no test files]
=== RUN   TestAccContainerCluster_withDefaultLegacyAbac
--- PASS: TestAccContainerCluster_withDefaultLegacyAbac (233.43s)
PASS
ok  	github.com/terraform-providers/terraform-provider-google/google	233.458s

@danawillow
Copy link
Contributor

Thanks a bunch!

@danawillow danawillow merged commit b8adcc2 into hashicorp:master Apr 4, 2018
@mrparkers mrparkers deleted the gke-update-default-legacy-abac branch April 4, 2018 18:23
chrisst pushed a commit to chrisst/terraform-provider-google that referenced this pull request Nov 9, 2018
…lt (hashicorp#1281)

* Updates the default GKE legacy ABAC setting to false

* Updates docs for container_cluster

* Update test comments

* Format fix

* Adds ImportState test step to default legacy ABAC test
@ghost
Copy link

ghost commented Nov 19, 2018

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants