Changed the type of automatic field and added support for customer_managed_encryption in automatic field#8662
Conversation
…naged_encryption in automatic field
|
Hello! I am a robot. It looks like you are a: Community Contributor @slevenick, 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. |
|
@slevenick While generating the providers after changing the automatic field to block type in the terraform configs (i.e tests and examples), I encountered that for the documentation for the resources |
|
@slevenick Could you please review this PR? |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Breaking Change(s) DetectedThe following breaking change(s) were detected within your pull request.
If you believe this detection to be incorrect please raise the concern with your reviewer. If you intend to make this change you will need to wait for a major release window. An Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 25 files changed, 558 insertions(+), 48 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 16 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDataSourceSecretManagerSecret_basic|TestAccCloudBuildTrigger_cloudbuildTriggerWebhookConfigExample|TestAccCloudfunctions2function_cloudfunctions2SecretVolumeExample|TestAccCloudfunctions2function_cloudfunctions2SecretEnvExample|TestAccCloudBuildTrigger_webhook_config|TestAccSecretManagerSecretIamMemberGenerated|TestAccSecretManagerSecretIamBindingGenerated|TestAccCloudFunctionsFunction_secretMount|TestAccCloudFunctionsFunction_secretEnvVar|TestAccSecretManagerSecretIamPolicyGenerated|TestAccSecretManagerSecret_import|TestAccSecretManagerSecret_secretWithAutomaticCmekExample|TestAccSecretManagerSecret_cmek|TestAccSecretManagerSecret_secretConfigBasicExample|TestAccSecretManagerSecret_automaticCmekUpdate|TestAccSecretManagerSecret_annotationsUpdate |
|
Rerun these tests in REPLAYING mode to catch issues
|
|
I need to check if this is an acceptable change. My understanding is that we need to support an upgrade path through the last 4.x version to 5.0.0, which may not be possible with this PR. |
slevenick
left a comment
There was a problem hiding this comment.
See #8721
I don't think this is an acceptable change to make as implemented here.
The alternative that I can see would be changing the name of the field so that we can support the side-by-side of the old field and the new field in the same version of 4.x, and then removing the old field in 5.0.0
| replication { | ||
| automatic { | ||
| customer_managed_encryption { | ||
| kms_key_name = "<%= ctx[:vars]['kms_key_name'] %>" |
There was a problem hiding this comment.
Where is the kms_key and keyring defined?
For automatic replication location is global and CMEK should also be defined in a global keyring.
There was a problem hiding this comment.
I've used the function BootstrapKMSKey as mentioned in the Secret.yaml that will use a particular keyring and a particular key i.e. tftest-shared-keyring-1 and tftest-shared-key-1 respectively in the test environment. The implementation of this function can be found here: https://github.com/GoogleCloudPlatform/magic-modules/blob/main/mmv1/third_party/terraform/acctest/bootstrap_test_utils.go#L42.
@slevenick Just FYI, the |
It's definitely hard to do to support two fields side-by-side, but I think it can be done. It might require a complicated CustomizeDiff function in the version where both are supported |
@slevenick I am thinking of adding custom_flatten and custom_expand for the |
|
This was abandoned in favor of #8838 |
Changed the type of the
automaticfield from TypeBool to TypeList and added support for thereplication.automatic.customer_managed_encryptionfield in thegoogle_secret_manager_secretresource.fixes hashicorp/terraform-provider-google#10679
fixes hashicorp/terraform-provider-google#9272
fixes hashicorp/terraform-provider-google#14282
If this PR is for Terraform, I acknowledge that I have:
make testandmake lintin the generated providers to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)