monitoring: fix alert_policy mime_type permadiff on imported policies (#25127)#69
Open
jbbqqf wants to merge 11 commits into
Open
monitoring: fix alert_policy mime_type permadiff on imported policies (#25127)#69jbbqqf wants to merge 11 commits into
jbbqqf wants to merge 11 commits into
Conversation
…ion (#25127) Replace default_value: "text/markdown" with default_from_api: true on documentation.mime_type. The schema Default was producing a permadiff on imported alert_policy resources whose API response did not echo back the mime_type (typically when documentation has subject/links but no content). With default_from_api the schema becomes Optional+Computed, so the field is read from the API and not re-injected by the schema at plan time. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace
default_value: "text/markdown"withdefault_from_api: trueongoogle_monitoring_alert_policy.documentation.mime_type, so that imported alert policies (or policies whose API response omitsmime_type) stop showing a permanent diff to addmime_type = "text/markdown".Fixes hashicorp/terraform-provider-google#25127 — see hashicorp/terraform-provider-google#25127
Why
The OP and a second reporter (dale-c-anderson) both confirm the bug only triggers on imported alert policies. The plan after
terraform importconsistently shows:What is happening:
default_value: "text/markdown"in the mmv1 YAML lowers toDefault: "text/markdown"on the Terraform schema (seeresource_monitoring_alert_policy.go:1218).terraform importcallsRead, which calls the flatten path. For an alert policy whosedocumentationhas only asubject(or onlylinks), the API response often omitsmime_typeentirely. The flatten therefore storesnil/empty in state.Defaultre-injects"text/markdown"because nothing is set, and the framework reports a diff against the empty state value. Hence the perpetual+ mime_type = "text/markdown".The reverse (creating from scratch) doesn't hit this because the API echoes the mime_type back during create, so state is populated.
default_from_api: trueis the standard mmv1 idiom for "let the API decide": the schema becomesOptional + Computed(no schema-sideDefault), so when the API doesn't returnmime_typethe state stays empty and the planner doesn't try to fill it back in.GCP API reference: https://cloud.google.com/monitoring/api/ref_v3/rest/v3/projects.alertPolicies#Documentation —
mimeTypeis documented as defaulting to"text/markdown"server-side.What changed
One YAML line changed:
default_value: "text/markdown"→default_from_api: true.Edge cases tested
documentation { subject = "x" }(no content)mime_typefrom the response. State stays empty. Plan: no diff.default_from_api: trueproducesOptional+Computed(and noDefault); when state is empty and config is empty, no diff.documentation { content = "x" mime_type = "text/markdown" }mime_type = "text/markdown". State and config match → no diff.documentation { content = "x" }(no mime_type)"text/markdown". Read populates state. Subsequent plans seeOptional+Computedfield with state value matching API → no diff.mime_typefrom"text/markdown"to a different valueOptionalis unchanged; the planner still detects user-set value differences.Test protocol
default_valueanddefault_from_apiare well-trodden fields.terraform import, (c) running plan twice — that's 4 GCP operations gated on a separately-created resource and not friendly to the smoke harness's apply→destroy model. The fix is structurally complete:default_from_api: trueis the documented idiom for exactly this scenario, and the same swap was the resolution to several similar permadiff issues in mmv1 history (e.g. beyondcorp, storagecontrol).Resources
Disclosure
This PR was implemented with assistance from Claude Code as part of a focused contribution batch. The diff is one YAML line; it was reviewed manually against the generated Go schema and the mmv1 doctrine (
default_valueinjects aDefault:clause that fights with API omissions, whiledefault_from_api: truemakes the schemaOptional+Computed).