Support location in google tags tag binding #6716

AarshDhokai · 2022-10-19T05:46:02Z

Fixing the issue : Add location attribute to google_tags_tag_binding

Proposing New Resource Because of this issue : Support location being in the domain of a resource's URLs for only a subset of resources in an MMv1 service

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).
Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
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).
Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

`google_tags_location_tag_bindings`

modular-magician · 2022-10-19T05:46:57Z

Hello! I am a robot who works on Magic Modules PRs.

I've detected that you're a community contributor. @megan07, a repository maintainer, has been assigned to assist you and help review your changes.

❓ First time contributing? Click here for more details

Your assigned reviewer will help review your code by:

Ensuring it's backwards compatible, covers common error cases, etc.
Summarizing the change into a user-facing changelog note.
Passes tests, either our "VCR" suite, a set of presubmit tests, or with manual test runs.

You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails.

If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox.

modular-magician · 2022-10-19T06:12: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 ( 6 files changed, 384 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 6 files changed, 384 insertions(+), 1 deletion(-))
TF Validator: Diff ( 4 files changed, 17 insertions(+), 3 deletions(-))

modular-magician · 2022-10-19T06:53:35Z

Tests analytics

Total tests: 2196
Passed tests 1955
Skipped tests: 239
Failed tests: 2

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccTags|TestAccFirebaserulesRelease_BasicRelease

modular-magician · 2022-10-19T07:05:49Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]

Tests failed during RECORDING mode:
TestAccTags[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

modular-magician · 2022-10-19T09:12:46Z

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 ( 6 files changed, 390 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 6 files changed, 390 insertions(+), 1 deletion(-))
TF Validator: Diff ( 4 files changed, 17 insertions(+), 3 deletions(-))

modular-magician · 2022-10-19T09:49:38Z

Tests analytics

Total tests: 2196
Passed tests 1956
Skipped tests: 239
Failed tests: 1

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccTags

modular-magician · 2022-10-19T09:59:45Z

Tests failed during RECORDING mode:
TestAccTags[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

modular-magician · 2022-10-19T10:23:30Z

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 ( 6 files changed, 396 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 6 files changed, 396 insertions(+), 1 deletion(-))
TF Validator: Diff ( 4 files changed, 17 insertions(+), 3 deletions(-))

modular-magician · 2022-10-19T11:09:07Z

Tests analytics

Total tests: 2196
Passed tests 1956
Skipped tests: 239
Failed tests: 1

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccTags

modular-magician · 2022-10-19T11:19:40Z

Tests failed during RECORDING mode:
TestAccTags[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

modular-magician · 2022-10-19T11:50: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 ( 6 files changed, 399 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 6 files changed, 399 insertions(+), 1 deletion(-))
TF Validator: Diff ( 4 files changed, 17 insertions(+), 3 deletions(-))

modular-magician · 2022-10-19T12:23:50Z

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 ( 6 files changed, 399 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 6 files changed, 399 insertions(+), 1 deletion(-))
TF Validator: Diff ( 4 files changed, 17 insertions(+), 3 deletions(-))

modular-magician · 2022-10-19T13:01:16Z

Tests analytics

Total tests: 2196
Passed tests 1955
Skipped tests: 239
Failed tests: 2

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccTags|TestAccFirebaserulesRelease_BasicRelease

modular-magician · 2022-10-19T13:31:02Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]

Tests failed during RECORDING mode:
TestAccTags[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

modular-magician · 2022-10-21T10:00:47Z

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 ( 6 files changed, 401 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 6 files changed, 401 insertions(+), 1 deletion(-))
TF Validator: Diff ( 4 files changed, 17 insertions(+), 3 deletions(-))

modular-magician · 2022-10-21T10:38:10Z

Tests analytics

Total tests: 2200
Passed tests 1959
Skipped tests: 239
Failed tests: 2

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccTags|TestAccFirebaserulesRelease_BasicRelease

modular-magician · 2022-10-21T11:07:18Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]

Tests failed during RECORDING mode:
TestAccTags[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

modular-magician · 2022-10-31T07:01:05Z

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 ( 6 files changed, 431 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 6 files changed, 431 insertions(+), 1 deletion(-))
TF Validator: Diff ( 4 files changed, 17 insertions(+), 3 deletions(-))

modular-magician · 2022-10-31T07:38:16Z

Tests analytics

Total tests: 2210
Passed tests 1970
Skipped tests: 239
Failed tests: 1

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccTags

modular-magician · 2022-11-24T15:05:15Z

Tests analytics

Total tests: 2281
Passed tests 2034
Skipped tests: 244
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccTags|TestAccLoggingBucketConfigProject_cmekSettings|TestAccFirebaserulesRelease_BasicRelease

modular-magician · 2022-11-24T15:17:30Z

Tests passed during RECORDING mode:
TestAccTags[Debug log]
TestAccLoggingBucketConfigProject_cmekSettings[Debug log]
TestAccFirebaserulesRelease_BasicRelease[Debug log]

All tests passed
View the build log or the debug log for each test

slevenick · 2022-11-24T16:58:46Z

mmv1/third_party/terraform/resources/resource_tags_location_tag_bindings.go.erb

+		return err
+	}
+
+	url, err := replaceVars(d, config, "{{TagsLocationBasePath}}tagBindings/?parent={{parent}}&pageSize=300")


Still relevant

mmv1/third_party/terraform/resources/resource_tags_location_tag_bindings.go.erb

slevenick · 2022-11-24T17:03:03Z

mmv1/third_party/terraform/resources/resource_tags_location_tag_bindings.go.erb

+		return nil, err
+	}
+
+	parent := d.Get("name").(string)


Parent == name once the URL-encoded values are replaced with slashes? Why wouldn't this be name?

Why do we have to handle escaped values anyways? Seems like we should remove them before storing the ID so users have actual full values rather than partially escaped strings in their state

slevenick · 2022-11-24T17:03:23Z

mmv1/third_party/terraform/resources/resource_tags_location_tag_bindings.go.erb

+
+func resourceTagsLocationTagBindingImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
+	config := meta.(*Config)
+	if err := parseImportId([]string{"(?P<location>[^/]+)/tagBindings/(?P<name>[^/]+)/tagValues/(?P<tag_value>[^/]+)"}, d, config); err != nil {


Seems like <name> here is actually <parent>

slevenick · 2022-11-24T17:05:58Z

mmv1/third_party/terraform/website/docs/r/google_tags_location_tag_binding.html.markdown

+
+In addition to the arguments listed above, the following computed attributes are exported:
+
+* `id` - an identifier for the resource with format `tagBindings/{{name}}`


According to this the format of id would be:
tagBindings/tagBindings/{full-resource-name}/{tag-value-name}

as name == tagBindings/{full-resource-name}/{tag-value-name}

slevenick · 2022-11-24T17:06:30Z

mmv1/third_party/terraform/website/docs/r/google_tags_location_tag_binding.html.markdown

+* `id` - an identifier for the resource with format `tagBindings/{{name}}`
+
+* `name` -
+  The generated id for the TagBinding. This is a string of the form: `tagBindings/{full-resource-name}/{tag-value-name}`


Should this be tagBindings/{full-resource-name}/tagValues/{tag-value-name} ?

modular-magician · 2022-11-25T05:42:35Z

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 ( 7 files changed, 556 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 7 files changed, 556 insertions(+), 1 deletion(-))
TF Validator: Diff ( 4 files changed, 17 insertions(+), 3 deletions(-))

modular-magician · 2022-11-25T06:34:12Z

Tests analytics

Total tests: 2281
Passed tests 2036
Skipped tests: 244
Failed tests: 1

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccLoggingBucketConfigProject_cmekSettings

modular-magician · 2022-11-25T06:35:23Z

Tests passed during RECORDING mode:
TestAccLoggingBucketConfigProject_cmekSettings[Debug log]

All tests passed
View the build log or the debug log for each test

modular-magician · 2022-11-29T06:53:45Z

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 ( 7 files changed, 557 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 7 files changed, 557 insertions(+), 1 deletion(-))
TF Validator: Diff ( 4 files changed, 17 insertions(+), 3 deletions(-))

modular-magician · 2022-11-29T07:39:37Z

Tests analytics

Total tests: 2302
Passed tests 2047
Skipped tests: 244
Failed tests: 11

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccLoggingBucketConfigProject_cmekSettings|TestAccDataFusionInstance_dataFusionInstanceZoneExample|TestAccDataFusionInstance_dataFusionInstanceEventExample|TestAccDataFusionInstance_dataFusionInstanceFullExample|TestAccComputeForwardingRule_update|TestAccVertexAIIndex_updated|TestAccVertexAIIndex_vertexAiIndexStreamingExample|TestAccVertexAIIndex_vertexAiIndexExample|TestAccDataFusionInstanceIamPolicyGenerated|TestAccDataFusionInstanceIamMemberGenerated|TestAccDataFusionInstanceIamBindingGenerated

modular-magician · 2022-11-29T08:31:39Z

Tests passed during RECORDING mode:
TestAccLoggingBucketConfigProject_cmekSettings[Debug log]
TestAccDataFusionInstance_dataFusionInstanceZoneExample[Debug log]
TestAccDataFusionInstance_dataFusionInstanceEventExample[Debug log]
TestAccDataFusionInstance_dataFusionInstanceFullExample[Debug log]
TestAccComputeForwardingRule_update[Debug log]
TestAccVertexAIIndex_updated[Debug log]
TestAccVertexAIIndex_vertexAiIndexStreamingExample[Debug log]
TestAccDataFusionInstanceIamPolicyGenerated[Debug log]
TestAccDataFusionInstanceIamMemberGenerated[Debug log]
TestAccDataFusionInstanceIamBindingGenerated[Debug log]

Tests failed during RECORDING mode:
TestAccVertexAIIndex_vertexAiIndexExample[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

modular-magician · 2022-11-30T06:45: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 ( 7 files changed, 558 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 7 files changed, 558 insertions(+), 1 deletion(-))
TF Validator: Diff ( 4 files changed, 17 insertions(+), 3 deletions(-))

modular-magician · 2022-11-30T07:36:04Z

Tests analytics

Total tests: 2302
Passed tests 2054
Skipped tests: 244
Failed tests: 4

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccCloudFunctions2Function_fullUpdate|TestAccLoggingBucketConfigProject_cmekSettings

modular-magician · 2022-11-30T07:40:10Z

Tests passed during RECORDING mode:
TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample[Debug log]
TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample[Debug log]
TestAccCloudFunctions2Function_fullUpdate[Debug log]
TestAccLoggingBucketConfigProject_cmekSettings[Debug log]

All tests passed
View the build log or the debug log for each test

AarshDhokai · 2022-12-20T05:57:33Z

Could you please review the changes @slevenick ?

nphilbrook · 2023-01-17T16:54:58Z

Good morning, I just want to chime in again that we are anxiously awaiting this capability and would love to see it merged soon.

AarshDhokai added 2 commits September 22, 2022 19:28

working on tags issue

4644966

added handwritten resource for location_tags

04d2ccb

modular-magician requested a review from megan07 October 19, 2022 05:46

AarshDhokai added 2 commits October 19, 2022 05:47

reverting_ruby_version

68ceb18

fixing merge conflicts in tags_test file

8b36f30

fixing_the_test_of_location_tag_binding

f307a00

fixing_the_test_og_tags_location_tag_binding

f70baa2

fixing the test case of location_tag_binding

db8a231

fixing the test case of location tag binding

2ce060a

fix_the_test_case_of_location_tag_binding

63ac886

fixing_the_test_and_added_different_destroy_producer

5031440

slevenick reviewed Nov 24, 2022

View reviewed changes

addressing the review comments

9334524

modular-magician requested a review from slevenick November 25, 2022 05:28

addressing the review comments

1f4a2a8

commiting to run pipeline

8cf6169

slevenick approved these changes Jan 19, 2023

View reviewed changes

slevenick merged commit a69f115 into GoogleCloudPlatform:main Jan 19, 2023

rileykarson mentioned this pull request Feb 8, 2023

Fix sidebar for google_tags_location_tag_binding #7256

Merged

5 tasks

This was referenced Feb 8, 2023

Fix sidebar for google_tags_location_tag_binding hashicorp/terraform-provider-google-beta#5185

Merged

Fix sidebar for google_tags_location_tag_binding hashicorp/terraform-provider-google#13683

Merged

rileykarson mentioned this pull request Sep 28, 2023

Update LocationTagBinding docs #9128

Merged

This was referenced Oct 2, 2023

Update LocationTagBinding docs hashicorp/terraform-provider-google-beta#6425

Merged

Update LocationTagBinding docs hashicorp/terraform-provider-google#16086

Merged

Update LocationTagBinding docs GoogleCloudPlatform/terraform-google-conversion#1537

Merged


		In addition to the arguments listed above, the following computed attributes are exported:

		* `id` - an identifier for the resource with format `tagBindings/{{name}}`

Support location in google tags tag binding #6716

Support location in google tags tag binding #6716

Conversation

AarshDhokai commented Oct 19, 2022 • edited Loading

modular-magician commented Oct 19, 2022

modular-magician commented Oct 19, 2022

Diff report

modular-magician commented Oct 19, 2022

Tests analytics

Action taken

modular-magician commented Oct 19, 2022

modular-magician commented Oct 19, 2022

Diff report

modular-magician commented Oct 19, 2022

Tests analytics

Action taken

modular-magician commented Oct 19, 2022

modular-magician commented Oct 19, 2022

Diff report

modular-magician commented Oct 19, 2022

Tests analytics

Action taken

modular-magician commented Oct 19, 2022

modular-magician commented Oct 19, 2022

Diff report

modular-magician commented Oct 19, 2022

Diff report

modular-magician commented Oct 19, 2022

Tests analytics

Action taken

modular-magician commented Oct 19, 2022

modular-magician commented Oct 21, 2022

Diff report

modular-magician commented Oct 21, 2022

Tests analytics

Action taken

modular-magician commented Oct 21, 2022

modular-magician commented Oct 31, 2022

Diff report

modular-magician commented Oct 31, 2022

Tests analytics

Action taken

modular-magician commented Nov 24, 2022

Tests analytics

Action taken

modular-magician commented Nov 24, 2022

slevenick Nov 24, 2022

Choose a reason for hiding this comment

slevenick Nov 24, 2022

Choose a reason for hiding this comment

slevenick Nov 24, 2022

Choose a reason for hiding this comment

slevenick Nov 24, 2022

Choose a reason for hiding this comment

slevenick Nov 24, 2022

Choose a reason for hiding this comment

slevenick Nov 24, 2022

Choose a reason for hiding this comment

modular-magician commented Nov 25, 2022

Diff report

modular-magician commented Nov 25, 2022

Tests analytics

Action taken

modular-magician commented Nov 25, 2022

modular-magician commented Nov 29, 2022

Diff report

modular-magician commented Nov 29, 2022

Tests analytics

Action taken

modular-magician commented Nov 29, 2022

modular-magician commented Nov 30, 2022

Diff report

modular-magician commented Nov 30, 2022

Tests analytics

Action taken

modular-magician commented Nov 30, 2022

AarshDhokai commented Dec 20, 2022

nphilbrook commented Jan 17, 2023

AarshDhokai commented Oct 19, 2022 •

edited

Loading