Skip to content

datacatalog: don't drop bool_value when ordered field is present#70

Open
jbbqqf wants to merge 11 commits into
mainfrom
feat/16856-datacatalog-tag-bool-with-order
Open

datacatalog: don't drop bool_value when ordered field is present#70
jbbqqf wants to merge 11 commits into
mainfrom
feat/16856-datacatalog-tag-bool-with-order

Conversation

@jbbqqf
Copy link
Copy Markdown
Owner

@jbbqqf jbbqqf commented May 9, 2026

Summary

Fixes a regression in the custom encoder for google_data_catalog_tag
where bool_value was silently dropped whenever the matching field had
any companion key — including pure metadata like order — instead of
only when another value-typed key (stringValue, doubleValue, …) was
present.

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

Why

The DataCatalog API rejects a field payload that contains more than one
value-typed entry (one of boolValue, stringValue, doubleValue,
timestampValue, enumValue, richtextValue). To prevent this on the
provider side, the custom encoder added in
GoogleCloudPlatform/magic-modules#4004
strips boolValue whenever a field has multiple entries, on the
assumption that a multi-entry field implies a value conflict.

That assumption is wrong. A user who writes:

fields {
  field_id     = "bool_field"
  display_name = "..."
  type {
    primitive_type = "BOOL"
  }
  order = 1   # any non-zero value
}

…and then a tag with:

fields {
  field_name = "bool_field"
  bool_value = false
}

…produces a field map with {boolValue: false, order: 1} after the
expand pass. The old encoder saw len(values) > 1 and stripped
boolValue — even though order is a metadata key, not a value-typed
key. The resulting payload had no value at all, breaking updates with
either an API error or a silent state mismatch.

Maintainer @roaks3 confirmed in the issue thread:

It looks like some of this behavior was explicitly changed with PR
GoogleCloudPlatform#4004, and we most likely missed an edge case.

GCP API reference:

What changed

mmv1 custom encoder template only — replaces the unconditional
"more-than-one-key" strip with a check that at least one other
value-typed
key is present. Pure metadata companions (order,
displayName, fieldName, column) no longer trigger the strip.

 mmv1/templates/terraform/encoders/data_catalog_tag.go.tmpl | 41 ++++++++++++++++---
 1 file changed, 33 insertions(+), 8 deletions(-)

The original "value-conflict" defense is preserved (e.g. if a user really
did set both bool_value and string_value on the same field, we still
strip boolValue — same as today).

Edge cases tested

# Scenario HCL excerpt Expected Verified by
1 bool_value alone, ordered field bool_value = false + tag template order = 1 Payload contains boolValue: false, update succeeds static — encoder retains the key because no other value-typed key is present
2 bool_value + string_value (legitimate conflict) both set on same field boolValue stripped (preserves PR GoogleCloudPlatform#4004 behavior); only stringValue reaches API static — encoder still strips when another value-typed key present
3 string_value only, ordered field string_value = "x" + order = 1 stringValue reaches API; boolValue was never present so encoder is no-op static — encoder skips early when no boolValue

Test protocol

Test Result Notes
Encoder logic review OK Iterates fields once; only strips boolValue when another value-typed key shares the same field
Backwards compatibility OK Behavior unchanged for the conflict case PR GoogleCloudPlatform#4004 was designed to handle
Live BEFORE/AFTER smoke not run DataCatalog tag template provisioning + tag update needs a working entry group; live smoke deferred for this batch. The fix is logic-tightening on a clearly-identified bug; reviewer can validate with the issue's exact reproducer.

Resources

Disclosure

This PR was drafted with assistance from Claude Code as part of a parallel
contribution batch. The encoder change was reviewed against the
DataCatalog API documentation and against the original maintainer
acknowledgement in the issue thread. Live smoke was not run; the author
(a human) will review the diff and the modular-magician downstream PRs
before requesting maintainer review.

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

The custom encoder for google_data_catalog_tag previously stripped
boolValue from a field payload whenever the field map had more than one
entry. That logic was meant to enforce the DataCatalog rule "a field
contains at most one *value*-typed entry" (boolValue, stringValue,
doubleValue, timestampValue, enumValue, richtextValue), but it fired
indiscriminately on any companion key — including metadata such as
order, displayName, or fieldName.

When a tag template defines a BOOL field with a non-zero `order` (or any
non-default `display_name` etc.), the user's explicit `bool_value`
(typically `false`) would silently disappear from the request payload,
making subsequent updates fail or behave incorrectly.

Restrict the strip to the case where another *value*-typed key is also
present, which is the actual conflict the API rejects. Pure metadata
companions are now ignored.

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.

DataCatalog tag update fails when ordered boolean field is present.

8 participants