fix: preserve newlines in terraform.output YAML function
what
- Fixed the
!terraform.outputYAML function stripping newlines from multiline strings - Modified
getTerraformOutputVariableto bypass yq processing for simple output retrieval - Updated
getValueWithTagto preserve whitespace in YAML values - Added comprehensive tests to verify newline preservation
why
The !terraform.output YAML function was stripping newlines from multiline terraform outputs due to yq processing that converted them to flow scalars. This made it impossible to properly consume multiline terraform outputs (like SSH keys, certificates, or formatted text) from one component in another.
When a terraform component outputs:
"bar\nbaz\nbongo\n"
And another component consumes it via:
vars:
foo: !terraform.output component-a stack-a foo
The value was incorrectly becoming "bar baz bongo" instead of preserving the original newlines.
Solution Details
The root cause was in the getTerraformOutputVariable function in terraform_output_utils.go. It was using EvaluateYqExpression for all output retrievals, which internally converts multiline strings to flow scalars (space-separated).
The fix:
- For simple outputs (no dot-notation paths): Directly return the value from the outputs map, bypassing yq entirely
-
For complex paths (e.g.,
output.nested.value): Continue using yq for path traversal - Preserve whitespace in YAML tag values by not trimming them unnecessarily
This maintains backward compatibility for complex path queries while fixing the newline preservation issue for simple outputs.
Testing
- Added unit test
TestYamlFuncTerraformOutputWithNewlinesto verify newline preservation in the YAML function processing - Added integration test
TestDEV2982TerraformOutputNewlineIssuethat reproduces the exact scenario from the issue report - All existing tests continue to pass
Other YAML Functions
Investigated other YAML functions for similar issues:
-
!env: Not affected - usesos.LookupEnvwhich preserves the original value -
!includeand!include.raw: Not affected - properly handles file content through appropriate parsing
Fixes #1482 Fixes LINEAR-DEV-2982
Summary by CodeRabbit
-
Bug Fixes
- Preserve newlines in terraform.output values and YAML tag processing; avoid unintended whitespace trimming.
-
Performance
- Faster simple-key lookups by bypassing extra processing for direct matches.
-
Improvements
- Standardized logging key for Terraform outputs and enhanced debug messages for multiline values (including length hints).
-
Tests
- Added extensive unit and integration tests covering newline preservation, YAML parsing/encoding, and whitespace handling.
- Added precondition helper to skip Terraform-dependent tests when Terraform isn’t available.
📝 Walkthrough
Walkthrough
Standardizes logging keys for terraform output handling, adds a constant outputLogKey, introduces a fast-path direct-key retrieval to bypass yq for simple keys, improves debug logging for multiline output, narrows whitespace trimming to include-related YAML tags, and adds many unit/integration tests validating newline preservation and a Terraform-in-PATH test helper.
Changes
| Cohort / File(s) | Summary |
|---|---|
Terraform output utilitiesinternal/exec/terraform_output_utils.go |
Added outputLogKey constant; replaced literal "output" uses with the constant; added multiline-result debug logging; added direct simple-key fast path in GetTerraformOutput, getTerraformOutputVariable, and GetStaticRemoteStateOutput to return values without yq when key has no dot-notation. |
YAML utils corepkg/utils/yaml_utils.go |
Preserve node whitespace by default; removed unconditional TrimSpace in getValueWithTag; perform trimming only just-in-time for include/include.raw handling; updated comments and flow to keep whitespace/newlines unless include processing requires trimming. |
Exec YAML func testsinternal/exec/yaml_func_terraform_output_newline_test.go |
Added unit test TestYamlFuncTerraformOutputWithNewlines and an integration-skipping scaffold TestYamlFuncTerraformOutputIntegration to validate newline preservation and skip when Terraform not present. |
YAML utils newline testspkg/utils/yaml_utils_newline_test.go |
Added tests targeting getValueWithTag and processCustomTags to detect and prevent newline/whitespace stripping. |
Terraform output integration/debug teststests/debug_terraform_output_test.go, tests/terraform_output_newlines_integration_test.go, tests/terraform_output_preserve_newlines_test.go |
Added integration and debug tests that create temporary Terraform components/stacks, run describe/deploy flows, and assert that terraform.output values preserve newlines across stacks and component consumption; include environment checks and skips for missing tooling. |
YAML parsing behavior teststests/yaml_flow_scalar_test.go, tests/yaml_literal_newline_test.go, tests/yaml_tag_reconstruction_test.go, tests/yaml_tag_value_test.go |
Added tests covering YAML flow scalars, literal/folded styles, tag reconstruction, and verification that replacing tagged scalars preserves multiline values through marshal/unmarshal. |
YQ behavior testspkg/utils/yq_test.go |
Added helpers and tests documenting current yq read/write whitespace behaviors against multiline and block scalar cases; observe and log behavior rather than assert idealized formatting. |
Test preconditionstests/test_preconditions.go |
Added RequireTerraformInPath(t *testing.T) helper to skip tests when terraform binary is not in PATH. |
Sequence Diagram(s)
sequenceDiagram
autonumber
actor User
participant CLI as Atmos CLI
participant TF as Terraform Output Utils
participant Cache as Output Cache
participant YQ as yq Processor
User->>CLI: Request terraform.output(component, key)
CLI->>TF: GetTerraformOutput(component, key)
TF->>Cache: check cached value for component:key
alt cached
Cache-->>TF: cached value
TF-->>CLI: return value (as-is, newlines preserved)
else not cached
alt key is simple (no dot)
TF->>TF: execTerraformOutput(component) or direct lookup
TF-->>CLI: return raw value (bypass yq, preserve formatting)
else key is complex (path/dot-notation)
TF->>YQ: run yq extraction on output JSON/YAML
YQ-->>TF: extracted value
TF-->>CLI: return value
end
end
sequenceDiagram
autonumber
participant YAML as YAML Utils
participant Tag as getValueWithTag
participant Incl as Include Handler
YAML->>Tag: getValueWithTag(node)
Note right of Tag: Return tag + original node.Value (preserve whitespace)
alt include / include.raw
Tag->>Incl: processInclude(trimmed node.Value)
Incl-->>YAML: included content
else other tags
Tag-->>YAML: value with original whitespace/newlines
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- cloudposse/atmos#1252 — Related changes to
terraform.outputhandling and tests; strong overlap in feature area and test coverage. - cloudposse/atmos#1506 — Also modifies
pkg/utils/yaml_utils.goand custom tag processing; relevant to the whitespace/tag changes here. - cloudposse/atmos#1048 — Touches
execTerraformOutput-related code; overlaps with logging/exec paths updated in this change.
Suggested reviewers
- aknysh
- milldr
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
âś… Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title Check | ✅ Passed | The title succinctly and accurately describes the primary change—fixing newline preservation in the terraform.output YAML function—using clear, focused language without extraneous detail. |
| Linked Issues Check | âś… Passed | The implementation adds a fast-path in getTerraformOutputVariable to return raw multiline output, updates getValueWithTag to avoid trimming whitespace, and includes targeted unit and integration tests to confirm newline retention, directly satisfying the behaviors and repro steps outlined in issues #1482 and DEV-2982. |
| Out of Scope Changes Check | ✅ Passed | All modifications—including the constant refactor in terraform_output_utils.go, the YAML whitespace handling updates in yaml_utils.go, and the accompanying helper and test files—are aligned with preserving newlines for terraform.output and validating that behavior without introducing unrelated features. |
✨ Finishing touches
- [ ] 📝 Generate Docstrings
đź§Ş Generate unit tests
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
- [ ] Commit unit tests in branch
feature/dev-2982-terraformoutput-yaml-function-doesnt-not-handle-newlines
📜 Recent review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📥 Commits
Reviewing files that changed from the base of the PR and between 8bfe7b6a6a2be3b7aabe24a66368f90cca565e8b and 48c3998f01e5521389eac1faf65979f43d9267fa.
đź“’ Files selected for processing (2)
-
internal/exec/terraform_output_utils.go(7 hunks) -
pkg/utils/yaml_utils.go(3 hunks)
đźš§ Files skipped from review as they are similar to previous changes (1)
- internal/exec/terraform_output_utils.go
đź§° Additional context used
đź““ Path-based instructions (3)
pkg/**/*.go
đź“„ CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Target >80% coverage for packages, focusing on pkg/ and internal/exec/.
Files:
-
pkg/utils/yaml_utils.go
**/*.go
đź“„ CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks Follow Go error handling idioms and use meaningful error messages Wrap errors with context using fmt.Errorf("context: %w", err) Consider custom error types for domain-specific errors Follow standard Go coding style; run gofmt and goimports Use snake_case for environment variables Document complex logic with inline comments
**/*.go: All comments must end with periods. Wrap all returned errors with static errors from errors/errors.go; never return dynamic errors directly. Use fmt.Errorf with %w to wrap the static error first, then add context details. Always bind environment variables via viper.BindEnv, providing an ATMOS_ alternative for each external var. All new configurations must support Go templating using the shared FuncMap(); test template rendering with various contexts. Prefer SDKs over shelling out to binaries; use standard library for cross-platform behavior (filepath.Join, os.PathSeparator, runtime.GOOS). For non-standard execution paths, capture telemetry via telemetry.CaptureCmd or telemetry.CaptureCmdString without user data.
Files:
-
pkg/utils/yaml_utils.go
**/!(*_test).go
đź“„ CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
-
pkg/utils/yaml_utils.go
đź§ Learnings (2)
📚 Learning: 2025-04-24T01:40:13.576Z
Learnt from: haitham911
PR: cloudposse/atmos#1202
File: pkg/config/process_yaml.go:0-0
Timestamp: 2025-04-24T01:40:13.576Z
Learning: When processing YAML nodes with custom directives in Go using gopkg.in/yaml.v3, setting node.Tag = "" is sufficient to prevent re-processing of the node. It's not necessary to also clear node.Value after updating the configuration store (e.g., Viper), as the original value doesn't affect subsequent operations once the tag is removed.
Applied to files:
-
pkg/utils/yaml_utils.go
📚 Learning: 2025-09-26T05:52:03.918Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T05:52:03.918Z
Learning: Applies to **/*.go : All comments must end with periods.
Applied to files:
-
pkg/utils/yaml_utils.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
🔇 Additional comments (3)
pkg/utils/yaml_utils.go (3)
175-176: Comments still missing terminal periods.The past review correctly flagged these comments for missing periods. The provided code still shows:
// Only trim value when it's needed for file paths or includes // For custom tags that will be processed, preserve the original valueBoth lines need periods at the end to satisfy godot lint rules.
Apply this diff:
- // Only trim value when it's needed for file paths or includes - // For custom tags that will be processed, preserve the original value + // Only trim value when it's needed for file paths or includes. + // For custom tags that will be processed, preserve the original value.Based on coding guidelines.
187-189: Just-in-time trimming logic is correct; add period to comment.The trimming strategy is sound—deferring
TrimSpaceuntil it's actually needed for file paths preserves newlines for other tags. However, line 187 needs a period:- // For include tags, we need to trim the value as it's a file path + // For include tags, we need to trim the value as it's a file path.Based on coding guidelines.
214-218: Whitespace preservation is correct; comments need periods.The logic correctly preserves newlines by returning the raw
n.Valuewithout trimming. However, lines 216-217 still lack terminal periods despite the past review:- // Don't trim the value as it may contain intentional whitespace/newlines - // The value will be processed by the YAML function handlers which will handle it appropriately + // Don't trim the value as it may contain intentional whitespace/newlines. + // The value will be processed by the YAML function handlers which will handle it appropriately.Based on coding guidelines.
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.
đź§Ş Early access (Sonnet 4.5): enabled
We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.
Note:
- Public repositories are always opted into early access features.
- You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.
Comment @coderabbitai help to get the list of available commands and usage tips.
Codecov Report
:x: Patch coverage is 72.00000% with 14 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 58.47%. Comparing base (f51d0f9) to head (b9c32a7).
:warning: Report is 209 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| internal/exec/terraform_output_utils.go | 69.69% | 10 Missing :warning: |
| tests/test_preconditions.go | 60.00% | 4 Missing :warning: |
:x: Your patch check has failed because the patch coverage (72.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.
Additional details and impacted files
@@ Coverage Diff @@
## main #1517 +/- ##
==========================================
+ Coverage 58.38% 58.47% +0.09%
==========================================
Files 285 285
Lines 31466 31508 +42
==========================================
+ Hits 18371 18424 +53
+ Misses 11195 11182 -13
- Partials 1900 1902 +2
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 58.47% <72.00%> (+0.09%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Files with missing lines | Coverage Δ | |
|---|---|---|
| pkg/utils/yaml_utils.go | 63.08% <100.00%> (+1.28%) |
:arrow_up: |
| tests/test_preconditions.go | 77.53% <60.00%> (-0.81%) |
:arrow_down: |
| internal/exec/terraform_output_utils.go | 51.63% <69.69%> (+1.63%) |
:arrow_up: |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏
[!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. Consider refactoring it into smaller, more focused PRs to facilitate a smoother review process.
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏