Make terraform_labels field immutable for immutable resources #9394

zli82016 · 2023-11-02T19:47:34Z

fixes hashicorp/terraform-provider-google#16374
part of hashicorp/terraform-provider-google#16424

ForceNew is added to the following resources. The state upgrades are needed for these resources to avoid resource recreation when upgrading to provider 5.x.

Beyondcorp_app_gateway
Bigquery_job
Certificate_manager_certificate_issuance_config
Cloud_run_domain_mapping
Database_migration_servcie_private_connection
Datastream_private_connection
Ml_engine_mode
Network_connectivity_policy_basd_route
Network_services_service_binding
Dataproc_workflow_template (DCL)

Empty update method is added to the handwritten resources and ForceNew is removed from 'labels' field to address the error All fields are ForceNew or Computed w/out Optional, Update is superfluous

Compute_instance_template
Compute_region_instance_template

Unaffected resources with update methods:

Dataflow_flex_template
dataflow_job
tpu_node
Certificate_manager_trust_confgi
Dataproc_job
Edge_container_vpn_connection

Release Note Template for Downstream PRs (will be copied)

provider: made `terraform_labels` immutable in immutable resources to not block the upgrade. This will create a Terraform plan that recreates the resource on `4.X` -> `5.6.0` upgrade for affected resources. A mitigation to backfill the values during the upgrade is planned, and will release resource-by-resource.

modular-magician · 2023-11-02T19:57:29Z

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.

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

NickElliot · 2023-11-02T20:34:13Z

The diff appears to be missing all generated immutable resources such as google_cloud_run_domain_mapping

modular-magician · 2023-11-02T21:09:11Z

Tests analytics

Total tests: 3197
Passed tests 2872
Skipped tests: 325
Affected tests: 0

$\textcolor{green}{\textsf{All tests passed in REPLAYING mode.}}$
View the build log

zli82016 · 2023-11-02T21:23:35Z

The diff appears to be missing all generated immutable resources such as google_cloud_run_domain_mapping

Yeah, submitted a new revision.

modular-magician · 2023-11-02T21:32:31Z

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.

Terraform GA: Diff ( 24 files changed, 24 insertions(+))
Terraform Beta: Diff ( 27 files changed, 27 insertions(+))

modular-magician · 2023-11-02T21:58:14Z

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.

Terraform GA: Diff ( 15 files changed, 15 insertions(+))
Terraform Beta: Diff ( 16 files changed, 16 insertions(+))

modular-magician · 2023-11-02T23:11:02Z

Tests analytics

Total tests: 3198
Passed tests 2871
Skipped tests: 325
Affected tests: 2

Action taken

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

TestAccDataprocClusterIamPolicy|TestAccSpannerDatabaseIamPolicy

Get to know how VCR tests work

modular-magician · 2023-11-02T23:22:41Z

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.

Terraform GA: Diff ( 19 files changed, 122 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 22 files changed, 161 insertions(+), 2 deletions(-))

modular-magician · 2023-11-03T00:34:46Z

Tests analytics

Total tests: 3202
Passed tests 2873
Skipped tests: 327
Affected tests: 2

Action taken

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

TestAccCloudRunDomainMapping_migration|TestAccComputeInstanceTemplate_migration

Get to know how VCR tests work

modular-magician · 2023-11-03T00:59:34Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccCloudRunDomainMapping_migration[Debug log]
TestAccComputeInstanceTemplate_migration[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$
TestAccCloudRunDomainMapping_migration[Error message] [Debug log]
TestAccComputeInstanceTemplate_migration[Error message] [Debug log]

Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.

Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.

$\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 · 2023-11-03T02:03:01Z

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.

Terraform GA: Diff ( 20 files changed, 126 insertions(+), 3 deletions(-))
Terraform Beta: Diff ( 23 files changed, 165 insertions(+), 4 deletions(-))

modular-magician · 2023-11-03T03:14:32Z

Tests analytics

Total tests: 3204
Passed tests 2871
Skipped tests: 329
Affected tests: 4

Action taken

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

TestAccLoggingBucketConfigProject_locked|TestAccLoggingProjectSink_updatePreservesCustomWriter|TestAccDataSourceGoogleServiceAccountAccessToken_basic|TestAccDataSourceGoogleServiceAccountIdToken_impersonation

Get to know how VCR tests work

modular-magician · 2023-11-03T03:20:33Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDataSourceGoogleServiceAccountAccessToken_basic[Debug log]
TestAccDataSourceGoogleServiceAccountIdToken_impersonation[Debug log]

Rerun these tests in REPLAYING mode to catch issues

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

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

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

modular-magician · 2023-11-03T21:38:07Z

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.

Terraform GA: Diff ( 16 files changed, 122 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 18 files changed, 160 insertions(+), 2 deletions(-))

zli82016 · 2023-11-03T21:41:09Z

I will move forward with add ForceNew: true to terraform_labels field.

I tested with the solution to use empty update, but stopped working on it for a couple of reasons.

It will modify the current model. For any new immutable resources, it needs to add the custom_update code. It will introduce confusion to the contributors.
After adding update method to the immutable resource with metadata.labels field (google_cloud_run_domain_mapping ), an error is returned

All fields are ForceNew or Computed w/out Optional, Update is superfluous

rileykarson · 2023-11-03T21:48:22Z

All fields are ForceNew or Computed w/out Optional, Update is superfluous

Ah, because terraform_labels is Computed+ForceNew, which is not well-recognised by Terraform. Got it!

zli82016 · 2023-11-03T21:57:06Z

All fields are ForceNew or Computed w/out Optional, Update is superfluous

Ah, because terraform_labels is Optional+ForceNew, which is not well-recognised by Terraform. Got it!

Yeah. For the immutable resource with top level terraform_labels field, this error can be addressed by removing ForceNew from top level labels field, then labels field needs update.

But for the immutable resource with metadata.terraform_labels, because all of top level fields are ForceNew, even making metadata.labels without ForceNew, it is not recognized by Terraform.

modular-magician · 2023-11-03T21:57:09Z

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.

Terraform GA: Diff ( 15 files changed, 121 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 17 files changed, 159 insertions(+), 2 deletions(-))

rileykarson

Haven't made a full pass yet, but will Mon

mmv1/third_party/terraform/website/docs/guides/version_5_upgrade.html.markdown

mmv1/third_party/terraform/services/dataflow/resource_dataflow_flex_template_job_test.go.erb

modular-magician · 2023-11-03T22:55:11Z

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.

Terraform GA: Diff ( 15 files changed, 278 insertions(+), 4 deletions(-))
Terraform Beta: Diff ( 17 files changed, 316 insertions(+), 4 deletions(-))

modular-magician · 2023-11-03T23:05:37Z

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.

Terraform GA: Diff ( 15 files changed, 280 insertions(+), 3 deletions(-))
Terraform Beta: Diff ( 17 files changed, 318 insertions(+), 3 deletions(-))

modular-magician · 2023-11-07T19:04:34Z

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.

Terraform GA: Diff ( 14 files changed, 309 insertions(+), 4 deletions(-))
Terraform Beta: Diff ( 15 files changed, 311 insertions(+), 4 deletions(-))

modular-magician · 2023-11-07T20:28:38Z

Tests analytics

Total tests: 3211
Passed tests 2881
Skipped tests: 328
Affected tests: 2

Action taken

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

TestAccDataprocJobIamPolicy|TestAccSpannerDatabaseIamPolicy

Get to know how VCR tests work

modular-magician · 2023-11-07T20:37:36Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDataprocJobIamPolicy[Debug log]
TestAccSpannerDatabaseIamPolicy[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\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

…CloudPlatform#9394) * Make terraform_labels field immutable for immutable resources * Revert change for dataflow job * Skip updatable resources * Use empty update for the handwritten immutable resources * Update upgrade guide * Remove dataflow tests * Address comments * Add forceNew to all subfields

zli82016 force-pushed the terraform-labels-immutable branch 2 times, most recently from 68a2f1a to 37838a2 Compare November 2, 2023 21:22

zli82016 force-pushed the terraform-labels-immutable branch from 37838a2 to d9382e4 Compare November 2, 2023 21:48

zli82016 force-pushed the terraform-labels-immutable branch from d9382e4 to 6e5401b Compare November 2, 2023 23:12

zli82016 force-pushed the terraform-labels-immutable branch from 6e5401b to 7d92968 Compare November 3, 2023 01:53

zli82016 requested a review from rileykarson November 3, 2023 15:19

zli82016 closed this Nov 3, 2023

zli82016 reopened this Nov 3, 2023

zli82016 force-pushed the terraform-labels-immutable branch from 24d95ed to 6fd8930 Compare November 3, 2023 21:47

rileykarson reviewed Nov 3, 2023

View reviewed changes

mmv1/third_party/terraform/website/docs/guides/version_5_upgrade.html.markdown Outdated Show resolved Hide resolved

mmv1/third_party/terraform/services/dataflow/resource_dataflow_flex_template_job_test.go.erb Outdated Show resolved Hide resolved

zli82016 added 2 commits November 7, 2023 10:35

Remove dataflow tests

4efcc88

Address comments

949fb6c

zli82016 force-pushed the terraform-labels-immutable branch 2 times, most recently from 97821a5 to bbcf1a8 Compare November 7, 2023 18:50

Add forceNew to all subfields

da34c4f

zli82016 force-pushed the terraform-labels-immutable branch from bbcf1a8 to da34c4f Compare November 7, 2023 18:51

zli82016 requested a review from rileykarson November 7, 2023 20:53

rileykarson approved these changes Nov 7, 2023

View reviewed changes

zli82016 merged commit 0756b25 into main Nov 7, 2023
6 checks passed

zli82016 deleted the terraform-labels-immutable branch November 7, 2023 21:06

This was referenced Nov 7, 2023

Make terraform_labels field immutable for immutable resources hashicorp/terraform-provider-google-beta#6613

Merged

Make terraform_labels field immutable for immutable resources hashicorp/terraform-provider-google#16469

Merged

zli82016 mentioned this pull request Nov 8, 2023

terraform_labels can't be updated on google_compute_instance_template hashicorp/terraform-provider-google#16374

Closed

melinath added service/terraform and removed service/dataproc service/datastream service/beyondcorp service/run service/bigquery service/compute-instances service/aiplatform-prediction service/datamigration service/certificatemanager service/networkservices-traffic-director labels Mar 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make terraform_labels field immutable for immutable resources #9394

Make terraform_labels field immutable for immutable resources #9394

zli82016 commented Nov 2, 2023 •

edited

Loading

modular-magician commented Nov 2, 2023

NickElliot commented Nov 2, 2023

modular-magician commented Nov 2, 2023

zli82016 commented Nov 2, 2023

modular-magician commented Nov 2, 2023

modular-magician commented Nov 2, 2023

modular-magician commented Nov 2, 2023

modular-magician commented Nov 2, 2023

modular-magician commented Nov 3, 2023

modular-magician commented Nov 3, 2023

modular-magician commented Nov 3, 2023

modular-magician commented Nov 3, 2023

modular-magician commented Nov 3, 2023

modular-magician commented Nov 3, 2023

zli82016 commented Nov 3, 2023

rileykarson commented Nov 3, 2023 •

edited

Loading

zli82016 commented Nov 3, 2023 •

edited

Loading

modular-magician commented Nov 3, 2023

rileykarson left a comment

modular-magician commented Nov 3, 2023

modular-magician commented Nov 3, 2023

modular-magician commented Nov 7, 2023

modular-magician commented Nov 7, 2023

modular-magician commented Nov 7, 2023

Make terraform_labels field immutable for immutable resources #9394

Make terraform_labels field immutable for immutable resources #9394

Conversation

zli82016 commented Nov 2, 2023 • edited Loading

modular-magician commented Nov 2, 2023

Diff report

NickElliot commented Nov 2, 2023

modular-magician commented Nov 2, 2023

Tests analytics

zli82016 commented Nov 2, 2023

modular-magician commented Nov 2, 2023

Diff report

modular-magician commented Nov 2, 2023

Diff report

modular-magician commented Nov 2, 2023

Tests analytics

Action taken

modular-magician commented Nov 2, 2023

Diff report

modular-magician commented Nov 3, 2023

Tests analytics

Action taken

modular-magician commented Nov 3, 2023

Rerun these tests in REPLAYING mode to catch issues

modular-magician commented Nov 3, 2023

Diff report

modular-magician commented Nov 3, 2023

Tests analytics

Action taken

modular-magician commented Nov 3, 2023

Rerun these tests in REPLAYING mode to catch issues

modular-magician commented Nov 3, 2023

Diff report

zli82016 commented Nov 3, 2023

rileykarson commented Nov 3, 2023 • edited Loading

zli82016 commented Nov 3, 2023 • edited Loading

modular-magician commented Nov 3, 2023

Diff report

rileykarson left a comment

Choose a reason for hiding this comment

modular-magician commented Nov 3, 2023

Diff report

modular-magician commented Nov 3, 2023

Diff report

modular-magician commented Nov 7, 2023

Diff report

modular-magician commented Nov 7, 2023

Tests analytics

Action taken

modular-magician commented Nov 7, 2023

Rerun these tests in REPLAYING mode to catch issues

zli82016 commented Nov 2, 2023 •

edited

Loading

rileykarson commented Nov 3, 2023 •

edited

Loading

zli82016 commented Nov 3, 2023 •

edited

Loading