google_chronicle_rule: suppress trailing-newline permadiff on text#17991
Open
tsushanth wants to merge 1 commit into
Open
google_chronicle_rule: suppress trailing-newline permadiff on text#17991tsushanth wants to merge 1 commit into
text#17991tsushanth wants to merge 1 commit into
Conversation
The Chronicle Rules API silently appends a trailing `\n` to every stored rule body, so a rule submitted as `}` is read back as `}\n`. Because the provider does not normalise this, every plan after a successful apply shows the same phantom diff on every google_chronicle_rule (1,126 rules in the reporter's environment). Mirrors the same `custom_code.constants` + `diff_suppress_func` pattern already used by `google_chronicle_rule_deployment.run_frequency`. The suppress function trims trailing `\n` from both sides so that: - a config without a trailing newline matches the API value with one - multiple trailing newlines in the config also collapse cleanly - any other character difference (including a CRLF vs LF inside the body) still surfaces as a diff Existing acceptance fixtures use `<<-EOT` heredoc, which adds a trailing newline implicitly, so the rule.basic / rule.with_data_access_scope tests keep their current behaviour. Fixes hashicorp/terraform-provider-google#27881.
|
Googlers: For automatic test runs see go/terraform-auto-test-runs. @rileykarson, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
Collaborator
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit 2bddb8a: Diff reportYour PR generated the following diffs in downstream repositories:
Test reportAnalytics
Affected Service Packages
Step 1: Replaying Mode 🟢 All tests passed in Replaying mode! No Recording was needed. View the replaying VCR build log @tsushanth, @rileykarson, @ankitgoyal0301 VCR tests complete for 2bddb8a! |
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.
Fixes hashicorp/terraform-provider-google#27881.
Summary
The Chronicle Rules API silently appends a trailing
\nto every stored rule body. Because the provider does not normalise this, every plan after a successfulterraform applyshows the same phantom diff on everygoogle_chronicle_rule— the reporter sees it across all 1,126 rules they manage with Terraform.Reproduction (paraphrased from the linked issue):
Fix
Add a small resource-specific diff suppressor on
google_chronicle_rule.textthat trims trailing\nfrom both sides before comparing:This mirrors the existing
custom_code.constants+diff_suppress_funcwiring already used bygoogle_chronicle_rule_deployment.run_frequency(templates/terraform/constants/chronicle_rule_deployment.go.tmpl).The suppress logic is intentionally narrow:
"…}\n\n"and"…}\n"are still treated as equal.Tests
The existing fixtures
chronicle_rule_basic.tf.tmpl,chronicle_rule_with_data_access_scope.tf.tmpl, andchronicle_rule_with_force_deletion.tf.tmplauthor thetextfield via<<-EOT … EOTheredocs, which implicitly add a trailing newline. They currently pass by coincidence (the API's trailing\nhappens to match the heredoc's). With this change they continue to pass because the suppressor normalises both directions. Happy to add an acceptance fixture that authorstextwithout a trailing newline — typically the more interesting regression case — if reviewers would like one.Release Note