fix: resolve stack context for locals enabling !terraform.state and other YAML functions#2207
Merged
fix: resolve stack context for locals enabling !terraform.state and other YAML functions#2207
locals enabling !terraform.state and other YAML functions#2207Conversation
The 2-arg form of !terraform.state in locals failed with "stack is required" because extractLocalsFromRawYAML passed an empty string as the current stack. Add deriveStackNameForLocals() and computeStackFileName() to derive the stack name from the file path, vars, and atmos config before processing locals. Includes unit tests with mock StateGetter, integration tests for Go template conditionals with !env, a new locals-conditional fixture, and comprehensive architecture documentation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add Sprig functions (pipe upper), multiple components (myapp-worker with suffixed full_name), and file-scoped isolation commentary. Update README with feature sections and try-it commands. Add TestExampleLocalsWorkerComponent test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n details Add comprehensive documentation for locals processing pipeline, cross-component references with !terraform.state, environment variable conditionals with !env, and updated best practices. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
Dependency ReviewThe following issues were found:
License Issuesgo.mod
Scanned Files
|
Contributor
📝 WalkthroughWalkthroughDerives and propagates a stack name when processing file-scoped locals so YAML functions that require a stack (e.g., 2-arg !terraform.state) can resolve. Adds deriveStackNameForLocals and computeStackFileName helpers, wires derived stack into ProcessStackLocals, adds tests, fixtures, docs, example updates, dependency bumps, and a small import cleanup. Changes
Sequence Diagram(s)mermaid 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)
📝 Coding Plan
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 Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
…n locals docs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
aknysh
changed the title
!terraform.state works!terraform.state and YAML functions
![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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
NOTICE (1)
1-1757:⚠️ Potential issue | 🟠 MajorRegenerate
NOTICEbefore merge.The dependency review workflow is failing because this file is still out of sync with the generator output. Please run
./scripts/generate-notice.shand commit the regenerated file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@NOTICE` around lines 1 - 1757, The NOTICE file is out of sync with the generator; run the provided generator script (./scripts/generate-notice.sh) to regenerate the NOTICE contents, verify the updated NOTICE matches the generator output, add the regenerated NOTICE to the commit, and push the change so the dependency review workflow passes.
🧹 Nitpick comments (1)
website/docs/stacks/locals.mdx (1)
43-43: Fix component type typo:absible→ansible.Line 43 and Line 58 contain a misspelling that can mislead users.
✏️ Suggested doc fix
-1. Parse raw YAML → extract locals sections (global, terraform, helmfile, packer, absible, etc.) +1. Parse raw YAML → extract locals sections (global, terraform, helmfile, packer, ansible, etc.) -Global locals → Component-type locals (terraform/helmfile/packer/absible) → Component-level locals +Global locals → Component-type locals (terraform/helmfile/packer/ansible) → Component-level localsAlso applies to: 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/stacks/locals.mdx` at line 43, Replace the misspelled component type "absible" with the correct "ansible" in the locals list occurrences (e.g., the line containing "Parse raw YAML → extract locals sections (global, terraform, helmfile, packer, absible, etc.)" and the second occurrence around the later locals list); update both instances so the list reads "...packer, ansible, etc." to correct the documentation.
🤖 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-03-15-locals-terraform-state-missing-stack-context.md`:
- Around line 122-132: The two bullets under "File-Scoped Locals (Global +
Section)" and "Component-Level Locals" contradict each other about precedence;
standardize on component-level locals overriding file-scoped locals by changing
the file-scoped statement "Cannot be overridden by individual components." to
"Can be overridden by component-level locals." and ensure the "Component-Level
Locals" block keeps "Override file-scoped locals." and any surrounding
explanatory text or examples consistently reflect that precedence for
File-Scoped Locals and Component-Level Locals.
In `@examples/locals/README.md`:
- Line 15: Update the bullet "File-scoped isolation — dev.yaml and prod.yaml
have independent locals" by adding a terminal period at the end so it reads
"File-scoped isolation — dev.yaml and prod.yaml have independent locals." to
match punctuation consistency in the README.
In `@examples/locals/stacks/deploy/dev.yaml`:
- Around line 49-50: The comment claiming "Section-level locals (under
terraform:) inherit from global locals. This shows that you can also define
locals here." is misleading because there is no actual locals: block; either
update the comment to accurately describe that this section adds another
component rather than demonstrating locals, or add a real locals: block under
terraform: to match the comment; locate the terraform: section in the file and
either change the comment text to reflect adding a component or insert a locals:
mapping (e.g., example variable definitions) under terraform: so the example and
comment align.
In `@internal/exec/stack_processor_utils_test.go`:
- Around line 2621-2644: Replace hardcoded Unix-style test paths with
OS-agnostic paths by building them with filepath.Join where the test cases set
the filePath field; e.g., update test entries that set filePath:
"/project/stacks/..." (including cases named "nested org path", "yml extension"
and others around those ranges) to construct the same path using
filepath.Join("projectRoot", "stacks", ...) or filepath.Join("/project")
equivalents and join the filename with the extension using filepath.Join so
tests don't assume forward slashes; ensure expected values remain computed with
filepath.Join as in the test and only modify the filePath test inputs.
In `@internal/exec/stack_processor_utils.go`:
- Around line 146-152: computeStackFileName currently only strips the final file
extension so names like "deploy/dev.yaml.tmpl" become "deploy/dev.yaml"; change
the logic in computeStackFileName to iteratively remove known extensions (e.g.,
".tmpl", ".yaml", ".yml", ".json", etc.) by looping with filepath.Ext on rel and
trimming the extension while the ext is in the allowed removable set, ensuring
rel ends up with the base stack name; reference the rel and ext variables in
computeStackFileName and preserve use of filepath functions (no hardcoded
separators) when handling paths.
---
Outside diff comments:
In `@NOTICE`:
- Around line 1-1757: The NOTICE file is out of sync with the generator; run the
provided generator script (./scripts/generate-notice.sh) to regenerate the
NOTICE contents, verify the updated NOTICE matches the generator output, add the
regenerated NOTICE to the commit, and push the change so the dependency review
workflow passes.
---
Nitpick comments:
In `@website/docs/stacks/locals.mdx`:
- Line 43: Replace the misspelled component type "absible" with the correct
"ansible" in the locals list occurrences (e.g., the line containing "Parse raw
YAML → extract locals sections (global, terraform, helmfile, packer, absible,
etc.)" and the second occurrence around the later locals list); update both
instances so the list reads "...packer, ansible, etc." to correct the
documentation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 253dd494-e3ce-4198-b086-4d978a23b98c
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
NOTICEdocs/fixes/2026-03-15-locals-terraform-state-missing-stack-context.mdexamples/locals/README.mdexamples/locals/components/terraform/myapp/main.tfexamples/locals/stacks/deploy/dev.yamlexamples/locals/stacks/deploy/prod.yamlexamples/quick-start-advanced/Dockerfilego.modinternal/exec/stack_processor_utils.gointernal/exec/stack_processor_utils_test.gointernal/terraform_backend/terraform_backend_s3.gopkg/ai/analyze/analyze_test.gopkg/devcontainer/lifecycle_rebuild_test.gotests/cli_locals_test.gotests/fixtures/scenarios/locals-conditional/atmos.yamltests/fixtures/scenarios/locals-conditional/components/terraform/mock/main.tftests/fixtures/scenarios/locals-conditional/stacks/deploy/pr-empty.yamltests/fixtures/scenarios/locals-conditional/stacks/deploy/pr-set.yamlwebsite/docs/stacks/locals.mdx
💤 Files with no reviewable changes (1)
- internal/terraform_backend/terraform_backend_s3.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2207 +/- ##
=======================================
Coverage 77.40% 77.40%
=======================================
Files 962 962
Lines 91284 91318 +34
=======================================
+ Hits 70657 70685 +28
+ Misses 16550 16549 -1
- Partials 4077 4084 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Handle .yaml.tmpl/.yml.tmpl extensions in computeStackFileName (longest suffix first) - Add test cases for .yaml.tmpl, .yml.tmpl, no extension, and unknown extension - Use filepath.Join() for all test paths (cross-platform compliance) - Fix absible → ansible typo in locals docs - Fix misleading comment in dev.yaml example - Fix override semantics contradiction in fix doc Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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-03-15-locals-terraform-state-missing-stack-context.md`:
- Around line 488-503: The test summary counts in the docs are stale: update the
listed counts for TestComputeStackFileName, TestDeriveStackNameForLocals, and
the integration tests (TestLocalsGoTemplateConditionalWithEnvSet,
TestLocalsGoTemplateConditionalWithEnvEmpty) to match the actual test
definitions or replace numeric counts with non-numeric wording (e.g., "several
cases" or "multiple scenarios") so they won't drift; specifically ensure the
entries for TestComputeStackFileName, TestDeriveStackNameForLocals,
TestExtractLocalsFromRawYAML_* and the two TestLocalsGoTemplateConditional*
integration tests reflect current case counts or use the non-numeric phrasing.
In `@website/docs/stacks/locals.mdx`:
- Around line 42-59: The docs currently list "ansible" as part of the locals
resolution pipeline and scope chain but the implementation only resolves global
+ terraform + helmfile + packer; update the text in the locals.mdx content to
either remove "ansible" from the pipeline list and the scope chain example (the
lines showing "global, terraform, helmfile, packer, ansible, etc." and "Global
locals → Component-type locals (terraform/helmfile/packer/ansible) →
Component-level locals") or annotate "ansible" as not yet supported/planned, so
the documented pipeline matches the actual behavior used by the locals
resolution code path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c0e54922-8b3c-4ad4-bc56-473ef3cb8786
📒 Files selected for processing (5)
docs/fixes/2026-03-15-locals-terraform-state-missing-stack-context.mdexamples/locals/stacks/deploy/dev.yamlinternal/exec/stack_processor_utils.gointernal/exec/stack_processor_utils_test.gowebsite/docs/stacks/locals.mdx
docs/fixes/2026-03-15-locals-terraform-state-missing-stack-context.md
Outdated
Show resolved
Hide resolved
- Update TestComputeStackFileName case count from 4 to 8 in fix doc - Remove ansible from locals pipeline/scope docs (only terraform/helmfile/packer supported) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
website/docs/stacks/locals.mdx (1)
34-34: Minor: Cycle path format differs from actual error output.The docs show
a -> b -> c -> awith ASCII arrows, but the actual error message uses Unicode arrows (a → b → c → a) performatCyclePath()in the resolver. Consider aligning for consistency when users compare error output to docs.Suggested fix
-3. **Cycle Detection:** Circular references are detected and reported with clear error messages showing the dependency path (e.g., `a -> b -> c -> a`). +3. **Cycle Detection:** Circular references are detected and reported with clear error messages showing the dependency path (e.g., `a → b → c → a`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/stacks/locals.mdx` at line 34, The docs currently show cycle paths with ASCII arrows but the runtime uses Unicode arrows from formatCyclePath(); update the example in locals.mdx to match the actual error output (e.g., change `a -> b -> c -> a` to `a → b → c → a`) so users see the same format as produced by the resolver's formatCyclePath() function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@website/docs/stacks/locals.mdx`:
- Line 34: The docs currently show cycle paths with ASCII arrows but the runtime
uses Unicode arrows from formatCyclePath(); update the example in locals.mdx to
match the actual error output (e.g., change `a -> b -> c -> a` to `a → b → c →
a`) so users see the same format as produced by the resolver's formatCyclePath()
function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e817d9a0-01c5-4b3e-8ec8-ab27876f68d1
📒 Files selected for processing (2)
docs/fixes/2026-03-15-locals-terraform-state-missing-stack-context.mdwebsite/docs/stacks/locals.mdx
aknysh
changed the title
!terraform.state and YAML functionslocals enabling !terraform.state and other YAML functions
osterman
approved these changes
Mar 16, 2026
osterman
pushed a commit
that referenced
this pull request
Mar 16, 2026
…nd other YAML functions (#2207) * updates * fix: derive stack name for locals so !terraform.state works (#2080) The 2-arg form of !terraform.state in locals failed with "stack is required" because extractLocalsFromRawYAML passed an empty string as the current stack. Add deriveStackNameForLocals() and computeStackFileName() to derive the stack name from the file path, vars, and atmos config before processing locals. Includes unit tests with mock StateGetter, integration tests for Go template conditionals with !env, a new locals-conditional fixture, and comprehensive architecture documentation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update locals example to showcase all features concisely Add Sprig functions (pipe upper), multiple components (myapp-worker with suffixed full_name), and file-scoped isolation commentary. Update README with feature sections and try-it commands. Add TestExampleLocalsWorkerComponent test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update locals documentation with processing pipeline and YAML function details Add comprehensive documentation for locals processing pipeline, cross-component references with !terraform.state, environment variable conditionals with !env, and updated best practices. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * update docs * [autofix.ci] apply automated fixes * Fix broken link and add Gomplate/Atmos template function references in locals docs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Improve test coverage and address PR review feedback - Handle .yaml.tmpl/.yml.tmpl extensions in computeStackFileName (longest suffix first) - Add test cases for .yaml.tmpl, .yml.tmpl, no extension, and unknown extension - Use filepath.Join() for all test paths (cross-platform compliance) - Fix absible → ansible typo in locals docs - Fix misleading comment in dev.yaml example - Fix override semantics contradiction in fix doc Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address PR review: fix test count, remove ansible from locals scopes - Update TestComputeStackFileName case count from 4 to 8 in fix doc - Remove ansible from locals pipeline/scope docs (only terraform/helmfile/packer supported) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
|
These changes were released in v1.210.0-test.28. |
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(2-arg form) inlocalsnow correctly derives the stack name from the file path, fixing the "stack is required" error reported in!terraform.statein catalog filelocalsfails with "stack is required" error #2080stack_processor_utils_test.gocoveringcomputeStackFileName,deriveStackNameForLocals,!env+ Go template conditionals, and!terraform.statemock-based tests (2-arg and 3-arg forms)cli_locals_test.gofor Go template conditionals with!env(set/empty) and the worker component exampletests/fixtures/scenarios/locals-conditional/with isolated stack files and unique env var names to avoid stack cache interferenceexamples/locals/updated to showcase all locals features concisely — basic locals, dependency resolution, settings/vars access, Sprig pipe functions, complex maps, and multiple components (myapp+myapp-worker)website/docs/stacks/locals.mdxrewritten with processing pipeline details, cross-component references (!terraform.state2-arg and 3-arg forms), environment variable conditionals (!env+ Go templateif), Gomplate/Atmos template function support, and updated best practicesdocs/fixes/2026-03-15-locals-terraform-state-missing-stack-context.mdwith root cause analysis, architecture details, and test resultswhy
extractLocalsFromRawYAML()instack_processor_utils.gopassed""ascurrentStacktoProcessStackLocals. The 2-arg form of!terraform.state(e.g.,!terraform.state vpc .vpc_id) usescurrentStackas the stack name, so it failed with "stack is required" when used insidelocalsderiveStackNameForLocals()andcomputeStackFileName()functions that derive the stack name from the file path using the same logic asdescribe locals, making the 2-arg form work correctly in localsreferences
!terraform.statein catalog filelocalsfails with "stack is required" error #2080!terraform.stateYAML function docs at https://atmos.tools/functions/yamlSummary by CodeRabbit
Bug Fixes
Documentation
New Features / Examples
Tests
Chores