Add new types KeyValueLabels and KeyValueAnnotations by zli82016 · Pull Request #8519 · GoogleCloudPlatform/magic-modules

zli82016 · 2023-08-02T16:36:13Z

Add the new type KeyValueLables and KeyValueAnnotations, which will trigger to add the field effective_labels or effective_annotaitons to the properties.

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).
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).
Generated Terraform providers, and ran make test and make lint in the generated providers to ensure it passes unit and linter tests.
Ran relevant acceptance tests using my own Google Cloud project and credentials (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read Write release notes before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

modular-magician · 2023-08-02T16:49:24Z

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

Field labels transitioned from optional+computed to optional google_bigquery_dataset - reference

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 override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 6 files changed, 143 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 6 files changed, 291 insertions(+), 8 deletions(-))
TF Conversion: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-08-02T17:45:41Z

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

Field labels transitioned from optional+computed to optional google_bigquery_dataset - reference

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 override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 6 files changed, 151 insertions(+), 8 deletions(-))
Terraform Beta: Diff ( 6 files changed, 306 insertions(+), 13 deletions(-))
TF Conversion: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-08-02T19:03:16Z

Tests analytics

Total tests: 2922
Passed tests 2604
Skipped tests: 302
Affected tests: 16

Action taken

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

TestAccDataSourceComputeAddresses|TestAccBigQueryDataTable_bigtable|TestAccBigQueryDataset_bigqueryDatasetDefaultCollationSetExample|TestAccBigQueryDataset_bigqueryDatasetWithMaxTimeTravelHoursExample|TestAccBigQueryDataset_bigqueryDatasetCaseInsensitiveNamesExample|TestAccBigQueryDataset_storageBillModel|TestAccBigQueryDataset_regionalLocation|TestAccBigQueryDataset_access|TestAccBigQueryDataset_bigqueryDatasetAuthorizedDatasetExample|TestAccBigQueryDataset_withProvider5|TestAccBigQueryDataset_bigqueryDatasetBasicExample|TestAccBigQueryDataset_datasetWithContents|TestAccComputeAddress_networkTier_withLabels|TestAccComputeAddress_networkTier|TestAccComputeAddress_networkTier_withProvider5|TestAccBigQueryDataset_basic

Get to know how VCR tests work

modular-magician · 2023-08-02T19:11:59Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccBigQueryDataTable_bigtable[Debug log]
TestAccBigQueryDataset_withProvider5[Debug log]
TestAccComputeAddress_networkTier[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$
TestAccBigQueryDataset_withProvider5[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{Tests failed during RECORDING mode:}}$
TestAccDataSourceComputeAddresses[Error message] [Debug log]
TestAccBigQueryDataset_bigqueryDatasetDefaultCollationSetExample[Error message] [Debug log]
TestAccBigQueryDataset_bigqueryDatasetWithMaxTimeTravelHoursExample[Error message] [Debug log]
TestAccBigQueryDataset_bigqueryDatasetCaseInsensitiveNamesExample[Error message] [Debug log]
TestAccBigQueryDataset_storageBillModel[Error message] [Debug log]
TestAccBigQueryDataset_regionalLocation[Error message] [Debug log]
TestAccBigQueryDataset_access[Error message] [Debug log]
TestAccBigQueryDataset_bigqueryDatasetAuthorizedDatasetExample[Error message] [Debug log]
TestAccBigQueryDataset_bigqueryDatasetBasicExample[Error message] [Debug log]
TestAccBigQueryDataset_datasetWithContents[Error message] [Debug log]
TestAccComputeAddress_networkTier_withLabels[Error message] [Debug log]
TestAccComputeAddress_networkTier_withProvider5[Error message] [Debug log]
TestAccBigQueryDataset_basic[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-08-02T22:59:04Z

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

Field labels transitioned from optional+computed to optional google_bigquery_dataset - reference

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 override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 7 files changed, 204 insertions(+), 44 deletions(-))
Terraform Beta: Diff ( 7 files changed, 359 insertions(+), 49 deletions(-))
TF Conversion: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-08-03T04:22:51Z

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

Field labels transitioned from optional+computed to optional google_bigquery_dataset - reference

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 override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 9 files changed, 298 insertions(+), 48 deletions(-))
Terraform Beta: Diff ( 11 files changed, 596 insertions(+), 59 deletions(-))
TF Conversion: Diff ( 8 files changed, 253 insertions(+), 3 deletions(-))

modular-magician · 2023-08-03T06:07:54Z

Tests analytics

Total tests: 2925
Passed tests 2619
Skipped tests: 304
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

TestAccComputeAddress_networkTier_withLabels|TestAccBigQueryDataset_basic

Get to know how VCR tests work

modular-magician · 2023-08-03T06:11:58Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccComputeAddress_networkTier_withLabels[Debug log]
TestAccBigQueryDataset_basic[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

modular-magician · 2023-08-03T17:42:01Z

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

Field labels transitioned from optional+computed to optional google_bigquery_dataset - reference

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 override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 8 files changed, 286 insertions(+), 48 deletions(-))
Terraform Beta: Diff ( 11 files changed, 597 insertions(+), 59 deletions(-))
TF Conversion: Diff ( 8 files changed, 253 insertions(+), 3 deletions(-))

modular-magician · 2023-08-03T19:26:13Z

Tests analytics

Total tests: 2927
Passed tests 2623
Skipped tests: 304
Affected tests: 0

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

modular-magician · 2023-08-03T22:37:31Z

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

Field labels transitioned from optional+computed to optional google_bigquery_dataset - reference

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 override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 8 files changed, 286 insertions(+), 48 deletions(-))
Terraform Beta: Diff ( 11 files changed, 597 insertions(+), 59 deletions(-))
TF Conversion: Diff ( 8 files changed, 253 insertions(+), 3 deletions(-))

modular-magician · 2023-08-04T00:09:45Z

Tests analytics

Total tests: 2931
Passed tests 2627
Skipped tests: 304
Affected tests: 0

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

zli82016 · 2023-08-04T17:14:51Z

TestAccComputeAddress_networkTier_withProvider5 passed locally.

rileykarson

Made a first pass! Can we target this against the 5.0.0 branch now that that exists?

rileykarson · 2023-08-04T17:48:03Z

      organize and group your datasets
-    default_from_api: true
+
+      **Note**: This field does not list all labels present on the resource in GCP.


Here & others: Let's use the term "authoritative", since we use it in https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam (and all other IAM pages).

Something like: "This field is non-authoritative, and will only manage the labels present in your configuration. Please refer to the field effective_labels for all of the labels present on the resource.`

(effective_labels is output-only, so it's not entirely correct to call it authoritative)

zli82016 · 2023-08-04T22:26:46Z

Made a first pass! Can we target this against the 5.0.0 branch now that that exists?

Changed the base branch to 5.0.0

modular-magician · 2023-08-04T22:33:52Z

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

Field labels transitioned from optional+computed to optional google_bigquery_dataset - reference

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 override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 8 files changed, 204 insertions(+), 47 deletions(-))
Terraform Beta: Diff ( 12 files changed, 395 insertions(+), 58 deletions(-))
TF Conversion: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-08-04T22:44:08Z

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

Field labels transitioned from optional+computed to optional google_bigquery_dataset - reference

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 override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 8 files changed, 204 insertions(+), 47 deletions(-))
Terraform Beta: Diff ( 12 files changed, 395 insertions(+), 58 deletions(-))
TF Conversion: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-08-05T00:05:44Z

Tests analytics

Total tests: 2924
Passed tests 2619
Skipped tests: 305
Affected tests: 0

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

rileykarson

No concerns on my end, but not reviewing to approve as I'm about to go OOO. Adding @c2thorn as reviewer as I'll be out and they're the likely reviewer for subsequent changes

rileykarson · 2023-08-07T20:30:30Z

+    end
+
+    def build_effective_labels_field(name, min_version)
+      description = "All of #{name} (key/value pairs)\


Should this be consistent w/ handwritten fields?

Yes, it should. Will use this description for all of the effective_labels/effective_annotations field.

modular-magician · 2023-08-08T22:31:29Z

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

Field labels transitioned from optional+computed to optional google_bigquery_dataset - reference

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 override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 8 files changed, 204 insertions(+), 47 deletions(-))
Terraform Beta: Diff ( 12 files changed, 395 insertions(+), 58 deletions(-))
TF Conversion: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-08-08T22:54:44Z

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

Breaking Change(s) Detected

The following breaking change(s) were detected within your pull request.

Field labels transitioned from optional+computed to optional google_bigquery_dataset - reference

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 override-breaking-change label can be added to allow merging.

Diff report

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

Terraform GA: Diff ( 8 files changed, 204 insertions(+), 47 deletions(-))
Terraform Beta: Diff ( 12 files changed, 395 insertions(+), 58 deletions(-))
TF Conversion: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2023-08-09T00:15:10Z

Tests analytics

Total tests: 2924
Passed tests 2619
Skipped tests: 305
Affected tests: 0

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

c2thorn · 2023-08-09T15:35:33Z

Seems good on initial review. Can we get entries on the main branch for the 5.0.0 upgrade guide explaining any breaking changes and manual steps user's will need to take as they upgrade?

zli82016 · 2023-08-10T17:26:57Z

Seems good on initial review. Can we get entries on the main branch for the 5.0.0 upgrade guide explaining any breaking changes and manual steps user's will need to take as they upgrade?

Thanks for the reminder. Created the PR-8608 to add the first version of guide for labels. Will add more details to the guide with more PRs being pushed to the 5.0.0 branch.

* Add new type KeyValueLabels * Use KeyValueLabels in resource google_compute_address * func access_path in type * Add new type KeyValueAnnotations * Add new type KeyValueAnnotations * Fix the syntax error * Modify descriptions for labels field * Fix tests * Only read labels fingerprint when set labels * Remove version check * Fix tgc * Refactor code * Remove logger * New function properities_with_excluded * Address comments * Revert the TGC changes * Fix rake syntax errors * Fix the bug to set labels in the state * Type transform

zli82016 added 5 commits July 28, 2023 11:54

Add new type KeyValueLabels

7cc033e

Use KeyValueLabels in resource google_compute_address

abe748e

func access_path in type

0cfe4ba

Add new type KeyValueAnnotations

a8609ad

Add new type KeyValueAnnotations

26b5348

zli82016 added 2 commits August 2, 2023 10:30

Fix the syntax error

c13d3d3

Modify descriptions for labels field

54e29fa

Fix tests

3ca790f

zli82016 added 2 commits August 2, 2023 16:28

Only read labels fingerprint when set labels

7c71222

Remove version check

93b363c

zli82016 changed the title ~~Add new types KeyValueLabels and KeyValueAnnotations~~ [Will not merge]Add new types KeyValueLabels and KeyValueAnnotations Aug 2, 2023

Fix tgc

5b36608

zli82016 added 2 commits August 3, 2023 10:24

Refactor code

5af38a0

Remove logger

da2ea41

New function properities_with_excluded

e677fe6

zli82016 requested a review from rileykarson August 4, 2023 17:15

rileykarson reviewed Aug 4, 2023

View reviewed changes

zli82016 added 2 commits August 4, 2023 15:18

Address comments

c2f53c3

Revert the TGC changes

68be4ea

zli82016 changed the base branch from main to FEATURE-BRANCH-major-release-5.0.0 August 4, 2023 22:26

Fix rake syntax errors

7b7d888

zli82016 changed the title ~~[Will not merge]Add new types KeyValueLabels and KeyValueAnnotations~~ Add new types KeyValueLabels and KeyValueAnnotations Aug 7, 2023

zli82016 requested a review from rileykarson August 7, 2023 16:48

rileykarson reviewed Aug 7, 2023

View reviewed changes

rileykarson requested a review from c2thorn August 7, 2023 20:31

Fix the bug to set labels in the state

b2c6afe

Type transform

baf7d0e

c2thorn approved these changes Aug 15, 2023

View reviewed changes

zli82016 merged commit f390fc9 into FEATURE-BRANCH-major-release-5.0.0 Aug 15, 2023

zli82016 deleted the labesl-mmv1 branch August 15, 2023 19:45

Uh oh!

Conversation

zli82016 commented Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

modular-magician commented Aug 2, 2023

Breaking Change(s) Detected

Diff report

Uh oh!

modular-magician commented Aug 2, 2023

Breaking Change(s) Detected

Diff report

Uh oh!

modular-magician commented Aug 2, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Aug 2, 2023

Rerun these tests in REPLAYING mode to catch issues

Uh oh!

modular-magician commented Aug 2, 2023

Breaking Change(s) Detected

Diff report

Uh oh!

modular-magician commented Aug 3, 2023

Breaking Change(s) Detected

Diff report

Uh oh!

modular-magician commented Aug 3, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Aug 3, 2023

Rerun these tests in REPLAYING mode to catch issues

Uh oh!

modular-magician commented Aug 3, 2023

Breaking Change(s) Detected

Diff report

Uh oh!

modular-magician commented Aug 3, 2023

Tests analytics

Uh oh!

modular-magician commented Aug 3, 2023

Breaking Change(s) Detected

Diff report

Uh oh!

modular-magician commented Aug 4, 2023

Tests analytics

Uh oh!

zli82016 commented Aug 4, 2023

Uh oh!

rileykarson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rileykarson Aug 4, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zli82016 commented Aug 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

modular-magician commented Aug 4, 2023

Breaking Change(s) Detected

Diff report

Uh oh!

modular-magician commented Aug 4, 2023

Breaking Change(s) Detected

Diff report

Uh oh!

modular-magician commented Aug 5, 2023

Tests analytics

Uh oh!

zli82016 commented Aug 2, 2023 •

edited

Loading

zli82016 commented Aug 4, 2023 •

edited

Loading

rileykarson left a comment •

edited

Loading

zli82016 commented Aug 10, 2023 •

edited

Loading