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

Make sure TypeList keys still have empty values #1685

Merged

Conversation

emilymye
Copy link
Contributor

  • If key is not set in ResourceData, diff will not trigger
  • Moved nil checks into flatten methods since this matches generated resources and also cleans Read() a bit
  • Moved masterAuth logic into a flatten method

Fixes #1675 and also possibly other things

@emilymye emilymye requested review from nat-henderson, paddycarver and danawillow and removed request for paddycarver June 20, 2018 23:48
Copy link
Contributor

@nat-henderson nat-henderson left a comment

Choose a reason for hiding this comment

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

That definitely fixes other things! Did you run the tests? If they all still pass this looks great.


d.Set("maintenance_policy", flattenMaintenancePolicy(cluster.MaintenancePolicy))
d.Set("master_auth", flattenMasterAuth(cluster.MasterAuth))
d.Set("master_authorized_networks_config", flattenMasterAuthorizedNetworksConfig(cluster.MasterAuthorizedNetworksConfig))
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we check the error on these? Complex types with d.Set can sometimes get tricky. :/

@emilymye
Copy link
Contributor Author

I ran most of the tests (in batches because cluster limits and I haven't changed that quota yet)

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v --run="TestAccContainerCluster_withM" -timeout 120m
=== RUN TestAccContainerCluster_withMasterAuthConfig
=== PAUSE TestAccContainerCluster_withMasterAuthConfig
=== RUN TestAccContainerCluster_withMasterAuthorizedNetworksConfig
=== PAUSE TestAccContainerCluster_withMasterAuthorizedNetworksConfig
=== RUN TestAccContainerCluster_withMaintenanceWindow
=== PAUSE TestAccContainerCluster_withMaintenanceWindow
=== CONT TestAccContainerCluster_withMasterAuthConfig
=== CONT TestAccContainerCluster_withMaintenanceWindow
=== CONT TestAccContainerCluster_withMasterAuthorizedNetworksConfig
--- PASS: TestAccContainerCluster_withMasterAuthConfig (306.09s)
--- PASS: TestAccContainerCluster_withMaintenanceWindow (309.49s)
--- PASS: TestAccContainerCluster_withMasterAuthorizedNetworksConfig (313.31s)
PASS
ok github.com/terraform-providers/terraform-provider-google/google313.498s
TF_ACC=1 go test ./google -v --run="TestAccContainerCluster_basic" -timeout 120m
=== RUN TestAccContainerCluster_basic
=== PAUSE TestAccContainerCluster_basic
=== CONT TestAccContainerCluster_basic
--- PASS: TestAccContainerCluster_basic (276.99s)
PASS
ok github.com/terraform-providers/terraform-provider-google/google277.155s

@paddycarver paddycarver merged commit 9bd15ad into hashicorp:master Jun 22, 2018
@ghost
Copy link

ghost commented Nov 17, 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 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 17, 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.

4 participants