-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Fix notebooks instances recreating for scopes and CMEK #9915
Conversation
Hello! I am a robot. It looks like you are a: @SarahFrench, 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. |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 75 insertions(+), 3 deletions(-)) |
Tests analyticsTotal tests: Click here to see the affected service packages
Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccNotebooksRuntime_update|TestAccNotebooksInstance_notebookInstanceFullExample |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 75 insertions(+), 3 deletions(-)) |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 72 insertions(+), 3 deletions(-)) |
Tests analyticsTotal tests: Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccNotebooksInstance_notebookInstanceFullExample |
|
The recent error with the acceptance tests are due to this code being generated with empty strings as values for vars:
instance_name: 'notebooks-instance'
+ key_name: 'my-key'
+ keyring_name: 'my-keyring' |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 72 insertions(+), 3 deletions(-)) |
Tests analyticsTotal tests: Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccNotebooksInstance_notebookInstanceFullExample |
|
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.
Some refactoring suggestions and a question about the issue with scopes.
Also, the latest test failure is due to a diff:
~ kms_key = "projects/ci-test-project-188019/locations/global/keyRings/tf-test-tftest-shared-keyring-12bfr7fw32x/cryptoKeys/tf-test-tftest-shared-key-12bfr7fw32x/cryptoKeyVersions/1" -> "projects/ci-test-project-188019/locations/global/keyRings/tf-test-tftest-shared-keyring-12bfr7fw32x/cryptoKeys/tf-test-tftest-shared-key-12bfr7fw32x" # forces replacement
The POST request to create the resource doesn't include cryptoKeyVersions/1
in the value, but the response from the API includes cryptoKeyVersions/1
. This could be addressed by a diff suppress function also.
mmv1/templates/terraform/examples/notebook_instance_full.tf.erb
Outdated
Show resolved
Hide resolved
Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com>
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 53 insertions(+), 3 deletions(-)) |
Tests analyticsTotal tests: Click here to see the affected service packages
|
Co-authored-by: Cameron Thornton <camthornton@google.com>
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 3 files changed, 64 insertions(+), 6 deletions(-)) |
Tests analyticsTotal tests: Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccNotebooksInstance_notebookInstanceFullExample |
Rerun these tests in REPLAYING mode to catch issues
|
Hey @SarahFrench, could you please take a look at the PR. |
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.
Sorry for the delay - I was juggling a few tasks and wanted to do a quick manual test of this PR before approving
…atform#9915) * add labels to runtime update test * fix notebooks instances recreating for scopes and CMEK * Update resource_notebooks_runtime_test.go.erb * Update Instance.yaml * Apply suggestions from code review regarding kms keys Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> * Update notebook_instance_full.tf.erb * remove scope diff suppress * add vars in test * Enable parallelstore API (GoogleCloudPlatform#9919) * Add Cloud Deploy Delivery Pipeline (GoogleCloudPlatform#9888) Co-authored-by: Cameron Thornton <camthornton@google.com> * Promote support for replica mv in resource_bigquery_table to GA (GoogleCloudPlatform#9909) * Ignore .github folder when using Copywrite tool (GoogleCloudPlatform#9920) * networksecurity/AddressGroup: remove unused custom code (GoogleCloudPlatform#9829) Co-authored-by: Luca Prete <lucaprete@google.com> * Add fine grained resource for instance group membership (GoogleCloudPlatform#9891) * Add fine-grained resource * Add InstanceGroupMembership resource * add acceptance tests for the resource * Add message to the resource * Fix trailing spaces * Fix trailing spaces * Add zone to test * Add zone to test * Review fixes - change fields labels, update descriptions and indentation * Undo mistake change * Remove flatten from the encoder * Remove "Region" prefix in the test files and decoder, as it could be replaces with the existingfill_to_relative_path custom_flatten * Add beta support for confidentialInstanceType (GoogleCloudPlatform#9847) * Modify posture deployment field descriptions (GoogleCloudPlatform#9900) * modify posture dpeloyment field description * Update mmv1/products/securityposture/PostureDeployment.yaml Co-authored-by: Nick Elliot <nickelliot@google.com> * Update mmv1/products/securityposture/PostureDeployment.yaml Co-authored-by: Nick Elliot <nickelliot@google.com> --------- Co-authored-by: Sneha Prasad <snehaprasad@google.com> Co-authored-by: Nick Elliot <nickelliot@google.com> * fixing version == beta to unless version = ga (GoogleCloudPlatform#9922) * Fix branching for beta (GoogleCloudPlatform#9928) * fixing version == beta to unless version = ga * fixing beta branching * fix notebooks instances recreating for scopes and CMEK * diff suppress for kms key * remove scope diff suppress * add vars in test * change kms key to use existing one in tests * add kms key diff suppress * add acctest. to BootstrapKMSKeyInLocation * add vars in test * change kms key to use existing one in tests * add vars in test * change kms key to use existing one in tests * fix notebooks instances recreating for scopes and CMEK * fix merge conflicts * add labels to runtime update test * fix notebooks instances recreating for scopes and CMEK * Update resource_notebooks_runtime_test.go.erb * Update Instance.yaml * Apply suggestions from code review regarding kms keys Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> * Update notebook_instance_full.tf.erb * remove scope diff suppress * add vars in test * fix notebooks instances recreating for scopes and CMEK * diff suppress for kms key * remove scope diff suppress * add vars in test * change kms key to use existing one in tests * add kms key diff suppress * add acctest. to BootstrapKMSKeyInLocation * add vars in test * change kms key to use existing one in tests * add vars in test * change kms key to use existing one in tests * fix notebooks instances recreating for scopes and CMEK * fix merge conflicts * retrigger checks --------- Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> Co-authored-by: Shogo Watanabe <1286807+nownabe@users.noreply.github.com> Co-authored-by: Cameron Thornton <camthornton@google.com> Co-authored-by: wj-chen <wjchen@google.com> Co-authored-by: Luca Prete <preteluca@gmail.com> Co-authored-by: Luca Prete <lucaprete@google.com> Co-authored-by: Filip 'Cherit' Szóstak <filipszostak@google.com> Co-authored-by: Dionna Amalie Glaze <drdeeglaze@gmail.com> Co-authored-by: Sneha Prasad <32434989+snpd25@users.noreply.github.com> Co-authored-by: Sneha Prasad <snehaprasad@google.com> Co-authored-by: Nick Elliot <nickelliot@google.com> Co-authored-by: unnatinadupalli <156947936+unnatinadupalli@users.noreply.github.com>
…atform#9915) * add labels to runtime update test * fix notebooks instances recreating for scopes and CMEK * Update resource_notebooks_runtime_test.go.erb * Update Instance.yaml * Apply suggestions from code review regarding kms keys Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> * Update notebook_instance_full.tf.erb * remove scope diff suppress * add vars in test * Enable parallelstore API (GoogleCloudPlatform#9919) * Add Cloud Deploy Delivery Pipeline (GoogleCloudPlatform#9888) Co-authored-by: Cameron Thornton <camthornton@google.com> * Promote support for replica mv in resource_bigquery_table to GA (GoogleCloudPlatform#9909) * Ignore .github folder when using Copywrite tool (GoogleCloudPlatform#9920) * networksecurity/AddressGroup: remove unused custom code (GoogleCloudPlatform#9829) Co-authored-by: Luca Prete <lucaprete@google.com> * Add fine grained resource for instance group membership (GoogleCloudPlatform#9891) * Add fine-grained resource * Add InstanceGroupMembership resource * add acceptance tests for the resource * Add message to the resource * Fix trailing spaces * Fix trailing spaces * Add zone to test * Add zone to test * Review fixes - change fields labels, update descriptions and indentation * Undo mistake change * Remove flatten from the encoder * Remove "Region" prefix in the test files and decoder, as it could be replaces with the existingfill_to_relative_path custom_flatten * Add beta support for confidentialInstanceType (GoogleCloudPlatform#9847) * Modify posture deployment field descriptions (GoogleCloudPlatform#9900) * modify posture dpeloyment field description * Update mmv1/products/securityposture/PostureDeployment.yaml Co-authored-by: Nick Elliot <nickelliot@google.com> * Update mmv1/products/securityposture/PostureDeployment.yaml Co-authored-by: Nick Elliot <nickelliot@google.com> --------- Co-authored-by: Sneha Prasad <snehaprasad@google.com> Co-authored-by: Nick Elliot <nickelliot@google.com> * fixing version == beta to unless version = ga (GoogleCloudPlatform#9922) * Fix branching for beta (GoogleCloudPlatform#9928) * fixing version == beta to unless version = ga * fixing beta branching * fix notebooks instances recreating for scopes and CMEK * diff suppress for kms key * remove scope diff suppress * add vars in test * change kms key to use existing one in tests * add kms key diff suppress * add acctest. to BootstrapKMSKeyInLocation * add vars in test * change kms key to use existing one in tests * add vars in test * change kms key to use existing one in tests * fix notebooks instances recreating for scopes and CMEK * fix merge conflicts * add labels to runtime update test * fix notebooks instances recreating for scopes and CMEK * Update resource_notebooks_runtime_test.go.erb * Update Instance.yaml * Apply suggestions from code review regarding kms keys Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> * Update notebook_instance_full.tf.erb * remove scope diff suppress * add vars in test * fix notebooks instances recreating for scopes and CMEK * diff suppress for kms key * remove scope diff suppress * add vars in test * change kms key to use existing one in tests * add kms key diff suppress * add acctest. to BootstrapKMSKeyInLocation * add vars in test * change kms key to use existing one in tests * add vars in test * change kms key to use existing one in tests * fix notebooks instances recreating for scopes and CMEK * fix merge conflicts * retrigger checks --------- Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> Co-authored-by: Shogo Watanabe <1286807+nownabe@users.noreply.github.com> Co-authored-by: Cameron Thornton <camthornton@google.com> Co-authored-by: wj-chen <wjchen@google.com> Co-authored-by: Luca Prete <preteluca@gmail.com> Co-authored-by: Luca Prete <lucaprete@google.com> Co-authored-by: Filip 'Cherit' Szóstak <filipszostak@google.com> Co-authored-by: Dionna Amalie Glaze <drdeeglaze@gmail.com> Co-authored-by: Sneha Prasad <32434989+snpd25@users.noreply.github.com> Co-authored-by: Sneha Prasad <snehaprasad@google.com> Co-authored-by: Nick Elliot <nickelliot@google.com> Co-authored-by: unnatinadupalli <156947936+unnatinadupalli@users.noreply.github.com>
…atform#9915) * add labels to runtime update test * fix notebooks instances recreating for scopes and CMEK * Update resource_notebooks_runtime_test.go.erb * Update Instance.yaml * Apply suggestions from code review regarding kms keys Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> * Update notebook_instance_full.tf.erb * remove scope diff suppress * add vars in test * Enable parallelstore API (GoogleCloudPlatform#9919) * Add Cloud Deploy Delivery Pipeline (GoogleCloudPlatform#9888) Co-authored-by: Cameron Thornton <camthornton@google.com> * Promote support for replica mv in resource_bigquery_table to GA (GoogleCloudPlatform#9909) * Ignore .github folder when using Copywrite tool (GoogleCloudPlatform#9920) * networksecurity/AddressGroup: remove unused custom code (GoogleCloudPlatform#9829) Co-authored-by: Luca Prete <lucaprete@google.com> * Add fine grained resource for instance group membership (GoogleCloudPlatform#9891) * Add fine-grained resource * Add InstanceGroupMembership resource * add acceptance tests for the resource * Add message to the resource * Fix trailing spaces * Fix trailing spaces * Add zone to test * Add zone to test * Review fixes - change fields labels, update descriptions and indentation * Undo mistake change * Remove flatten from the encoder * Remove "Region" prefix in the test files and decoder, as it could be replaces with the existingfill_to_relative_path custom_flatten * Add beta support for confidentialInstanceType (GoogleCloudPlatform#9847) * Modify posture deployment field descriptions (GoogleCloudPlatform#9900) * modify posture dpeloyment field description * Update mmv1/products/securityposture/PostureDeployment.yaml Co-authored-by: Nick Elliot <nickelliot@google.com> * Update mmv1/products/securityposture/PostureDeployment.yaml Co-authored-by: Nick Elliot <nickelliot@google.com> --------- Co-authored-by: Sneha Prasad <snehaprasad@google.com> Co-authored-by: Nick Elliot <nickelliot@google.com> * fixing version == beta to unless version = ga (GoogleCloudPlatform#9922) * Fix branching for beta (GoogleCloudPlatform#9928) * fixing version == beta to unless version = ga * fixing beta branching * fix notebooks instances recreating for scopes and CMEK * diff suppress for kms key * remove scope diff suppress * add vars in test * change kms key to use existing one in tests * add kms key diff suppress * add acctest. to BootstrapKMSKeyInLocation * add vars in test * change kms key to use existing one in tests * add vars in test * change kms key to use existing one in tests * fix notebooks instances recreating for scopes and CMEK * fix merge conflicts * add labels to runtime update test * fix notebooks instances recreating for scopes and CMEK * Update resource_notebooks_runtime_test.go.erb * Update Instance.yaml * Apply suggestions from code review regarding kms keys Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> * Update notebook_instance_full.tf.erb * remove scope diff suppress * add vars in test * fix notebooks instances recreating for scopes and CMEK * diff suppress for kms key * remove scope diff suppress * add vars in test * change kms key to use existing one in tests * add kms key diff suppress * add acctest. to BootstrapKMSKeyInLocation * add vars in test * change kms key to use existing one in tests * add vars in test * change kms key to use existing one in tests * fix notebooks instances recreating for scopes and CMEK * fix merge conflicts * retrigger checks --------- Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> Co-authored-by: Shogo Watanabe <1286807+nownabe@users.noreply.github.com> Co-authored-by: Cameron Thornton <camthornton@google.com> Co-authored-by: wj-chen <wjchen@google.com> Co-authored-by: Luca Prete <preteluca@gmail.com> Co-authored-by: Luca Prete <lucaprete@google.com> Co-authored-by: Filip 'Cherit' Szóstak <filipszostak@google.com> Co-authored-by: Dionna Amalie Glaze <drdeeglaze@gmail.com> Co-authored-by: Sneha Prasad <32434989+snpd25@users.noreply.github.com> Co-authored-by: Sneha Prasad <snehaprasad@google.com> Co-authored-by: Nick Elliot <nickelliot@google.com> Co-authored-by: unnatinadupalli <156947936+unnatinadupalli@users.noreply.github.com>
…atform#9915) * add labels to runtime update test * fix notebooks instances recreating for scopes and CMEK * Update resource_notebooks_runtime_test.go.erb * Update Instance.yaml * Apply suggestions from code review regarding kms keys Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> * Update notebook_instance_full.tf.erb * remove scope diff suppress * add vars in test * Enable parallelstore API (GoogleCloudPlatform#9919) * Add Cloud Deploy Delivery Pipeline (GoogleCloudPlatform#9888) Co-authored-by: Cameron Thornton <camthornton@google.com> * Promote support for replica mv in resource_bigquery_table to GA (GoogleCloudPlatform#9909) * Ignore .github folder when using Copywrite tool (GoogleCloudPlatform#9920) * networksecurity/AddressGroup: remove unused custom code (GoogleCloudPlatform#9829) Co-authored-by: Luca Prete <lucaprete@google.com> * Add fine grained resource for instance group membership (GoogleCloudPlatform#9891) * Add fine-grained resource * Add InstanceGroupMembership resource * add acceptance tests for the resource * Add message to the resource * Fix trailing spaces * Fix trailing spaces * Add zone to test * Add zone to test * Review fixes - change fields labels, update descriptions and indentation * Undo mistake change * Remove flatten from the encoder * Remove "Region" prefix in the test files and decoder, as it could be replaces with the existingfill_to_relative_path custom_flatten * Add beta support for confidentialInstanceType (GoogleCloudPlatform#9847) * Modify posture deployment field descriptions (GoogleCloudPlatform#9900) * modify posture dpeloyment field description * Update mmv1/products/securityposture/PostureDeployment.yaml Co-authored-by: Nick Elliot <nickelliot@google.com> * Update mmv1/products/securityposture/PostureDeployment.yaml Co-authored-by: Nick Elliot <nickelliot@google.com> --------- Co-authored-by: Sneha Prasad <snehaprasad@google.com> Co-authored-by: Nick Elliot <nickelliot@google.com> * fixing version == beta to unless version = ga (GoogleCloudPlatform#9922) * Fix branching for beta (GoogleCloudPlatform#9928) * fixing version == beta to unless version = ga * fixing beta branching * fix notebooks instances recreating for scopes and CMEK * diff suppress for kms key * remove scope diff suppress * add vars in test * change kms key to use existing one in tests * add kms key diff suppress * add acctest. to BootstrapKMSKeyInLocation * add vars in test * change kms key to use existing one in tests * add vars in test * change kms key to use existing one in tests * fix notebooks instances recreating for scopes and CMEK * fix merge conflicts * add labels to runtime update test * fix notebooks instances recreating for scopes and CMEK * Update resource_notebooks_runtime_test.go.erb * Update Instance.yaml * Apply suggestions from code review regarding kms keys Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> * Update notebook_instance_full.tf.erb * remove scope diff suppress * add vars in test * fix notebooks instances recreating for scopes and CMEK * diff suppress for kms key * remove scope diff suppress * add vars in test * change kms key to use existing one in tests * add kms key diff suppress * add acctest. to BootstrapKMSKeyInLocation * add vars in test * change kms key to use existing one in tests * add vars in test * change kms key to use existing one in tests * fix notebooks instances recreating for scopes and CMEK * fix merge conflicts * retrigger checks --------- Co-authored-by: Sarah French <15078782+SarahFrench@users.noreply.github.com> Co-authored-by: Shogo Watanabe <1286807+nownabe@users.noreply.github.com> Co-authored-by: Cameron Thornton <camthornton@google.com> Co-authored-by: wj-chen <wjchen@google.com> Co-authored-by: Luca Prete <preteluca@gmail.com> Co-authored-by: Luca Prete <lucaprete@google.com> Co-authored-by: Filip 'Cherit' Szóstak <filipszostak@google.com> Co-authored-by: Dionna Amalie Glaze <drdeeglaze@gmail.com> Co-authored-by: Sneha Prasad <32434989+snpd25@users.noreply.github.com> Co-authored-by: Sneha Prasad <snehaprasad@google.com> Co-authored-by: Nick Elliot <nickelliot@google.com> Co-authored-by: unnatinadupalli <156947936+unnatinadupalli@users.noreply.github.com>
service_account_scopes
hashicorp/terraform-provider-google#16587Release Note Template for Downstream PRs (will be copied)