Skip to content

cloudbuild: suppress comment_control permadiff on pull_request blocks (#19853)#61

Open
jbbqqf wants to merge 11 commits into
mainfrom
feat/19853-cloudbuild-trigger-comment-control-permadiff
Open

cloudbuild: suppress comment_control permadiff on pull_request blocks (#19853)#61
jbbqqf wants to merge 11 commits into
mainfrom
feat/19853-cloudbuild-trigger-comment-control-permadiff

Conversation

@jbbqqf
Copy link
Copy Markdown
Owner

@jbbqqf jbbqqf commented May 9, 2026

Summary

Suppress the permanent diff on comment_control in google_cloudbuild_trigger's pull-request filters. When users explicitly set comment_control = "COMMENTS_DISABLED" (the API's semantic default), the API stores/returns an empty value because the field is omitempty in the proto, and Terraform shows a forever-pending update to add COMMENTS_DISABLED back.

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

Why

In the Cloud Build v1 API client (google.golang.org/api/cloudbuild/v1):

type PullRequestFilter struct {
    // ...
    CommentControl string `json:"commentControl,omitempty"`
}

The omitempty JSON tag matches a behavior we can confirm against state output in the issue: a user sets comment_control = "COMMENTS_DISABLED", applies, and on the next plan the provider reads back comment_control = "" from state. Result: comment_control = "" -> "COMMENTS_DISABLED" shows up on every plan, forever.

There are four such fields in mmv1/products/cloudbuild/Trigger.yaml (one per pull_request block — repository_event_config, github, bitbucket_server_trigger_config, and source_to_build); the bug applies to all of them. The mmv1 codebase already has a perfect helper for this exact shape — tpgresource.EmptyOrDefaultStringSuppress(default) — used in roughly a dozen other places (e.g. cloudscheduler/Job.yaml, hypercomputecluster/Cluster.yaml).

GCP API reference: https://cloud.google.com/build/docs/api/reference/rest/v1/projects.triggers#PullRequestFilter

What changed

mmv1 YAML edit to a single file:

mmv1/products/cloudbuild/Trigger.yaml | 4 ++++

Adds diff_suppress_func: 'tpgresource.EmptyOrDefaultStringSuppress("COMMENTS_DISABLED")' on each of the four commentControl fields.

Edge cases tested

# Scenario Expected Verified by
1 comment_control unset in HCL API returns ""; state stays ""; no diff Behavior unchanged from main (suppress treats "" == "").
2 comment_control = "COMMENTS_DISABLED" (the OP's case) First apply: provider sends COMMENTS_DISABLED, API drops it via omitempty, returns "". Read sets state to "". Suppress treats "" vs "COMMENTS_DISABLED" as equal → no diff on next plan. Static — EmptyOrDefaultStringSuppress is a well-tested helper (utils_test.go:899 TestEmptyOrDefaultStringSuppress).
3 comment_control = "COMMENTS_ENABLED" (non-default) Provider sends COMMENTS_ENABLED, API persists it, read returns COMMENTS_ENABLED, state matches config → no diff. The suppress helper does not affect the non-default case ("COMMENTS_DISABLED" != "COMMENTS_ENABLED"). Same — covered by helper unit tests.
4 Switching from COMMENTS_DISABLED to COMMENTS_ENABLED Real change — should show diff and update Suppress only matches ("", "COMMENTS_DISABLED"); transitions between non-empty values are not suppressed.

Test protocol

Test Result Notes
YAML lint / mmv1 generation n/a in this PR Will be exercised by mmv1 CI. The change is one extra line per affected field, all using an existing third-party helper.
Live BEFORE/AFTER smoke not run A live smoke would require an installed Cloud Build → GitHub connection in the test project (the OP's repro uses google_cloudbuildv2_repository against a real GitHub repo). That's expensive to set up and tear down per scenario, and the diff is structurally proven by the API client's omitempty tag plus a well-established suppression helper.

The author (a human) verified:

  1. The API client (api@v0.278.0/cloudbuild/v1/cloudbuild-gen.go:3647) uses json:"commentControl,omitempty".
  2. The EmptyOrDefaultStringSuppress helper exists at mmv1/third_party/terraform/tpgresource/common_diff_suppress.go:14 and is unit-tested.
  3. The same pattern is used elsewhere in the same DSL.

Resources

Disclosure

This PR was implemented with assistance from Claude Code as part of a focused contribution batch. The diff was reviewed manually against the GCP REST API documentation and the vendored Go client, which together confirm the omitempty behavior that produces the permadiff.

jcromanu and others added 11 commits May 8, 2026 16:43
…19853)

The Cloud Build PullRequestFilter.commentControl field is annotated
omitempty in the API client, so when set to its semantic default
(COMMENTS_DISABLED) the API returns an empty string. The provider then
flattens "" into state and Terraform shows a permanent diff against any
config that explicitly sets COMMENTS_DISABLED.

Add EmptyOrDefaultStringSuppress("COMMENTS_DISABLED") on every
commentControl field (repository_event_config, github,
bitbucket_server_trigger_config, source_to_build pull_request blocks).

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_cloudbuild_trigger.repository_event_config is not saved correctly in state

8 participants