Skip to content
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

Apply new labels model to more resources (part 1) #8950

Merged

Conversation

zli82016
Copy link
Member

@zli82016 zli82016 commented Sep 13, 2023

  1. Handle the case that the labels field is inside a flatten object (google_bigquery_job)
  2. Make effective_labels immutable if labels filed is immutable
  3. Apply the labels model to more MMV1 resources

Release Note Template for Downstream PRs (will be copied)

provider: made the `labels` field be non-authoritative and only manage the labels defined by the users on the resource through Terraform in all of the resources with standard `labels` field.
provider: added the output-only `terraform_labels` field to all of the resources with standard `labels` field. This field merges the labels defined by the users on the resource through Terraform and the default labels configured on the provider.
provider: added the output-only `effective_labels` to all of the resources with standard `labels` field. This field lists all of labels present on the resource in GCP, including the labels configured through Terraform, the system, and other clients.

@zli82016 zli82016 force-pushed the labels-mmv1-more-resources-1 branch from 64e959a to 299c7f6 Compare September 13, 2023 22:02
@modular-magician
Copy link
Collaborator

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

Breaking Change Detection Failed

The breaking change detector crashed during execution. This is usually due to the downstream provider(s) failing to compile. Please investigate or follow up with your reviewer.

Diff report

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

Terraform GA: Diff ( 73 files changed, 1972 insertions(+), 561 deletions(-))
Terraform Beta: Diff ( 80 files changed, 2242 insertions(+), 631 deletions(-))
TF Conversion: Diff ( 28 files changed, 402 insertions(+), 388 deletions(-))

@zli82016 zli82016 force-pushed the labels-mmv1-more-resources-1 branch 2 times, most recently from 0f3f10f to 9bf01e6 Compare September 13, 2023 22:57
@modular-magician
Copy link
Collaborator

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

Breaking Change Detection Failed

The breaking change detector crashed during execution. This is usually due to the downstream provider(s) failing to compile. Please investigate or follow up with your reviewer.

Diff report

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

Terraform GA: Diff ( 92 files changed, 2125 insertions(+), 635 deletions(-))
Terraform Beta: Diff ( 99 files changed, 2395 insertions(+), 705 deletions(-))
TF Conversion: Diff ( 29 files changed, 419 insertions(+), 405 deletions(-))
TF OiCS: Diff ( 1 file changed, 3 insertions(+))

@zli82016 zli82016 force-pushed the labels-mmv1-more-resources-1 branch from 9bf01e6 to a9eb91c Compare September 13, 2023 23:06
@modular-magician
Copy link
Collaborator

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

Breaking Change Detection Failed

The breaking change detector crashed during execution. This is usually due to the downstream provider(s) failing to compile. Please investigate or follow up with your reviewer.

Diff report

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

Terraform GA: Diff ( 93 files changed, 2135 insertions(+), 635 deletions(-))
Terraform Beta: Diff ( 100 files changed, 2405 insertions(+), 705 deletions(-))
TF Conversion: Diff ( 29 files changed, 419 insertions(+), 405 deletions(-))
TF OiCS: Diff ( 1 file changed, 3 insertions(+))

@zli82016 zli82016 force-pushed the labels-mmv1-more-resources-1 branch from a9eb91c to 2ccb8fc Compare September 14, 2023 03:56
@modular-magician
Copy link
Collaborator

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_certificate_manager_certificate_map_entry - reference
  • Field labels transitioned from optional+computed to optional google_certificate_manager_certificate_map - 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 ( 90 files changed, 2052 insertions(+), 622 deletions(-))
Terraform Beta: Diff ( 97 files changed, 2322 insertions(+), 692 deletions(-))
TF Conversion: Diff ( 28 files changed, 394 insertions(+), 394 deletions(-))
TF OiCS: Diff ( 1 file changed, 3 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3009
Passed tests 2696
Skipped tests: 303
Affected tests: 10

Action taken

Found 10 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccAccessContextManager|TestAccDataSourceGoogleArtifactRegistryRepositoryConfig|TestAccBigQueryDataTable_bigtable|TestAccBigtableAppProfile_bigtableAppProfileMulticlusterExample|TestAccBigtableAppProfile_bigtableAppProfileAnyclusterExample|TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample|TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateDnsExample|TestAccCloudFunctions2Function_update|TestAccDataSourceGoogleCloudFunctions2Function_basic|TestAccCloudIdentityGroup

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

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_certificate_manager_certificate_map_entry - reference
  • Field labels transitioned from optional+computed to optional google_certificate_manager_certificate_map - 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 ( 93 files changed, 2129 insertions(+), 642 deletions(-))
Terraform Beta: Diff ( 100 files changed, 2399 insertions(+), 712 deletions(-))
TF Conversion: Diff ( 29 files changed, 412 insertions(+), 412 deletions(-))
TF OiCS: Diff ( 1 file changed, 3 insertions(+))

@zli82016 zli82016 force-pushed the labels-mmv1-more-resources-1 branch from 9611318 to da6c57d Compare September 14, 2023 05:30
@modular-magician
Copy link
Collaborator

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_certificate_manager_certificate_map_entry - reference
  • Field labels transitioned from optional+computed to optional google_certificate_manager_certificate_map - 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 ( 93 files changed, 2134 insertions(+), 642 deletions(-))
Terraform Beta: Diff ( 100 files changed, 2404 insertions(+), 712 deletions(-))
TF Conversion: Diff ( 29 files changed, 412 insertions(+), 412 deletions(-))
TF OiCS: Diff ( 1 file changed, 3 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3009
Passed tests 2690
Skipped tests: 303
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
TestAccAccessContextManager|TestAccDataSourceGoogleArtifactRegistryRepositoryConfig|TestAccBigQueryDataTable_bigtable|TestAccBigQueryJob_bigqueryJobQueryExample|TestAccBigQueryJob_bigqueryJobLoadTableReferenceExample|TestAccBigQueryJob_bigqueryJobLoadParquetExample|TestAccBigQueryJob_bigqueryJobLoadExample|TestAccBigQueryJob_bigqueryJobLoadGeojsonExample|TestAccBigQueryJob_bigqueryJobQueryTableReferenceExample|TestAccBigtableAppProfile_bigtableAppProfileMulticlusterExample|TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample|TestAccBigtableAppProfile_bigtableAppProfileAnyclusterExample|TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateDnsExample|TestAccCloudFunctions2Function_update|TestAccDataSourceGoogleCloudFunctions2Function_basic|TestAccCloudIdentityGroup

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateDnsExample[Debug log]
TestAccCloudFunctions2Function_update[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:}}$
TestAccAccessContextManager[Error message] [Debug log]
TestAccDataSourceGoogleArtifactRegistryRepositoryConfig[Error message] [Debug log]
TestAccBigQueryDataTable_bigtable[Error message] [Debug log]
TestAccBigQueryJob_bigqueryJobQueryExample[Error message] [Debug log]
TestAccBigQueryJob_bigqueryJobLoadTableReferenceExample[Error message] [Debug log]
TestAccBigQueryJob_bigqueryJobLoadParquetExample[Error message] [Debug log]
TestAccBigQueryJob_bigqueryJobLoadExample[Error message] [Debug log]
TestAccBigQueryJob_bigqueryJobLoadGeojsonExample[Error message] [Debug log]
TestAccBigQueryJob_bigqueryJobQueryTableReferenceExample[Error message] [Debug log]
TestAccBigtableAppProfile_bigtableAppProfileMulticlusterExample[Error message] [Debug log]
TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample[Error message] [Debug log]
TestAccBigtableAppProfile_bigtableAppProfileAnyclusterExample[Error message] [Debug log]
TestAccDataSourceGoogleCloudFunctions2Function_basic[Error message] [Debug log]
TestAccCloudIdentityGroup[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

@zli82016 zli82016 force-pushed the labels-mmv1-more-resources-1 branch 3 times, most recently from bc3006d to c86a857 Compare September 14, 2023 18:09
@modular-magician
Copy link
Collaborator

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_certificate_manager_certificate_map_entry - reference
  • Field labels transitioned from optional+computed to optional google_certificate_manager_certificate_map - 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 ( 101 files changed, 2164 insertions(+), 672 deletions(-))
Terraform Beta: Diff ( 108 files changed, 2448 insertions(+), 756 deletions(-))
TF Conversion: Diff ( 29 files changed, 412 insertions(+), 412 deletions(-))
TF OiCS: Diff ( 1 file changed, 3 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3009
Passed tests 2700
Skipped tests: 303
Affected tests: 6

Action taken

Found 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccAccessContextManager|TestAccDataSourceGoogleArtifactRegistryRepositoryConfig|TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample|TestAccDataSourceGoogleCloudFunctions2Function_basic|TestAccCloudIdentityGroup|TestAccDataSourceGoogleServiceAccountIdToken_impersonation

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample[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:}}$
TestAccAccessContextManager[Error message] [Debug log]
TestAccDataSourceGoogleArtifactRegistryRepositoryConfig[Error message] [Debug log]
TestAccDataSourceGoogleCloudFunctions2Function_basic[Error message] [Debug log]
TestAccCloudIdentityGroup[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

@zli82016 zli82016 force-pushed the labels-mmv1-more-resources-1 branch from f08d192 to 2fb2868 Compare September 14, 2023 23:08
@modular-magician
Copy link
Collaborator

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_certificate_manager_certificate_map_entry - reference
  • Field labels transitioned from optional+computed to optional google_certificate_manager_certificate_map - 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 ( 100 files changed, 2108 insertions(+), 669 deletions(-))
Terraform Beta: Diff ( 107 files changed, 2392 insertions(+), 753 deletions(-))
TF Conversion: Diff ( 28 files changed, 409 insertions(+), 409 deletions(-))
TF OiCS: Diff ( 1 file changed, 3 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3036
Passed tests 2730
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
TestAccDataSourceGoogleArtifactRegistryRepositoryConfig|TestAccDataSourceGoogleCloudFunctions2Function_basic

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

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

@zli82016 zli82016 force-pushed the labels-mmv1-more-resources-1 branch from 2fb2868 to 65dc3ab Compare September 15, 2023 16:41
@modular-magician
Copy link
Collaborator

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_certificate_manager_certificate_map_entry - reference
  • Field labels transitioned from optional+computed to optional google_certificate_manager_certificate_map - 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 ( 101 files changed, 2115 insertions(+), 676 deletions(-))
Terraform Beta: Diff ( 116 files changed, 2421 insertions(+), 782 deletions(-))
TF Conversion: Diff ( 28 files changed, 409 insertions(+), 409 deletions(-))
TF OiCS: Diff ( 1 file changed, 3 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3036
Passed tests 2731
Skipped tests: 304
Affected tests: 1

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
TestAccGkeonpremBareMetalCluster_bareMetalClusterUpdateBasic

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccGkeonpremBareMetalCluster_bareMetalClusterUpdateBasic[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

@zli82016 zli82016 changed the title Apply new labels model to more resources Apply new labels model to more resources (part 1) Sep 15, 2023
@@ -24,9 +24,11 @@ func TestAccDataSourceGoogleCloudFunctions2Function_basic(t *testing.T) {
{
Config: testAccDataSourceGoogleCloudFunctions2FunctionConfig(functionName,
bucketName, zipFilePath),
// As the value of "labels" and "terraform_lables" in the state is dpendent on the configuration,
// and these fields are not set in the configuration of the data source, so these fields are empty in the state of the data source.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's a good point that I didn't consider. Time-permitting we should try to correct that- we could copy the value of effective_labels on top of labels in the datasource's Read function post-resource Read. We could always fix that in a minor or patch release, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the idea. I will test and handle all of the relevant data sources later.

mmv1/api/resource.rb Show resolved Hide resolved
@modular-magician
Copy link
Collaborator

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_certificate_manager_certificate_map_entry - reference
  • Field labels transitioned from optional+computed to optional google_certificate_manager_certificate_map - 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 ( 101 files changed, 2115 insertions(+), 676 deletions(-))
Terraform Beta: Diff ( 116 files changed, 2421 insertions(+), 782 deletions(-))
TF Conversion: Diff ( 28 files changed, 409 insertions(+), 409 deletions(-))
TF OiCS: Diff ( 1 file changed, 3 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3035
Passed tests 2729
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
TestAccDataSourceGoogleServiceAccountAccessToken_basic|TestAccDataSourceGoogleServiceAccountIdToken_impersonation

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\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{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$
TestAccDataSourceGoogleServiceAccountAccessToken_basic[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

@zli82016
Copy link
Member Author

The failed test is unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants