fix: YAML functions fail with templated args when custom delimiters include quotes#2098
Merged
fix: YAML functions fail with templated args when custom delimiters include quotes#2098
Conversation
…nclude quotes (#2052) When custom template delimiters contain single-quote characters (e.g., ["'{{", "}}'"]), yaml.v3's single-quote escaping ('') interferes with delimiter patterns, producing invalid YAML after template replacement. This affects all YAML functions (!terraform.state, !terraform.output, !store, !exec, !env, !include, etc.) since they all start with '!' which triggers single-quoted encoding. Add ConvertToYAMLPreservingDelimiters that detects delimiter/quoting conflicts and forces double-quoted YAML style for affected scalars, preserving delimiter characters literally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Contributor
📝 WalkthroughWalkthroughAdds delimiter-safe YAML serialization and tests to preserve custom template delimiters (including those with single quotes) during YAML encoding. Replaces prior serialization calls with the new ConvertToYAMLPreservingDelimiters across template-processing call sites and adds fixtures, integration tests, and documentation describing the fix. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
pkg/utils/yaml_utils.go (1)
639-717: Consider moving YAML-encoding helpers to a dedicated package.
delimiterConflictsWithYAMLQuoting,ensureDoubleQuotedForDelimiterSafety, andConvertToYAMLPreservingDelimitersare tightly cohesive — apkg/yamlenc/package would keeppkg/utils/leaner and satisfy the no-new-functions-in-utils guideline. The existing//nolint:revive // File length justified…at the bottom of the file signals this file is already pushing its size budget.As per coding guidelines: "Avoid utils package bloat — don't add new functions to
pkg/utils/. Create purpose-built packages for new functionality."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/utils/yaml_utils.go` around lines 639 - 717, The three cohesive helpers ConvertToYAMLPreservingDelimiters, delimiterConflictsWithYAMLQuoting, and ensureDoubleQuotedForDelimiterSafety should be moved out of pkg/utils into a new purpose-built package (e.g., pkg/yamlenc) to avoid utils bloat; create pkg/yamlenc with the same implementations, move any tightly-coupled dependencies they need (e.g., YAMLOptions, DefaultYAMLIndent, yamlBufferPool or expose internal helpers from existing packages), update imports where ConvertToYAMLPreservingDelimiters is called to use the new package, keep function names or export them as needed, and remove these functions from pkg/utils (and adjust/remove the file-length nolint comment if it’s no longer needed). Ensure package-level tests and build pass after updating references.tests/yaml_functions_custom_delimiters_test.go (2)
53-99: Two subtests run identicalExecuteDescribeComponentcalls — merge to eliminate duplicate work.Both
"component-2 with terraform.state static args"(lines 53–73) and"component-2 with terraform.state templated stack arg"(lines 75–99) callExecuteDescribeComponentwith exactly the same parameters. This triggers the full stack-processing pipeline twice for the same result. Merge them into a single subtest and assert all three vars together.♻️ Suggested refactor
- t.Run("component-2 with terraform.state static args", func(t *testing.T) { - componentSection, err := e.ExecuteDescribeComponent( - &e.ExecuteDescribeComponentParams{ - Component: "component-2", - Stack: "test", - ProcessTemplates: true, - ProcessYamlFunctions: true, - }, - ) - - require.NoError(t, err, "component-2 with static terraform.state args should load without errors") - require.NotNil(t, componentSection) - - vars, ok := componentSection["vars"].(map[string]interface{}) - require.True(t, ok, "vars should be a map") - - // Static arg: !terraform.state component-1 foo - assert.Equal(t, "foo-from-state", vars["foo"], "static terraform.state should resolve from state file") - }) - - t.Run("component-2 with terraform.state templated stack arg", func(t *testing.T) { - // This is the core test for GitHub issue `#2052`. - componentSection, err := e.ExecuteDescribeComponent( - &e.ExecuteDescribeComponentParams{ - Component: "component-2", - Stack: "test", - ProcessTemplates: true, - ProcessYamlFunctions: true, - }, - ) - - require.NoError(t, err, "component-2 with templated terraform.state args should NOT fail with 'did not find expected key' (issue `#2052`)") - require.NotNil(t, componentSection) - - vars, ok := componentSection["vars"].(map[string]interface{}) - require.True(t, ok, "vars should be a map") - - assert.Equal(t, "bar-from-state", vars["bar"], "templated terraform.state with custom delimiters should resolve correctly") - assert.Equal(t, "baz-from-state", vars["baz"], "templated terraform.state with custom delimiters should resolve correctly") - }) + t.Run("component-2 with static and templated terraform.state args (issue `#2052`)", func(t *testing.T) { + componentSection, err := e.ExecuteDescribeComponent( + &e.ExecuteDescribeComponentParams{ + Component: "component-2", + Stack: "test", + ProcessTemplates: true, + ProcessYamlFunctions: true, + }, + ) + + require.NoError(t, err, "component-2 should NOT fail with 'did not find expected key' (issue `#2052`)") + require.NotNil(t, componentSection) + + vars, ok := componentSection["vars"].(map[string]interface{}) + require.True(t, ok, "vars should be a map") + + // Static arg: !terraform.state component-1 foo + assert.Equal(t, "foo-from-state", vars["foo"], "static terraform.state should resolve from state file") + // Templated arg: !terraform.state component-1 '{{ .stack }}' bar/baz + assert.Equal(t, "bar-from-state", vars["bar"], "templated terraform.state with custom delimiters should resolve correctly") + assert.Equal(t, "baz-from-state", vars["baz"], "templated terraform.state with custom delimiters should resolve correctly") + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/yaml_functions_custom_delimiters_test.go` around lines 53 - 99, The two subtests both call e.ExecuteDescribeComponent with identical params; call ExecuteDescribeComponent once (e.g., in a single t.Run combining "component-2 with terraform.state ...") and reuse the returned componentSection for all assertions: check vars map existence and assert vars["foo"] == "foo-from-state" and vars["bar"] == "bar-from-state" and vars["baz"] == "baz-from-state"; keep ProcessTemplates and ProcessYamlFunctions true and preserve the original assertions' messages, but remove the duplicated second ExecuteDescribeComponent invocation.
104-131:TestCustomDelimitersTemplateProcessingduplicates the component-1 subtest already inTestTerraformStateWithCustomDelimiters.The single subtest here (
"settings templates resolve with custom delimiters", lines 111–131) navigates to the same fixture, callsExecuteDescribeComponentfor the same component/stack, and asserts the same three vars as lines 29–51. Consider removing this function or folding any genuinely distinct scenario intoTestTerraformStateWithCustomDelimiters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/yaml_functions_custom_delimiters_test.go` around lines 104 - 131, The test TestCustomDelimitersTemplateProcessing duplicates the subtest "settings templates resolve with custom delimiters" (same fixture, same ExecuteDescribeComponent call and same assertions) already covered by TestTerraformStateWithCustomDelimiters; remove TestCustomDelimitersTemplateProcessing entirely or fold any unique assertions into TestTerraformStateWithCustomDelimiters, ensuring you keep the ExecuteDescribeComponent call and the three asserts for vars["foo"], vars["bar"], and vars["baz"] in the remaining test and delete the redundant subtest.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/fixes/2026-02-20-terraform-state-custom-delimiters.md`:
- Around line 92-97: The YAML example has a misplaced closing single-quote in
the data value causing incorrect quoting: fix the string for the key `data` so
the outer single-quoted scalar encloses the entire value `!store my-store ''{{
.stack }}'' key` (i.e., move the closing quote to after `key`) so all internal
single-quotes remain escaped and the outer quote wraps the whole value; check
the similar patterns for `test` and `cmd` to ensure each value uses matching
outer single-quotes with internal quotes escaped as `''`.
In `@pkg/utils/yaml_utils.go`:
- Around line 639-644: Fix two typos in the doc comments: in the
ConvertToYAMLPreservingDelimiters comment update the example delimiter to
["'{{", "}}'"] (replace the stray trailing double-quote) and change the
description that currently says internal single quotes are escaped as a
double-quote to correctly state they are escaped by doubling (''), and make the
same correction in the ensureDoubleQuotedForDelimiterSafety comment where it
currently shows (") — replace with ('').
- Around line 645-684: Two call sites still invoke ConvertToYAML before template
expansion; replace those calls with ConvertToYAMLPreservingDelimiters and pass
the active template delimiters so single-quote conflicts are avoided.
Specifically, in the backend config template path (the call that currently does
ProcessTmpl(ConvertToYAML(...), ...)) and in the stack manifest template path
(the call that feeds a YAML string directly into ProcessTmpl), swap
ConvertToYAML(...) for ConvertToYAMLPreservingDelimiters(..., delimiters) where
"delimiters" is the same delimiter slice/variable used for template processing;
preserve existing opts/indent/error handling and return semantics. Ensure you
import/propagate any delimiter variable if not already in scope and keep
ProcessTmpl usage unchanged.
In `@tests/yaml_functions_custom_delimiters_test.go`:
- Line 22: Replace the meaningless require.NotNil(t, atmosConfig) (since
atmosConfig is a value of type schema.AtmosConfiguration) with a meaningful
assertion: remove the NotNil call and instead assert the Initialized field
(e.g., require.True(t, atmosConfig.Initialized)) or another relevant
field/property; locate the assertion near the existing require.NoError that
checks err in tests/yaml_functions_custom_delimiters_test.go and update both
occurrences referenced (lines around the current checks, variable atmosConfig)
accordingly.
---
Nitpick comments:
In `@pkg/utils/yaml_utils.go`:
- Around line 639-717: The three cohesive helpers
ConvertToYAMLPreservingDelimiters, delimiterConflictsWithYAMLQuoting, and
ensureDoubleQuotedForDelimiterSafety should be moved out of pkg/utils into a new
purpose-built package (e.g., pkg/yamlenc) to avoid utils bloat; create
pkg/yamlenc with the same implementations, move any tightly-coupled dependencies
they need (e.g., YAMLOptions, DefaultYAMLIndent, yamlBufferPool or expose
internal helpers from existing packages), update imports where
ConvertToYAMLPreservingDelimiters is called to use the new package, keep
function names or export them as needed, and remove these functions from
pkg/utils (and adjust/remove the file-length nolint comment if it’s no longer
needed). Ensure package-level tests and build pass after updating references.
In `@tests/yaml_functions_custom_delimiters_test.go`:
- Around line 53-99: The two subtests both call e.ExecuteDescribeComponent with
identical params; call ExecuteDescribeComponent once (e.g., in a single t.Run
combining "component-2 with terraform.state ...") and reuse the returned
componentSection for all assertions: check vars map existence and assert
vars["foo"] == "foo-from-state" and vars["bar"] == "bar-from-state" and
vars["baz"] == "baz-from-state"; keep ProcessTemplates and ProcessYamlFunctions
true and preserve the original assertions' messages, but remove the duplicated
second ExecuteDescribeComponent invocation.
- Around line 104-131: The test TestCustomDelimitersTemplateProcessing
duplicates the subtest "settings templates resolve with custom delimiters" (same
fixture, same ExecuteDescribeComponent call and same assertions) already covered
by TestTerraformStateWithCustomDelimiters; remove
TestCustomDelimitersTemplateProcessing entirely or fold any unique assertions
into TestTerraformStateWithCustomDelimiters, ensuring you keep the
ExecuteDescribeComponent call and the three asserts for vars["foo"],
vars["bar"], and vars["baz"] in the remaining test and delete the redundant
subtest.
…eview feedback - Move ConvertToYAMLPreservingDelimiters, DelimiterConflictsWithYAMLQuoting, and EnsureDoubleQuotedForDelimiterSafety from pkg/utils/ to pkg/yaml/ - Add perf.Track() to all public functions - Merge duplicate component-2 subtests in integration test - Remove duplicate TestCustomDelimitersTemplateProcessing - Replace require.NotNil on struct with require.True on Initialized field - Fix YAML example typo in docs (mismatched single-quote escaping for !store) - Update all callers to use atmosYaml import alias Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/yaml_functions_custom_delimiters_test.go (1)
32-39: Consider passing the already-loadedatmosConfigto avoid redundant re-initialization.
AtmosConfigis optional but passing&atmosConfigdirectly toExecuteDescribeComponentParamsavoids a secondInitCliConfigcall insideExecuteDescribeComponentand makes the test's configuration dependency explicit. Same applies to the second subtest at Lines 59-66.♻️ Proposed refactor
componentSection, err := e.ExecuteDescribeComponent( &e.ExecuteDescribeComponentParams{ + AtmosConfig: &atmosConfig, Component: "component-1", Stack: "test", ProcessTemplates: true, ProcessYamlFunctions: true, }, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/yaml_functions_custom_delimiters_test.go` around lines 32 - 39, The test currently calls e.ExecuteDescribeComponent without supplying the preloaded atmosConfig, causing ExecuteDescribeComponent to re-run InitCliConfig; update both subtests to pass the already-loaded atmosConfig by setting the AtmosConfig field on ExecuteDescribeComponentParams to &atmosConfig so ExecuteDescribeComponent can use the provided config and avoid redundant initialization (refer to e.ExecuteDescribeComponent and ExecuteDescribeComponentParams and the local atmosConfig variable).pkg/yaml/delimiter.go (1)
83-102:perf.Trackdeferred on every recursive invocation.
EnsureDoubleQuotedForDelimiterSafetycalls itself recursively for every child node. Each call acquires and defers aperf.Trackclosure. When tracking is disabled (the common case),perf.Trackreturns a no-op immediately, so runtime impact is minimal. For very large YAML documents, though, the cumulative closure allocation per node is non-zero.Consider extracting the recursion into a private unexported helper (e.g.,
ensureDoubleQuotedForDelimiterSafetyRecursive) so theperf.Trackfires only once on the public entry point.♻️ Proposed refactor
func EnsureDoubleQuotedForDelimiterSafety(node *goyaml.Node) { defer perf.Track(nil, "yaml.EnsureDoubleQuotedForDelimiterSafety")() - if node == nil { - return - } - - switch node.Kind { - case goyaml.ScalarNode: - if strings.ContainsRune(node.Value, '\'') { - node.Style = goyaml.DoubleQuotedStyle - } - case goyaml.DocumentNode, goyaml.MappingNode, goyaml.SequenceNode: - for _, child := range node.Content { - EnsureDoubleQuotedForDelimiterSafety(child) - } - } + ensureDoubleQuotedRecursive(node) +} + +func ensureDoubleQuotedRecursive(node *goyaml.Node) { + if node == nil { + return + } + + switch node.Kind { + case goyaml.ScalarNode: + if strings.ContainsRune(node.Value, '\'') { + node.Style = goyaml.DoubleQuotedStyle + } + case goyaml.DocumentNode, goyaml.MappingNode, goyaml.SequenceNode: + for _, child := range node.Content { + ensureDoubleQuotedRecursive(child) + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/yaml/delimiter.go` around lines 83 - 102, The perf.Track call is deferred on every recursive invocation of EnsureDoubleQuotedForDelimiterSafety causing unnecessary closure allocations for large YAMLs; refactor by making EnsureDoubleQuotedForDelimiterSafety the public entry that calls perf.Track once, then delegate recursion to a new unexported helper (e.g., ensureDoubleQuotedForDelimiterSafetyRecursive) which contains the current switch logic and recursive calls, and remove perf.Track from the recursive helper so only the public function wraps the single perf.Track defer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/yaml/delimiter_test.go`:
- Around line 27-40: The test "left delimiter with single quote conflicts" is
duplicated with "both delimiters with single quotes conflict" because both use
delimiters []string{"'{{", "}}'"}; update the first test case (name: "left
delimiter with single quote conflicts") so its delimiters slice only has the
left delimiter with a single quote and a normal right delimiter (e.g., change
delimiters from {"'{{", "}}'"} to {"'{{", "}}"}) so it exercises the left-only
conflict path; locate and update the delimiters slice in the test case entry in
delimiter_test.go.
---
Nitpick comments:
In `@pkg/yaml/delimiter.go`:
- Around line 83-102: The perf.Track call is deferred on every recursive
invocation of EnsureDoubleQuotedForDelimiterSafety causing unnecessary closure
allocations for large YAMLs; refactor by making
EnsureDoubleQuotedForDelimiterSafety the public entry that calls perf.Track
once, then delegate recursion to a new unexported helper (e.g.,
ensureDoubleQuotedForDelimiterSafetyRecursive) which contains the current switch
logic and recursive calls, and remove perf.Track from the recursive helper so
only the public function wraps the single perf.Track defer.
In `@tests/yaml_functions_custom_delimiters_test.go`:
- Around line 32-39: The test currently calls e.ExecuteDescribeComponent without
supplying the preloaded atmosConfig, causing ExecuteDescribeComponent to re-run
InitCliConfig; update both subtests to pass the already-loaded atmosConfig by
setting the AtmosConfig field on ExecuteDescribeComponentParams to &atmosConfig
so ExecuteDescribeComponent can use the provided config and avoid redundant
initialization (refer to e.ExecuteDescribeComponent and
ExecuteDescribeComponentParams and the local atmosConfig variable).
…plicate test case - Extract ensureDoubleQuotedRecursive so perf.Track fires only once on the public EnsureDoubleQuotedForDelimiterSafety entry point - Fix duplicate test input: "left delimiter" case now uses right delimiter without single quote, distinct from "both delimiters" case Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/yaml/delimiter_test.go (2)
230-237: Bare type assertions in tests will panic with an unhelpful stack trace on failure.Several lines do
parsed["vars"].(map[string]interface{})without the two-value form. If the YAML structure changes or Unmarshal produces an unexpected shape, you get a panic instead of a clear test failure message.Not blocking, but using
requirekeeps failures readable:Suggested approach
- vars := parsed["vars"].(map[string]interface{}) + varsRaw, ok := parsed["vars"].(map[string]interface{}) + require.True(t, ok, "expected vars to be map[string]interface{}") + vars := varsRawAlso applies to: 439-445, 531-537
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/yaml/delimiter_test.go` around lines 230 - 237, The test uses a bare type assertion parsed["vars"].(map[string]interface{}) which will panic on unexpected shapes; change to the two-value form (vars, ok := parsed["vars"].(map[string]interface{})) and add a require.True(t, ok, "parsed.vars should be map[string]interface{}") (or require.IsType/require.NotNil) before using vars so failures produce readable test errors; apply the same fix to the other occurrences around the file (the other blocks referencing parsed and vars at/near lines 439-445 and 531-537) and keep the existing assert.Equal checks for individual keys after the guarded assertion.
407-414:!repo-rootand!cwdentries never hit the assertion branch.These values don't contain single quotes, so the
if strings.Contains(fn.value, "'")guard at line 434 skips all meaningful assertions. They only confirmConvertToYAMLPreservingDelimitersdoesn't error — which is already covered by other cases.Either remove them or add a distinct assertion for the no-quotes path so they carry their weight.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/yaml/delimiter_test.go` around lines 407 - 414, The two table cases with name "repo-root (no args but with quotes in context)" and "cwd (no args but with quotes in context)" never exercise the single-quote assertion because fn.value has no single quotes; update the test around ConvertToYAMLPreservingDelimiters to either remove those cases or add an explicit assertion for the no-quotes path (e.g., assert the output is unchanged or contains the same delimiter token) so they validate behavior — find the test loop that references fn.value and add a branch for values without "'" that checks the expected output for "!repo-root" and "!cwd" (or remove the entries if redundant).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/yaml/delimiter_test.go`:
- Around line 230-237: The test uses a bare type assertion
parsed["vars"].(map[string]interface{}) which will panic on unexpected shapes;
change to the two-value form (vars, ok :=
parsed["vars"].(map[string]interface{})) and add a require.True(t, ok,
"parsed.vars should be map[string]interface{}") (or
require.IsType/require.NotNil) before using vars so failures produce readable
test errors; apply the same fix to the other occurrences around the file (the
other blocks referencing parsed and vars at/near lines 439-445 and 531-537) and
keep the existing assert.Equal checks for individual keys after the guarded
assertion.
- Around line 407-414: The two table cases with name "repo-root (no args but
with quotes in context)" and "cwd (no args but with quotes in context)" never
exercise the single-quote assertion because fn.value has no single quotes;
update the test around ConvertToYAMLPreservingDelimiters to either remove those
cases or add an explicit assertion for the no-quotes path (e.g., assert the
output is unchanged or contains the same delimiter token) so they validate
behavior — find the test loop that references fn.value and add a branch for
values without "'" that checks the expected output for "!repo-root" and "!cwd"
(or remove the entries if redundant).
The .gitignore pattern `*.tfstate.*` was matching the directory name `terraform.tfstate.d` in the fixture path, preventing the test state file from being committed. Force-add the file so CI can find it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2098 +/- ##
==========================================
+ Coverage 76.19% 76.22% +0.02%
==========================================
Files 829 830 +1
Lines 78551 78597 +46
==========================================
+ Hits 59852 59907 +55
+ Misses 14956 14947 -9
Partials 3743 3743
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
osterman
approved these changes
Feb 22, 2026
|
These changes were released in v1.208.0-rc.0. |
|
These changes were released in v1.208.0-test.15. |
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
2 participants
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.
what
!terraform.state,!terraform.output,!store,!store.get,!env,!exec,!include,!include.raw,!random, etc.) fail withyaml: line NNN: did not find expected keywhen custom template delimiters contain single-quote characters (e.g.,["'{{", "}}'"])ConvertToYAMLPreservingDelimitersfunction that detects delimiter/YAML quoting conflicts and forces double-quoted YAML style for affected scalar values, preserving delimiter characters literally in the serialized outputinternal/exec/utils.go,internal/exec/describe_stacks.go(3 places),internal/exec/terraform_generate_varfiles.go,internal/exec/terraform_generate_backends.go) to use the new functionwhy
'as''. For example,!terraform.state vpc '{{ .stack }}' vpc_idbecomes'!terraform.state vpc ''{{ .stack }}'' vpc_id''{{and}}'then finds'{{within the''{{sequence and performs template replacement, producing'!terraform.state vpc 'nonprod' vpc_id'— which is invalid YAML because unescaped single quotes break the single-quoted string"!terraform.state vpc '{{ .stack }}' vpc_id". After template replacement:"!terraform.state vpc nonprod vpc_id"— valid YAML'gets double-quoted when delimiters contain'), so it automatically protects all current and future YAML functions{{/}}) are unaffected — the function falls back to standard encoding when there is no conflictTests
Unit tests (
pkg/utils/yaml_utils_delimiter_test.go) — 63 subtests:TestDelimiterConflictsWithYAMLQuoting— 8 subtests for delimiter conflict detection (nil, empty, single-element, default, custom with quotes)TestEnsureDoubleQuotedForDelimiterSafety— 6 subtests for yaml.Node style modification (scalars, mappings, sequences, document nodes, nil safety)TestConvertToYAMLPreservingDelimiters— 10 subtests for end-to-end serialization (round-trip value preservation, template replacement producing valid YAML, standard encoding breaking with custom delimiters, nested maps, lists, custom indent, empty data)TestAllYAMLFunctionsPreservedWithCustomDelimiters— 12 subtests verifying every YAML function prefix (!terraform.state,!terraform.output,!store,!store.get,!env,!exec,!template,!include,!include.raw,!repo-root,!cwd,!random)TestAllYAMLFunctionsTemplateReplacementWithCustomDelimiters— 9 subtests simulating the full pipeline (serialize → template replace → parse) for each functionTestStandardEncodingBreaksAllYAMLFunctionsWithCustomDelimiters— 18 subtests proving standard encoding breaks AND delimiter-safe encoding works for each of the 9 affected functionsIntegration tests (
tests/yaml_functions_custom_delimiters_test.go) — 4 subtests:TestTerraformStateWithCustomDelimiters— Regular templates with custom delimiters,!terraform.statewith static args,!terraform.statewith templated stack arg (core issue reproduction)TestCustomDelimitersTemplateProcessing— Settings template resolution with custom delimitersTest fixture (
tests/fixtures/scenarios/atmos-terraform-state-custom-delimiters/):atmos.yamlwith custom delimiters["'{{", "}}'"]!terraform.statewith templated argsreferences
!terraform.statefails with templated arguments when custom delimiters include quotes #2052Summary by CodeRabbit
Bug Fixes
Documentation
Tests
Chores