Add guide on deleted principals #3950

slevenick · 2020-09-03T22:31:31Z

Adds a guide on handling diffs based on deleted: IAM principals for the upcoming change

If this PR is for Terraform, I acknowledge that I have:

Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

modular-magician · 2020-09-03T22:38:55Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 50 insertions(+))
Terraform Beta: Diff ( 1 file changed, 50 insertions(+))

modular-magician · 2020-09-03T22:39:05Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=144421"

danawillow

You'll also want to add a link to this in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/third_party/terraform/website/google.erb.tmpl#L25

third_party/terraform/website/docs/guides/iam_deleted_members.html.markdown

danawillow · 2020-09-09T22:08:08Z

third_party/terraform/website/docs/guides/iam_deleted_members.html.markdown

+
+During the intermediate period it may be required to `taint` the `_iam_policy` resource to ensure any deleted principals are removed *before* the new principal is granted permission. This should only be necessary if you are continuing to see diffs after successful applies. For more information on using `taint` see the [official documentation](https://www.terraform.io/docs/commands/taint.html).
+
+**Note:** Tainting an `iam_policy` resource will delete and recreate it. If the account that Terraform uses to provision GCP resources requires permissions granted by the `iam_policy` resource it may result in Terraform being unable to complete the apply. This is possible for `google_project_iam_policy` and `google_organization_iam_policy`. Special care should be taken when tainting these resources.


Another way to do it would be to make an arbitrary change to the policy, like adding a random member and then just removing them right away after. This would avoid the need for a destroy/create, right?

No, the issue is that a diff shows up to remove the deleted form of a member and add the non-deleted form of that member. When Terraform attempts to apply that diff during the intermediate period, the API assumes that the non-deleted forms are references to the deleted forms and does not change them.

To successfully apply there must be the intermediate step of deleting all references to the deleted member, then the references to the non-deleted member can be added

Oh wow, that's tricky! Thanks for the explanation.

third_party/terraform/website/docs/guides/iam_deleted_members.html.markdown

danawillow · 2020-09-09T22:15:19Z

third_party/terraform/website/docs/guides/iam_deleted_members.html.markdown

+
+`_iam_binding` resources handle all the members who are granted a specific role for an IAM policy. These resources may see diffs if a member they grant a role to is deleted and recreated. Due to these resources not controlling the entire IAM policy you may see issues around diffs not being resolved as requests can include both the deleted and non-deleted form of a principal. 
+
+Tainting the `_iam_binding` resource using the [taint command](https://www.terraform.io/docs/commands/taint.html) may resolve this diff. If it does not, or results in an API error, you may need to manually remove any references to the deleted version of the principal causing the diff. This can be done through the Cloud Console UI for the resource. This could also be done via Terraform by specifying the entire IAM policy authoritatively using the `_iam_policy` resource.


same comment wrt making an arbitrary change

modular-magician · 2020-09-10T21:37:35Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 71 insertions(+))
Terraform Beta: Diff ( 3 files changed, 71 insertions(+))

modular-magician · 2020-09-10T21:37:42Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=145643"

modular-magician · 2020-09-10T22:05:36Z

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccComposerEnvironment_update|TestAccComposerEnvironment_withDatabaseConfig|TestAccComposerEnvironment_withWebServerConfig|TestAccComputeSnapshot_encryptionCMEK|TestAccComputeUrlMap_trafficDirectorRemoveRouteRule|TestAccComputeUrlMap_trafficDirectorPathUpdate|TestAccComputeUrlMap_defaultUrlRedirect|TestAccDataCatalogTagTemplate_dataCatalogTagTemplateBasicExample|TestAccDataCatalogTag_dataCatalogEntryTagBasicExample|TestAccDataCatalogTag_dataCatalogEntryGroupTagExample|TestAccDataCatalogTag_dataCatalogEntryTagFalseExample|TestAccDataCatalogTag_update|TestAccDataLossPreventionInspectTemplate_dlpInspectTemplateUpdate|TestAccDataLossPreventionInspectTemplate_dlpInspectTemplateBasicExample|TestAccDialogflowIntent_dialogflowIntentFullExample|TestAccDialogflowIntent_update|TestAccDialogflowEntityType_update|TestAccHealthcareFhirStore_healthcareFhirStoreBasicExample|TestAccHealthcareFhirStore_healthcareFhirStoreStreamingConfigExample|TestAccHealthcareFhirStore_basic|TestAccHealthcareHl7V2Store_healthcareHl7V2StoreParserConfigExample|TestAccIdentityPlatformInboundSamlConfig_inboundSamlConfigUpdate|TestAccIdentityPlatformDefaultSupportedIdpConfig_defaultSupportedIdpConfigUpdate|TestAccIdentityPlatformOauthIdpConfig_identityPlatformOauthIdpConfigUpdate|TestAccIdentityPlatformTenantDefaultSupportedIdpConfig_identityPlatformTenantDefaultSupportedIdpConfigUpdate|TestAccIdentityPlatformTenantInboundSamlConfig_identityPlatformTenantInboundSamlConfigUpdate|TestAccIdentityPlatformTenant_identityPlatformTenantUpdate|TestAccIdentityPlatformTenantOauthIdpConfig_identityPlatformTenantOauthIdpConfigUpdate|TestAccMonitoringGroup_update|TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpExample|TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpsExample|TestAccMonitoringUptimeCheckConfig_update|TestAccMonitoringUptimeCheckConfig_changeNonUpdatableFields|TestAccOSConfigPatchDeployment_osConfigPatchDeploymentFullExample|TestAccOSConfigGuestPolicies_osConfigGuestPoliciesRecipesExample|TestAccOSConfigPatchDeployment_osConfigPatchDeploymentInstanceExample|TestAccPubsubSubscription_pubsubSubscriptionPullExample|TestAccPubsubSubscription_emptyTTL|TestAccPubsubSubscription_update You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=145646"

modular-magician · 2020-09-10T22:29:59Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 3 files changed, 71 insertions(+))
Terraform Beta: Diff ( 3 files changed, 71 insertions(+))

modular-magician · 2020-09-10T22:30:08Z

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=145647"

modular-magician · 2020-09-10T22:57:32Z

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccComposerEnvironment_update|TestAccComposerEnvironment_withDatabaseConfig|TestAccComposerEnvironment_withWebServerConfig|TestAccComputeSnapshot_encryptionCMEK|TestAccComputeUrlMap_trafficDirectorPathUpdate|TestAccComputeUrlMap_trafficDirectorRemoveRouteRule|TestAccComputeUrlMap_defaultUrlRedirect|TestAccDataCatalogTagTemplate_dataCatalogTagTemplateBasicExample|TestAccDataCatalogTag_dataCatalogEntryGroupTagExample|TestAccDataCatalogTag_dataCatalogEntryTagBasicExample|TestAccDataCatalogTag_dataCatalogEntryTagFalseExample|TestAccDataCatalogTag_update|TestAccDataLossPreventionInspectTemplate_dlpInspectTemplateBasicExample|TestAccDataLossPreventionInspectTemplate_dlpInspectTemplateUpdate|TestAccDialogflowIntent_dialogflowIntentFullExample|TestAccDialogflowIntent_update|TestAccDialogflowEntityType_update|TestAccHealthcareFhirStore_healthcareFhirStoreBasicExample|TestAccHealthcareFhirStore_healthcareFhirStoreStreamingConfigExample|TestAccHealthcareFhirStore_basic|TestAccHealthcareHl7V2Store_healthcareHl7V2StoreParserConfigExample|TestAccIdentityPlatformInboundSamlConfig_inboundSamlConfigUpdate|TestAccIdentityPlatformDefaultSupportedIdpConfig_defaultSupportedIdpConfigUpdate|TestAccIdentityPlatformOauthIdpConfig_identityPlatformOauthIdpConfigUpdate|TestAccIdentityPlatformTenantDefaultSupportedIdpConfig_identityPlatformTenantDefaultSupportedIdpConfigUpdate|TestAccIdentityPlatformTenantInboundSamlConfig_identityPlatformTenantInboundSamlConfigUpdate|TestAccIdentityPlatformTenant_identityPlatformTenantUpdate|TestAccIdentityPlatformTenantOauthIdpConfig_identityPlatformTenantOauthIdpConfigUpdate|TestAccMonitoringGroup_update|TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpExample|TestAccMonitoringUptimeCheckConfig_uptimeCheckConfigHttpsExample|TestAccMonitoringUptimeCheckConfig_update|TestAccMonitoringUptimeCheckConfig_changeNonUpdatableFields|TestAccOSConfigPatchDeployment_osConfigPatchDeploymentFullExample|TestAccOSConfigGuestPolicies_osConfigGuestPoliciesRecipesExample|TestAccOSConfigPatchDeployment_osConfigPatchDeploymentInstanceExample|TestAccPubsubSubscription_pubsubSubscriptionPullExample|TestAccPubsubSubscription_emptyTTL|TestAccPubsubSubscription_update You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=145650"

google-cla bot added the cla: yes label Sep 3, 2020

slevenick requested a review from danawillow September 9, 2020 16:47

danawillow reviewed Sep 9, 2020

View reviewed changes

slevenick added 2 commits September 10, 2020 15:22

Add guide on deleted principals

909a58b

PR feedback, website link

264ed2f

slevenick force-pushed the iam-deleted-guide branch from b526530 to 264ed2f Compare September 10, 2020 22:22

slevenick requested a review from danawillow September 10, 2020 23:07

danawillow approved these changes Sep 11, 2020

View reviewed changes

slevenick merged commit 8cc3c36 into GoogleCloudPlatform:master Sep 11, 2020

This was referenced Sep 11, 2020

Add guide on deleted principals hashicorp/terraform-provider-google#7251

Merged

Add guide on deleted principals hashicorp/terraform-provider-google-beta#2475

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add guide on deleted principals #3950

Add guide on deleted principals #3950

slevenick commented Sep 3, 2020

modular-magician commented Sep 3, 2020

modular-magician commented Sep 3, 2020

danawillow left a comment

danawillow Sep 9, 2020

slevenick Sep 10, 2020

danawillow Sep 11, 2020

danawillow Sep 9, 2020

modular-magician commented Sep 10, 2020

modular-magician commented Sep 10, 2020

modular-magician commented Sep 10, 2020

modular-magician commented Sep 10, 2020

modular-magician commented Sep 10, 2020

modular-magician commented Sep 10, 2020


		During the intermediate period it may be required to `taint` the `_iam_policy` resource to ensure any deleted principals are removed before the new principal is granted permission. This should only be necessary if you are continuing to see diffs after successful applies. For more information on using `taint` see the [official documentation](https://www.terraform.io/docs/commands/taint.html).

		Note: Tainting an `iam_policy` resource will delete and recreate it. If the account that Terraform uses to provision GCP resources requires permissions granted by the `iam_policy` resource it may result in Terraform being unable to complete the apply. This is possible for `google_project_iam_policy` and `google_organization_iam_policy`. Special care should be taken when tainting these resources.


		`_iam_binding` resources handle all the members who are granted a specific role for an IAM policy. These resources may see diffs if a member they grant a role to is deleted and recreated. Due to these resources not controlling the entire IAM policy you may see issues around diffs not being resolved as requests can include both the deleted and non-deleted form of a principal.

		Tainting the `_iam_binding` resource using the [taint command](https://www.terraform.io/docs/commands/taint.html) may resolve this diff. If it does not, or results in an API error, you may need to manually remove any references to the deleted version of the principal causing the diff. This can be done through the Cloud Console UI for the resource. This could also be done via Terraform by specifying the entire IAM policy authoritatively using the `_iam_policy` resource.

Add guide on deleted principals #3950

Add guide on deleted principals #3950

Conversation

slevenick commented Sep 3, 2020

modular-magician commented Sep 3, 2020

Diff report:

modular-magician commented Sep 3, 2020

danawillow left a comment

Choose a reason for hiding this comment

danawillow Sep 9, 2020

Choose a reason for hiding this comment

slevenick Sep 10, 2020

Choose a reason for hiding this comment

danawillow Sep 11, 2020

Choose a reason for hiding this comment

danawillow Sep 9, 2020

Choose a reason for hiding this comment

modular-magician commented Sep 10, 2020

Diff report:

modular-magician commented Sep 10, 2020

modular-magician commented Sep 10, 2020

modular-magician commented Sep 10, 2020

Diff report:

modular-magician commented Sep 10, 2020

modular-magician commented Sep 10, 2020