Add deletion_protection field to Folders resource

[abd-goog]

Add deletion_protection field to make deletion actions require an explicit intent
Part of b/330143705

Fixes hashicorp/terraform-provider-google#18904

Release Note Template for Downstream PRs (will be copied)

google_folders: added `deletion_protection` field to `folders` to make deleting them require an explicit intent. `folder` resources now cannot be destroyed unless `deletion_protection = false` is set for the resource.

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 40 files changed, 126 insertions(+), 5 deletions(-))
google-beta provider: Diff ( 44 files changed, 131 insertions(+), 5 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 3 insertions(+))

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 40 files changed, 126 insertions(+), 5 deletions(-))
google-beta provider: Diff ( 44 files changed, 131 insertions(+), 5 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 3 insertions(+))

[github-actions]

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@melinath, 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.

[modular-magician]

Tests analytics

Total tests: 1271
Passed tests: 1159
Skipped tests: 111
Affected tests: 1

Click here to see the affected service packages

• cloudasset
• iam2
• kms
• logging
• orgpolicy
• resourcemanager
• accessapproval
• assuredworkloads
• securitycentermanagement
• compute
• securitycenter

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccLoggingFolderSettings_datasource

Get to know how VCR tests work

[modular-magician]

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccLoggingFolderSettings_datasource[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

[modular-magician]

Tests analytics

Total tests: 1271
Passed tests: 1159
Skipped tests: 111
Affected tests: 1

Click here to see the affected service packages

• iam2
• kms
• orgpolicy
• resourcemanager
• securitycenter
• securitycentermanagement
• accessapproval
• assuredworkloads
• logging
• cloudasset
• compute

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccLoggingFolderSettings_datasource

Get to know how VCR tests work

[modular-magician]

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccLoggingFolderSettings_datasource[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 41 files changed, 127 insertions(+), 5 deletions(-))
google-beta provider: Diff ( 45 files changed, 132 insertions(+), 5 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 3 insertions(+))

[modular-magician]

Tests analytics

Total tests: 1271
Passed tests: 1092
Skipped tests: 111
Affected tests: 68

Click here to see the affected service packages

• iam2
• logging
• securitycentermanagement
• compute
• assuredworkloads
• cloudasset
• kms
• orgpolicy
• resourcemanager
• securitycenter
• accessapproval

Action taken

Found 68 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccComputeBackendService_trafficDirectorUpdateBasic
• TestAccComputeBackendService_trafficDirectorUpdateFull
• TestAccComputeBackendService_trafficDirectorUpdateLbPolicies
• TestAccComputeForwardingRule_forwardingRuleExternallbExample
• TestAccComputeForwardingRule_forwardingRuleGlobalInternallbExample
• TestAccComputeForwardingRule_forwardingRuleHttpLbExample
• TestAccComputeForwardingRule_forwardingRuleInternallbExample
• TestAccComputeForwardingRule_forwardingRuleInternallbIpv6Example
• TestAccComputeForwardingRule_forwardingRuleIpAddressIpv6
• TestAccComputeForwardingRule_forwardingRuleL3DefaultExample
• TestAccComputeForwardingRule_forwardingRulePscRecreate
• TestAccComputeForwardingRule_forwardingRuleRegionalHttpXlbExample
• TestAccComputeForwardingRule_forwardingRuleRegionalSteeringExample
• TestAccComputeForwardingRule_forwardingRuleRegionalSteeringExampleUpdate
• TestAccComputeForwardingRule_forwardingRuleVpcPscExample
• TestAccComputeForwardingRule_forwardingRuleVpcPscExampleUpdate
• TestAccComputeForwardingRule_forwardingRuleVpcPscNoAutomateDnsExample
• TestAccComputeForwardingRule_internalHttpLbWithMigBackendExample
• TestAccComputeForwardingRule_internalTcpUdpLbWithMigBackendExample
• TestAccComputeForwardingRule_internalTcpUdpLbWithMigBackendExampleUpdate
• TestAccComputePacketMirroring_computePacketMirroringFullExample
• TestAccComputeRegionBackendService_basic
• TestAccComputeRegionBackendService_ilbBasic_withUnspecifiedProtocol
• TestAccComputeRegionBackendService_ilbUpdateBasic
• TestAccComputeRegionBackendService_ilbUpdateFull
• TestAccComputeRegionBackendService_regionBackendServiceBalancingModeExample
• TestAccComputeRegionBackendService_regionBackendServiceCacheExample
• TestAccComputeRegionBackendService_regionBackendServiceExternalExample
• TestAccComputeRegionBackendService_regionBackendServiceExternalWeightedExample
• TestAccComputeRegionBackendService_regionBackendServiceIlbRingHashExample
• TestAccComputeRegionBackendService_regionBackendServiceIlbRoundRobinExample
• TestAccComputeRegionBackendService_subsettingUpdate
• TestAccComputeRegionBackendService_withBackendAndIAP
• TestAccComputeRegionBackendService_withBackendInternal
• TestAccComputeRegionBackendService_withBackendInternalManaged
• TestAccComputeRegionBackendService_withBackendMultiNic
• TestAccComputeRegionBackendService_withConnectionDrainingAndUpdate
• TestAccComputeRegionBackendService_withSecurityPolicy
• TestAccComputeRegionNetworkEndpointGroup_regionNetworkEndpointGroupPscServiceAttachmentExample
• TestAccComputeRegionTargetHttpProxy_regionTargetHttpProxyBasicExample
• TestAccComputeRegionTargetHttpProxy_update
• TestAccComputeRegionTargetHttpsProxy_addSslPolicy_withForwardingRule
• TestAccComputeRegionTargetHttpsProxy_regionTargetHttpsProxyBasicExample
• TestAccComputeRegionTargetHttpsProxy_regionTargetHttpsProxyCertificateManagerCertificateExample
• TestAccComputeRegionTargetHttpsProxy_regionTargetHttpsProxyMtlsExample
• TestAccComputeRegionTargetHttpsProxy_update
• TestAccComputeRegionTargetTcpProxy_regionTargetTcpProxyBasicExample
• TestAccComputeRegionTargetTcpProxy_update
• TestAccComputeRegionUrlMap_advanced
• TestAccComputeRegionUrlMap_defaultRouteAction_full_update
• TestAccComputeRegionUrlMap_ilbPathUpdate
• TestAccComputeRegionUrlMap_ilbRouteUpdate
• TestAccComputeRegionUrlMap_noPathRulesWithUpdate
• TestAccComputeRegionUrlMap_regionUrlMapBasicExample
• TestAccComputeRegionUrlMap_regionUrlMapDefaultRouteActionExample
• TestAccComputeRegionUrlMap_regionUrlMapL7IlbPathExample
• TestAccComputeRegionUrlMap_regionUrlMapL7IlbPathPartialExample
• TestAccComputeRegionUrlMap_regionUrlMapL7IlbRouteExample
• TestAccComputeRegionUrlMap_regionUrlMapL7IlbRoutePartialExample
• TestAccComputeRegionUrlMap_regionUrlMapPathTemplateMatchExample
• TestAccComputeRegionUrlMap_update_path_matcher
• TestAccComputeRoute_routeIlbExample
• TestAccComputeRoute_routeIlbVipExample
• TestAccComputeServiceAttachment_serviceAttachmentBasicExample
• TestAccComputeServiceAttachment_serviceAttachmentBasicExampleUpdate
• TestAccComputeServiceAttachment_serviceAttachmentExplicitNetworksExample
• TestAccComputeServiceAttachment_serviceAttachmentExplicitProjectsExample
• TestAccComputeServiceAttachment_serviceAttachmentReconcileConnectionsExample

Get to know how VCR tests work

[melinath]

Please make sure that deletion_protection gets set in the import function similar to https://github.com/hashicorp/terraform-provider-google-beta/blob/a4bdc631489c4974394ba18123ecf32baa30109a/google-beta/services/container/resource_container_cluster.go#L6488-L6490

[modular-magician]

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccComputeBackendService_trafficDirectorUpdateBasic[Debug log]
TestAccComputeBackendService_trafficDirectorUpdateFull[Debug log]
TestAccComputeBackendService_trafficDirectorUpdateLbPolicies[Debug log]
TestAccComputeForwardingRule_forwardingRuleExternallbExample[Debug log]
TestAccComputeForwardingRule_forwardingRuleGlobalInternallbExample[Debug log]
TestAccComputeForwardingRule_forwardingRuleHttpLbExample[Debug log]
TestAccComputeForwardingRule_forwardingRuleInternallbExample[Debug log]
TestAccComputeForwardingRule_forwardingRuleInternallbIpv6Example[Debug log]
TestAccComputeForwardingRule_forwardingRuleIpAddressIpv6[Debug log]
TestAccComputeForwardingRule_forwardingRuleL3DefaultExample[Debug log]
TestAccComputeForwardingRule_forwardingRulePscRecreate[Debug log]
TestAccComputeForwardingRule_forwardingRuleRegionalHttpXlbExample[Debug log]
TestAccComputeForwardingRule_forwardingRuleRegionalSteeringExample[Debug log]
TestAccComputeForwardingRule_forwardingRuleRegionalSteeringExampleUpdate[Debug log]
TestAccComputeForwardingRule_forwardingRuleVpcPscExample[Debug log]
TestAccComputeForwardingRule_forwardingRuleVpcPscExampleUpdate[Debug log]
TestAccComputeForwardingRule_forwardingRuleVpcPscNoAutomateDnsExample[Debug log]
TestAccComputeForwardingRule_internalHttpLbWithMigBackendExample[Debug log]
TestAccComputeForwardingRule_internalTcpUdpLbWithMigBackendExample[Debug log]
TestAccComputeForwardingRule_internalTcpUdpLbWithMigBackendExampleUpdate[Debug log]
TestAccComputePacketMirroring_computePacketMirroringFullExample[Debug log]
TestAccComputeRegionBackendService_basic[Debug log]
TestAccComputeRegionBackendService_ilbBasic_withUnspecifiedProtocol[Debug log]
TestAccComputeRegionBackendService_ilbUpdateBasic[Debug log]
TestAccComputeRegionBackendService_ilbUpdateFull[Debug log]
TestAccComputeRegionBackendService_regionBackendServiceBalancingModeExample[Debug log]
TestAccComputeRegionBackendService_regionBackendServiceCacheExample[Debug log]
TestAccComputeRegionBackendService_regionBackendServiceExternalExample[Debug log]
TestAccComputeRegionBackendService_regionBackendServiceExternalWeightedExample[Debug log]
TestAccComputeRegionBackendService_regionBackendServiceIlbRingHashExample[Debug log]
TestAccComputeRegionBackendService_regionBackendServiceIlbRoundRobinExample[Debug log]
TestAccComputeRegionBackendService_subsettingUpdate[Debug log]
TestAccComputeRegionBackendService_withBackendAndIAP[Debug log]
TestAccComputeRegionBackendService_withBackendInternal[Debug log]
TestAccComputeRegionBackendService_withBackendInternalManaged[Debug log]
TestAccComputeRegionBackendService_withBackendMultiNic[Debug log]
TestAccComputeRegionBackendService_withConnectionDrainingAndUpdate[Debug log]
TestAccComputeRegionBackendService_withSecurityPolicy[Debug log]
TestAccComputeRegionNetworkEndpointGroup_regionNetworkEndpointGroupPscServiceAttachmentExample[Debug log]
TestAccComputeRegionTargetHttpProxy_regionTargetHttpProxyBasicExample[Debug log]
TestAccComputeRegionTargetHttpProxy_update[Debug log]
TestAccComputeRegionTargetHttpsProxy_addSslPolicy_withForwardingRule[Debug log]
TestAccComputeRegionTargetHttpsProxy_regionTargetHttpsProxyBasicExample[Debug log]
TestAccComputeRegionTargetHttpsProxy_regionTargetHttpsProxyCertificateManagerCertificateExample[Debug log]
TestAccComputeRegionTargetHttpsProxy_regionTargetHttpsProxyMtlsExample[Debug log]
TestAccComputeRegionTargetHttpsProxy_update[Debug log]
TestAccComputeRegionTargetTcpProxy_regionTargetTcpProxyBasicExample[Debug log]
TestAccComputeRegionTargetTcpProxy_update[Debug log]
TestAccComputeRegionUrlMap_advanced[Debug log]
TestAccComputeRegionUrlMap_defaultRouteAction_full_update[Debug log]
TestAccComputeRegionUrlMap_ilbPathUpdate[Debug log]
TestAccComputeRegionUrlMap_ilbRouteUpdate[Debug log]
TestAccComputeRegionUrlMap_noPathRulesWithUpdate[Debug log]
TestAccComputeRegionUrlMap_regionUrlMapBasicExample[Debug log]
TestAccComputeRegionUrlMap_regionUrlMapDefaultRouteActionExample[Debug log]
TestAccComputeRegionUrlMap_regionUrlMapL7IlbPathExample[Debug log]
TestAccComputeRegionUrlMap_regionUrlMapL7IlbPathPartialExample[Debug log]
TestAccComputeRegionUrlMap_regionUrlMapL7IlbRouteExample[Debug log]
TestAccComputeRegionUrlMap_regionUrlMapL7IlbRoutePartialExample[Debug log]
TestAccComputeRegionUrlMap_regionUrlMapPathTemplateMatchExample[Debug log]
TestAccComputeRegionUrlMap_update_path_matcher[Debug log]
TestAccComputeRoute_routeIlbExample[Debug log]
TestAccComputeRoute_routeIlbVipExample[Debug log]
TestAccComputeServiceAttachment_serviceAttachmentBasicExample[Debug log]
TestAccComputeServiceAttachment_serviceAttachmentBasicExampleUpdate[Debug log]
TestAccComputeServiceAttachment_serviceAttachmentExplicitNetworksExample[Debug log]
TestAccComputeServiceAttachment_serviceAttachmentExplicitProjectsExample[Debug log]
TestAccComputeServiceAttachment_serviceAttachmentReconcileConnectionsExample[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

---

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log or the debug log for each test

[abd-goog]

Please make sure that deletion_protection gets set in the import function similar to https://github.com/hashicorp/terraform-provider-google-beta/blob/a4bdc631489c4974394ba18123ecf32baa30109a/google-beta/services/container/resource_container_cluster.go#L6488-L6490

Added. Thanks!

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 41 files changed, 131 insertions(+), 5 deletions(-))
google-beta provider: Diff ( 45 files changed, 136 insertions(+), 5 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 3 insertions(+))

[modular-magician]

Tests analytics

Total tests: 1271
Passed tests: 1160
Skipped tests: 111
Affected tests: 0

Click here to see the affected service packages

• accessapproval
• compute
• kms
• orgpolicy
• resourcemanager
• securitycentermanagement
• assuredworkloads
• cloudasset
• iam2
• logging
• securitycenter

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log

[melinath]

I ended up checking with some other folks on the team to reconcile some differences between how this was implemented compared to some other deletion_protection fields and have created #11330 to document the best practices.

Basically the changes for you to make (assuming that I got everything down right in that PR) are:

• Set deletion_protection in read (as described in the new docs)
• Add logic to skip API requests if only deletion_protection is being modified (not strictly necessary but good to do)
• Remove setting of deletion protection in import (it seems like it may not be necessary after all if it's getting set in read - sorry to ask you to remove something I just asked you to add!)

I wanted to let you know about these changes in guidance even though they're not fully merged yet so that you can get a head start - but feel free to wait until it's merged if you prefer. (FYI there's a chance that setting during Import actually does need to be there due to some edge case that I wasn't able to figure out, but I think it probably ought to go.)

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 41 files changed, 132 insertions(+), 6 deletions(-))
google-beta provider: Diff ( 45 files changed, 137 insertions(+), 6 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 3 insertions(+))

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 41 files changed, 145 insertions(+), 6 deletions(-))
google-beta provider: Diff ( 45 files changed, 150 insertions(+), 6 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 3 insertions(+))

[modular-magician]

Tests analytics

Total tests: 1137
Passed tests: 1057
Skipped tests: 80
Affected tests: 0

Click here to see the affected service packages

• securitycentermanagement
• accessapproval
• logging
• securitycenter
• iam2
• kms
• orgpolicy
• resourcemanager
• assuredworkloads
• cloudasset
• compute

$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR.}}$

View the build log

[modular-magician]

Tests analytics

Total tests: 1137
Passed tests: 1057
Skipped tests: 80
Affected tests: 0

Click here to see the affected service packages

• compute
• kms
• orgpolicy
• resourcemanager
• securitycenter
• securitycentermanagement
• accessapproval
• cloudasset
• logging
• assuredworkloads
• iam2

$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR.}}$

View the build log

[abd-goog]

@melinath could you please review?
I don't see any failures in the VCR logs related to this change, lmk if I'm missing something.

[melinath]

I don't see anything either - rerunning to see if it's a flake /gcbrun

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 41 files changed, 145 insertions(+), 6 deletions(-))
google-beta provider: Diff ( 45 files changed, 150 insertions(+), 6 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 3 insertions(+))

[modular-magician]

Tests analytics

Total tests: 1137
Passed tests: 1057
Skipped tests: 80
Affected tests: 0

Click here to see the affected service packages

• assuredworkloads
• kms
• resourcemanager
• accessapproval
• compute
• iam2
• logging
• orgpolicy
• securitycenter
• securitycentermanagement
• cloudasset

$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR.}}$

View the build log

[melinath]

It looks like the error is coming from the data source tests, because the folder datasource doesn't have deletion_protection in its schema. You'll need to add it to the schema (and add a few lines that set it to nil, because it's not meaningful in that context.)

Something like:

# In schema
"deletion_protection": {
	Type:        schema.TypeBool,
	Computed:    true,
},

# In Read
	if err := d.Set("deletion_protection", nil); err != nil {
		return fmt.Errorf("Error setting deletion_protection: %s", err)
	}

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 42 files changed, 153 insertions(+), 6 deletions(-))
google-beta provider: Diff ( 46 files changed, 158 insertions(+), 6 deletions(-))
terraform-google-conversion: Diff ( 3 files changed, 3 insertions(+))

[modular-magician]

Tests analytics

Total tests: 1273
Passed tests: 1162
Skipped tests: 111
Affected tests: 0

Click here to see the affected service packages

• securitycenter
• securitycentermanagement
• accessapproval
• assuredworkloads
• logging
• resourcemanager
• orgpolicy
• cloudasset
• compute
• iam2
• kms

$\textcolor{green}{\textsf{All tests passed!}}$

View the build log

[abd-goog]

It looks like the error is coming from the data source tests, because the folder datasource doesn't have deletion_protection in its schema. You'll need to add it to the schema (and add a few lines that set it to nil, because it's not meaningful in that context.)

Something like:

# In schema
"deletion_protection": {
	Type:        schema.TypeBool,
	Computed:    true,
},

# In Read
	if err := d.Set("deletion_protection", nil); err != nil {
		return fmt.Errorf("Error setting deletion_protection: %s", err)
	}

Done, thanks!
Though I wonder why is the schema explicitly redeclared in data source instead of reusing the schema from the resource.

[melinath]

Though I wonder why is the schema explicitly redeclared in data source instead of reusing the schema from the resource.

The google_folder data source almost certainly pre-dates the tooling to reuse the resource schema... we could hypothetically switch it over but there probably hasn't been a reason to (and I'm not 100% sure it would Just Work without doing further investigation.)

[melinath]

LGTM, thanks!