Skip to content

tags: detect out-of-band deletion of google_tags_location_tag_binding (#24408)#77

Open
jbbqqf wants to merge 11 commits into
mainfrom
feat/24408-tags-location-binding-404
Open

tags: detect out-of-band deletion of google_tags_location_tag_binding (#24408)#77
jbbqqf wants to merge 11 commits into
mainfrom
feat/24408-tags-location-binding-404

Conversation

@jbbqqf
Copy link
Copy Markdown
Owner

@jbbqqf jbbqqf commented May 9, 2026

Summary

google_tags_location_tag_binding.Read did not detect that a tag binding
had been deleted out-of-band (gcloud / console / another tool). When the
listing under the parent returned no matching binding, the function
returned nil without clearing d.SetId(""), so Terraform kept the
resource in state and never offered to recreate it on the next plan.

Fixes hashicorp/terraform-provider-google#24408 — see hashicorp/terraform-provider-google#24408

Why

resourceTagsLocationTagBindingRead (in
mmv1/third_party/terraform/services/tags/resource_tags_location_tag_binding.go.tmpl)
hits the regional tagBindings.list?parent=...&pageSize=300 endpoint
and then walks the page to find the entry whose name matches the
resource's stored ID. Three paths in that walk currently mishandle the
"binding not found" outcome:

  1. No tagBindings key in the response (parent has zero bindings)
    — the code returned nil directly, leaving Terraform's state intact.
  2. Pagination break — when a subsequent page returned no
    tagBindings, the existing code did return nil, abandoning any
    bindings already accumulated from prior pages.
  3. flattenNestedTagsLocationTagBinding returns nil (the matching
    item was not found in any page) — the caller called d.Set("name", ...)
    on the nil map and continued, never clearing the ID.

In all three cases the resource quietly stayed in state. A subsequent
terraform plan then either succeeded with no diff (silently masking
the deletion) or failed later with a confusing "tag binding not found"
on update or destroy.

The sibling resource google_tags_tag_binding already handles this
correctly:

v, ok := res["tagBindings"]
if !ok || v == nil {
    log.Printf("[DEBUG] Removing TagsTagBinding because it no longer exists, ...")
    d.SetId("")
    return nil
}
...
if foundItem == nil {
    log.Printf("[DEBUG] Removing TagsTagBinding because it no longer exists.")
    d.SetId("")
    return nil
}

This change ports the same pattern to the location-aware variant.

GCP API reference:

What changed

Single hand-written template:

mmv1/third_party/terraform/services/tags/resource_tags_location_tag_binding.go.tmpl | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

Three behavior changes inside Read:

  1. When res["tagBindings"] is missing/nil, log + d.SetId("") + return.
  2. When a subsequent page returns no tagBindings, break out of the
    pagination loop instead of dropping the partial result.
  3. After flattenNestedTagsLocationTagBinding, if the result is nil,
    log + d.SetId("") + return, before any d.Set(...) call.

The fix matches the structure of resource_tags_tag_binding.go so the
two resources behave consistently from a state-drift perspective.

Edge cases tested

# Scenario Behavior expected Verified by
1 Parent has zero tag bindings d.SetId(""), log message, return; next plan proposes recreation Code review against sibling resource pattern + regenerated .go file inspection
2 Parent has bindings but the one in state was deleted flattenNestedTagsLocationTagBinding returns nil, the new if res == nil branch fires, ID cleared Regenerated .go file shows the new branch at Read line ~266
3 Parent has > 300 bindings (pagination); page N has results, page N+1 returns empty tagBindings Bindings from page 1..N are kept; loop exits cleanly via break; subsequent search either finds the binding (state preserved) or doesn't (case 2 fires) Code review — break replaces the early return nil that previously discarded pView

Test protocol

Test Result Notes
make build OUTPUT_PATH=... VERSION=ga (mmv1 regen) OK
go build ./google/services/tags/... on regenerated TPG OK service compiles
go build ./... on full regenerated TPG OK full provider compiles
Live BEFORE/AFTER smoke not run The drift-detection paths require an out-of-band gcloud resource-manager tags bindings delete between two plans on the same project resource. The test is a faithful port of the working pattern in resource_tags_tag_binding.go; live verification is best done as part of the existing TestAccTagsLocationTagBinding_* acceptance suite, which already exercises real bindings against the API.

I considered adding a dedicated drift acceptance test
(TestAccTagsLocationTagBinding_outOfBandDelete) but decided against
it in this PR to keep the diff minimal and reviewer-friendly. The
maintainers may want to add one in the existing
resource_tags_test.go (the file is shared by both binding variants
in mmv1/third_party/terraform/services/tags/).

Resources

Disclosure

This PR was drafted with assistance from Claude Code as part of a focused
contribution batch on hand-written TPG resources. The fix was reviewed
manually against the sibling google_tags_tag_binding resource pattern.
The mmv1 generation was run locally and the regenerated provider
compiled clean.

The author (a human) reviewed the diff and the regenerated .go file
before opening this PR.

jcromanu and others added 11 commits May 8, 2026 16:43
… (#24408)

resourceTagsLocationTagBindingRead listed bindings under the parent and
searched for the one matching the resource's name. When the binding had
been deleted outside of Terraform (e.g. via gcloud, the console, or
another tool), the listing returned without the matching item. The
existing code returned nil in that path without clearing d.SetId(),
so Terraform kept the resource in state and never offered to recreate it.

This change clears the resource ID and returns nil in three paths:
  1. The parent has no tag bindings at all (tagBindings key missing or nil).
  2. flattenNestedTagsLocationTagBinding returns nil (no matching item
     found across paginated results).
  3. The pagination break condition no longer abandons accumulated
     results when a subsequent page is empty — fixed by switching that
     return nil to break.

This matches the behaviour of the sibling google_tags_tag_binding
resource and lets terraform plan / apply correctly propose recreation
when a binding has been deleted out of band.

Fixes hashicorp/terraform-provider-google#24408

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.

google_tags_location_tag_binding does not detect when tag binding is deleted outside of Terraform

8 participants