-
Notifications
You must be signed in to change notification settings - Fork 2k
compute: resolve permadiff for display_name
in google_compute_organization_security_policy
#15325
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
base: main
Are you sure you want to change the base?
compute: resolve permadiff for display_name
in google_compute_organization_security_policy
#15325
Conversation
Hello! I am a robot. Tests will require approval from a repository maintainer to run. Googlers: For automatic test runs see go/terraform-auto-test-runs. @ScottSuarez, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could I ask why api is returning nil for this field? If the api does not return this field in general we could mark it as 'input' on the field config.
I assume you mean marking as output? However, it is an argument as well and not only an attribute so I am not sure if that makes sense? When looking at the API Docs it seems that the field actually recently got deprecated. Does it make sense to set a |
@ScottSuarez This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check, good to know. I always thought |
73cba12
to
ad11a88
Compare
This reverts commit ad11a88.
Reverted back to a Can you describe why |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted back to a
diff_suppress_func
instead ofignore_read
in 4e739ad after testing. Withignore_read
it only works for newly created resources, while adiff_suppress_func
also solves the issue in case there is already a pre-existing resource in terraform state.Can you describe why
ignore_read
would be preferred here? I have the feelingdiff_suppress_func
is actually the right solution here.
With ignore_read
it only works for newly created resources. Could you expand on this. I am not sure why this would be the case. We are effectively no longer reading the value from the API with ignore_read. What scenario did you run through where this didn't work so I can replicate locally
I can give an update tomorrow with the exact scenario, but it was an initial apply via the latest version of the provider and then a second apply with a build where UPDATE: With ad11a88 as build: Config used: resource "google_folder" "folder_24412" {
provider = google-beta
display_name = "folder-24412"
parent = "organizations/<masked>"
deletion_protection = false
}
resource "google_compute_organization_security_policy" "c_org_sec_policy_24412" {
provider = google-beta
description = "something"
parent = google_folder.folder_24412.name
type = "CLOUD_ARMOR"
display_name = "c-org-sec-policy-24412"
short_name = "c-org-sec-policy-24412"
}
➜ terraform apply -auto-approve
Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
+ create
Terraform will perform the following actions:
# google_compute_organization_security_policy.c_org_sec_policy_24412 will be created
+ resource "google_compute_organization_security_policy" "c_org_sec_policy_24412" {
+ description = "something"
+ display_name = "c-org-sec-policy-24412"
+ fingerprint = (known after apply)
+ id = (known after apply)
+ parent = (known after apply)
+ policy_id = (known after apply)
+ short_name = "c-org-sec-policy-24412"
+ type = "CLOUD_ARMOR"
}
# google_folder.folder_24412 will be created
+ resource "google_folder" "folder_24412" {
+ configured_capabilities = (known after apply)
+ create_time = (known after apply)
+ deletion_protection = false
+ display_name = "folder-24412"
+ folder_id = (known after apply)
+ id = (known after apply)
+ lifecycle_state = (known after apply)
+ management_project = (known after apply)
+ name = (known after apply)
+ parent = "organizations/<masked>"
}
Plan: 2 to add, 0 to change, 0 to destroy.
google_folder.folder_24412: Creating...
google_folder.folder_24412: Still creating... [00m10s elapsed]
google_folder.folder_24412: Creation complete after 14s [id=folders/641458936339]
google_compute_organization_security_policy.c_org_sec_policy_24412: Creating...
google_compute_organization_security_policy.c_org_sec_policy_24412: Still creating... [00m10s elapsed]
google_compute_organization_security_policy.c_org_sec_policy_24412: Still creating... [00m20s elapsed]
google_compute_organization_security_policy.c_org_sec_policy_24412: Creation complete after 24s [id=locations/global/securityPolicies/516529540653]
Apply complete! Resources: 2 added, 0 changed, 0 destroyed.
➜ TF_CLI_CONFIG_FILE="$(pwd)/tf-dev-override.tfrc" terraform apply -auto-approve
╷
│ Warning: Provider development overrides are in effect
│
│ The following provider development overrides are set in the CLI configuration:
│ - hashicorp/google in /Users/ramon/go/bin
│ - hashicorp/google-beta in /Users/ramon/go/bin
│
│ The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with published releases.
╵
google_folder.folder_24412: Refreshing state... [id=folders/641458936339]
google_compute_organization_security_policy.c_org_sec_policy_24412: Refreshing state... [id=locations/global/securityPolicies/516529540653]
Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement
Terraform will perform the following actions:
# google_compute_organization_security_policy.c_org_sec_policy_24412 must be replaced
-/+ resource "google_compute_organization_security_policy" "c_org_sec_policy_24412" {
+ display_name = "c-org-sec-policy-24412" # forces replacement
~ fingerprint = "p7-LpFHUuCk=" -> (known after apply)
~ id = "locations/global/securityPolicies/516529540653" -> (known after apply)
~ policy_id = "516529540653" -> (known after apply)
# (4 unchanged attributes hidden)
}
Plan: 1 to add, 0 to change, 1 to destroy.
google_compute_organization_security_policy.c_org_sec_policy_24412: Destroying... [id=locations/global/securityPolicies/516529540653]
google_compute_organization_security_policy.c_org_sec_policy_24412: Still destroying... [id=locations/global/securityPolicies/516529540653, 00m10s elapsed]
google_compute_organization_security_policy.c_org_sec_policy_24412: Destruction complete after 12s
google_compute_organization_security_policy.c_org_sec_policy_24412: Creating...
google_compute_organization_security_policy.c_org_sec_policy_24412: Still creating... [00m10s elapsed]
google_compute_organization_security_policy.c_org_sec_policy_24412: Still creating... [00m20s elapsed]
google_compute_organization_security_policy.c_org_sec_policy_24412: Creation complete after 23s [id=locations/global/securityPolicies/716440605623]
Apply complete! Resources: 1 added, 0 changed, 1 destroyed. With ad11a88 as build:
➜ terraform apply -auto-approve
Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
+ create
Terraform will perform the following actions:
# google_compute_organization_security_policy.c_org_sec_policy_24412 will be created
+ resource "google_compute_organization_security_policy" "c_org_sec_policy_24412" {
+ description = "something"
+ display_name = "c-org-sec-policy-24412"
+ fingerprint = (known after apply)
+ id = (known after apply)
+ parent = (known after apply)
+ policy_id = (known after apply)
+ short_name = "c-org-sec-policy-24412"
+ type = "CLOUD_ARMOR"
}
# google_folder.folder_24412 will be created
+ resource "google_folder" "folder_24412" {
+ configured_capabilities = (known after apply)
+ create_time = (known after apply)
+ deletion_protection = false
+ display_name = "folder-24412"
+ folder_id = (known after apply)
+ id = (known after apply)
+ lifecycle_state = (known after apply)
+ management_project = (known after apply)
+ name = (known after apply)
+ parent = "organizations/<masked>"
}
Plan: 2 to add, 0 to change, 0 to destroy.
google_folder.folder_24412: Creating...
google_folder.folder_24412: Still creating... [00m10s elapsed]
google_folder.folder_24412: Creation complete after 15s [id=folders/139662854581]
google_compute_organization_security_policy.c_org_sec_policy_24412: Creating...
google_compute_organization_security_policy.c_org_sec_policy_24412: Still creating... [00m10s elapsed]
google_compute_organization_security_policy.c_org_sec_policy_24412: Still creating... [00m20s elapsed]
google_compute_organization_security_policy.c_org_sec_policy_24412: Still creating... [00m30s elapsed]
google_compute_organization_security_policy.c_org_sec_policy_24412: Still creating... [00m40s elapsed]
google_compute_organization_security_policy.c_org_sec_policy_24412: Creation complete after 40s [id=locations/global/securityPolicies/754570047705]
Apply complete! Resources: 2 added, 0 changed, 0 destroyed.
➜ TF_CLI_CONFIG_FILE="$(pwd)/tf-dev-override.tfrc" terraform apply -auto-approve
╷
│ Warning: Provider development overrides are in effect
│
│ The following provider development overrides are set in the CLI configuration:
│ - hashicorp/google in /Users/ramon/go/bin
│ - hashicorp/google-beta in /Users/ramon/go/bin
│
│ The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with published releases.
╵
google_folder.folder_24412: Refreshing state... [id=folders/139662854581]
google_compute_organization_security_policy.c_org_sec_policy_24412: Refreshing state... [id=locations/global/securityPolicies/754570047705]
No changes. Your infrastructure matches the configuration.
Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.
Apply complete! Resources: 0 added, 0 changed, 0 destroyed.
|
@ScottSuarez This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I see what is happening, in the first cycle "" is being written to state for the display_name. Which then breaks the ignore_read.
I think it's fine to leave the ignore_read as it will properly store in state the correct value. We have a perma_diff today so introducing ignore_read won't introduce a new perma_diff, but will resolve forward looking.
Closes hashicorp/terraform-provider-google#24412
Was able to reproduce the permadiff via the config provided in the issue. Added a general function because I think this logic can be reused, in fact I already re-use the same logic in my other PR #15254.
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.
Manual tests (no diff with new build, while having a reproduced diff with latest provider version):