Repository navigation
core,platform: Emit bare Terraform traversals instead of interpolation-only references - #159
Merged
Merged
Conversation
Replace interpolation-only "${var.X}" / "${type.label.field}" substitution
inside jsonencode(...) RawHCLValue blobs with bare Terraform traversals
(var.X, type.label.field) so tflint's terraform_deprecated_interpolation
rule no longer fires on exported HCL (issue #158).
Bare tokens are not valid JSON, and processRawHCLValue re-parses the same
blob JSON on every rule application, so plain-path substitution now swaps
the quoted UUID for a quoted JSON-safe sentinel ("@@TFREF@@<token>@@") via
new shared helpers in raw_hcl_refs.go (BareRefPlaceholder /
MaterializeBareRefs). ResolveEmbeddedReferences materializes all sentinels
to bare tokens in one sweep after every rule has run. The rich_text
UnwrapMode path keeps its "${...}" form — its reference is re-embedded
mid-string inside the Slate wrapper, never interpolation-only.
Existing assertions are updated to the bare form (updated, not deleted);
new tests cover the multi-rule single-blob re-parse case, sentinel-leak
absence, and first-match substitution semantics. Docs updated to describe
the new replacement form.
Co-Authored-By: Claude Code <noreply@anthropic.com>
…oducers
Switch the three DaVinci connector-instance property producers from
interpolation-only "${var.X}" strings to bare Terraform traversals
(var.X) inside jsonencode({...}) blobs, so tflint's
terraform_deprecated_interpolation rule no longer fires on exported
HCL (issue #158).
- formatPropertyValue (simple path) writes the bare token directly
into the hand-built blob.
- The complex-property secret and scalar branches store the
marshal-safe sentinel core (core.BareRefSentinelCore) in the Go
map; core.MaterializeBareRefs swaps each quoted placeholder for the
bare token after json.MarshalIndent, before the jsonencode(...) wrap.
Sorted-key determinism is unaffected.
- core: export bareRefSentinelCore as BareRefSentinelCore (same body)
for the platform producer; BareRefPlaceholder's doc now states the
marshal-safety contract; MaterializeBareRefs' doc acknowledges the
accepted exact-quoted-form residual risk (task 1 review).
Extracted-variable metadata (names, count, types, sensitivity) is
unchanged; tfjson formatter code is untouched (bare-var blobs wrap
into valid "${jsonencode(...)}" template expressions, covered by a
new test). Tests updated to the bare form with added negative
assertions (no "${var." and no @@TFREF@@ leak); new HCL formatter
integration test validates bare rendering and hclwrite re-parse of
nested jsonencode blobs. ReplaceMaskedSecrets untouched (no
production callers).
Changelog: .changelog/pr-158.txt (release-note:enhancement).
Co-Authored-By: Claude Code <noreply@anthropic.com>
Teach the regression-compare comparators that a quoted interpolation-only
reference ("${var.x}", "${type.label.field}") and its bare-traversal form
(var.x, type.label.field) are equivalent when the entire compared string is
one traversal-shaped interpolation. The HCL path normalizes attribute values
and jsonencode blob text via normalizeValue; the .tf.json path normalizes
parsed values in compareJSONResource, including the old formatter's
double-wrapped "${"${var.x}"}" form. Object-literal values (graph_data-style
bodies) are de-interpolated per line so reports stay clean; those mismatches
were already classified acceptable.
Issue #158 intentionally changes the exported rendering form from quoted
interpolation-only refs to bare traversals inside jsonencode blobs (the two
forms evaluate identically in Terraform), so without this equivalence the
regression workflow would fail the PR on its own intentional output change.
The rule is deliberately narrow: partial interpolations
("prefix-${var.x}", rich-text Slate wrappers) and non-traversal expressions
(function calls, indexes, jsonencode calls) are never normalized, so genuine
value changes still compare as breaking.
Verified against the committed regression snapshot: the tool reports no
differences between the old-form snapshot and a de-interpolated copy, and
still reports breaking (exit 1) when a variable name genuinely changes.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Add a three-layer guard so tflint's terraform_deprecated_interpolation rule cannot regress silently on exported HCL (issue #158): - .tflint.hcl at the repo root: terraform ruleset enabled with the deprecated-interpolation rule explicitly on. Four other default rules are disabled with documented reasons — each fires on valid exporter output (naming_convention: 285 findings on the regression snapshot; unused_declarations: 7; standard_module_structure: 1) or does not terminate in practical time on a real flow export (map_duplicate_keys). None of them touches the target rule. - tests/testdata/tflint-sample/: a hand-written sample module mirroring the post-fix exporter output — connector instance jsonencode blobs with bare var.X values (simple and post-marshal materialized nested forms), and a flow node graph_data blob with a bare type.label.field resource reference. - Makefile: new tflint target running tflint against the sample module with the repo-root config; fails non-zero on findings. - regression.yaml: tflint step in the regression-test job for hcl matrix entries only (the rule does not apply to .tf.json), running against the fresh PR export. Adds a PR-head checkout into a pr-source subdirectory because the job previously never checked out the repo and the config lives there. - .gitignore: negate *.hcl for .tflint.hcl so the config is checked in. Verified: make tflint passes (exit 0); reintroducing an interpolation- only "${var.X}" or "${type.label.field}" string into a scratch copy of the sample module makes tflint report the finding and exit 2; the rule fires 92 findings on the pre-fix regression snapshot and 0 on a copy de-interpolated the way the fixed exporter renders. Co-Authored-By: Claude Code <noreply@anthropic.com>
The regression workflow's export steps pass --module-dir regression-module, so the child module lands at ./output-pr/regression-module (internal/module/generator.go joins OutputDir with ModuleDirName). The tflint step pointed at ./output-pr/ping-export-module — the export command's default name, which the workflow overrides — so tflint failed with a nonexistent --chdir (exit 1) on every hcl matrix entry and never linted the export. Fix the path and name it in .tflint.hcl's header comment so the config and the workflow cannot drift apart again. Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
Regression Test Results
✅ No regressions detectedAll export configurations produced compatible output. Generated by regression workflow • View run |
Regression Test Results
✅ No regressions detectedAll export configurations produced compatible output. Generated by regression workflow • View run |
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
1 participant
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
Exported HCL wrapped interpolation-only
"${var.X}"and"${type.label.field}"strings inside hand-builtjsonencode({...})RawHCLValue blobs, tripping tflint'sterraform_deprecated_interpolationrule on every exported module. This PR replaces the interpolation-only form with bare Terraform traversals (var.X,type.label.field) at the blob producers, so tflint reports zero findings on exported HCL, and adds a three-layer tflint guard so the rule cannot regress silently.What changed and why (not just which files): the exporter emitted interpolation-only strings in two kinds of places — (1) the DaVinci connector-instance property producers (
internal/platform/pingone/resource_connection.go, simple + complex property paths) and (2) the embedded-reference resolver (internal/core/embedded_references.go, variable / fallback / resolved-resource substitution sites). Because custom transforms return opaquecore.RawHCLValueblobs that the HCL formatter writes verbatim, the fix had to land in the producers, not the formatters. Substitution uses a JSON-safe sentinel (@@TFREF@@<token>@@) that is materialized to bare tokens in a final sweep — required becauseResolveEmbeddedReferencesre-parses the same blob JSON across multiple rule applications, and a bare token would break that re-parse. The regression-compare tool was also taught that an interpolation-only quoted reference and its bare form are equivalent (the change is intentionally output-visible), and a tflint config + sample module +make tflinttarget + CI step guard fresh exports.Evaluated Terraform output is identical — only the rendered text changes (quotes and
${}dropped). Re-export is recommended; no HCL rework beyond re-export.Fixes #158
PR quality checklist
TestFormatter_Format_RawHCLValue_BareVarRefs(HCL end-to-end: bare present, interpolation-only absent,hclwrite.ParseConfigvalidity, re-indentation),TestFormat_RawHCLValue_BareVarBlob_WrapsAsTemplate(tfjson wrap form),TestResolveEmbeddedReferences_MultiRule_SingleBlob(sentinel re-parse across rules),TestResolveEmbeddedReferences_NoSentinelLeak_Materialized,TestResolveEmbeddedReferences_FirstMatchSemantics_Preserved,raw_hcl_refs_test.go(100% coverage on the new helpers), the tworesource_connection_test.gotable runners with per-run negatives, anddeinterpolation_test.go(compare-tool equivalence, incl. negative cases: partial interpolations still break). Existing tests were updated, not deleted (all 14 platform${var.assertions and 11+ core assertions converted).go test ./internal/... -count=1green (14 packages);make test(15 packages) green;make tflintexit 0;make vet,gofmt,go run ./tools/validate-definitionsclean.TestResolveEmbeddedReferences_DisambiguatedName_StableAcrossEnvironmentsis a pre-existing ~20-30% flake (production nondeterminism: wildcard-map iteration order decides which same-titled fallback node claims the un-suffixed variable name). Verified failing at base42765c5in both directions; untouched by this branch. Re-run to confirm; follow-up issue flagged.make lintreports findings, all pre-existing at base — none on code this branch changes.make build'sgo mod tidyprunes 2 stalepingone-go-client v0.12.0go.sum lines (pre-existing; reverted in this branch's commits).jsonencode({...})blobs; variable names, counts, sensitivity flags, fallback allocation, and the dependency graph are untouched (metadata tests pass unmodified); tfjson formatter code has zero changes (its"${" + blob + "}"wrap keeps yielding valid.tf.json, existing tfjson${var.X}assertions pass unmodified); rich-textUnwrapMode == "rich_text"re-embeddings and any UUID substituted mid-string keep${...}..changelog/pr-159.txt—release-note:enhancementdescribing the output-format change, re-export recommendation, identical evaluated output, and unchanged.tf.jsonwrapping. (Renamed to match this PR number.)Verification beyond unit tests
make tflint(sample module mirroring exporter output): passes; reintroducing an interpolation-only string fails it (exit 2) in bothvar.Xandtype.label.fieldshapes.output-pr/regression-module), so a deprecated-interpolation finding fails the job; tfjson entries are correctly excluded.terraform_deprecated_interpolationfindings pre-fix, 0 findings on the same content in the post-fix form.value_mismatch, exit 1 (the equivalence does not mask real regressions).tests/regression/environment/export.shwith 1Password credentials — the committed snapshot verification above covers the same mechanism.Known limitations / accepted residual risks
"@@TFREF@@<token>@@"would be materialized as a traversal — practically unreachable (sanitizers strip@from generated tokens; no producer emits@@TFREF@@into user-visible strings) and documented inMaterializeBareRefs's doc comment.ReplaceMaskedSecrets(internal/core/transforms.go:201,211) still emits the old form — it has no production callers (no YAML definition usesmasked_secret) and was intentionally left untouched..tflint.hclfrom the PR head, so a PR could disable the target rule in its own config; the config diff is visible in review (candidate hardening follow-up: base-branch config sourcing or a CI canary).Files
internal/core/raw_hcl_refs.go(new) — sentinel constant,BareRefPlaceholder,BareRefSentinelCore,MaterializeBareRefs+ recursive attribute walkerinternal/core/embedded_references.go— three substitution sites → bare tokens via sentinel; end-of-resolution materialization sweepinternal/core/raw_hcl_refs_test.go,internal/core/embedded_references_test.go— new helper tests + 3 new resolver tests; assertions updated to bare forminternal/platform/pingone/resource_connection.go— three producer sites → bare form (direct write / marshal-safe sentinel + post-marshal materialization)internal/platform/pingone/resource_connection_test.go— 14 assertions updated + per-runner negativesinternal/platform/pingone/resource_flow_test.go— 4 plain-path assertions updated (rich-text untouched)internal/formatters/hcl/formatter_test.go— new bare-var integration testinternal/formatters/tfjson/formatter_test.go— new bare-var wrap testinternal/compare/content.go,internal/compare/json.go,internal/compare/deinterpolation_test.go— de-interpolation equivalence for the regression comparators.tflint.hcl,tests/testdata/tflint-sample/(5 files),Makefile,.github/workflows/regression.yaml,.gitignore— tflint guard (sample module, make target, CI step).changelog/pr-159.txt— release notecontributing/ARCHITECTURE.md,contributing/DEVELOPER_HANDBOOK.md— step 8 replacement-form docs (bare full-string substitution, two-phase sentinel, rich-text retains${...})🤖 Generated with Claude Code