Conversation
|
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 Review✅ No vulnerabilities or license issues found.Scanned Files
|
Contributor
📝 WalkthroughWalkthroughThreads a per-file template context through locals extraction/resolution, adds pluggable YAML-function processing into the locals resolver, updates signatures and error handling, and expands tests, fixtures, examples, and docs. Dependency NOTICE/go.mod entries were bumped. Changes
Sequence Diagram(s)sequenceDiagram
participant Processor as Stack Processor
participant StackUtil as Stack Utils
participant Locals as Locals Extractor
participant Resolver as Template Resolver
participant Output as Resolved Values
Processor->>StackUtil: extractAndAddLocalsToContext(rawYAML, filePath)
StackUtil->>Locals: extractLocalsFromRawYAML(yamlContent)
Locals->>Locals: parse locals, settings, vars, env -> extractLocalsResult
Locals-->>StackUtil: return extractLocalsResult
StackUtil->>Processor: inject extractLocalsResult into templateContext
Processor->>Processor: buildSectionTemplateContext(globalContext, sectionOverrides)
Processor->>Resolver: NewResolver().WithTemplateContext(mergedContext).WithYamlFunctionProcessor(processor)
Resolver->>Resolver: if YAML-function tag -> call yamlFunctionProcessor(value)
Resolver->>Resolver: render templates with mergedContext (locals take precedence)
Resolver-->>Output: final resolved locals/vars for components
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1994 +/- ##
==========================================
+ Coverage 74.93% 74.97% +0.04%
==========================================
Files 775 775
Lines 71355 71464 +109
==========================================
+ Hits 53471 53582 +111
Misses 14398 14398
+ Partials 3486 3484 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
📝 WalkthroughWalkthroughThis PR extends the locals resolution pipeline to support file-scoped template context, enabling locals to reference .settings, .vars, and .env from the same file during template processing. It introduces templateContext propagation through the stack processor and locals resolver, adds a new helper type for extracting multiple context sections, and bumps the doublestar dependency to v4.9.2. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Stack Processor
participant Extractor as Utils (extractLocalsFromRawYAML)
participant Resolver as Locals Resolver
participant TemplateEngine as Template Engine
Client->>Extractor: Extract locals, settings, vars, env from YAML
Extractor-->>Client: Return extractLocalsResult
Client->>Client: Build templateContext from config + extracted sections
Client->>Resolver: ResolveComponentLocals with templateContext
Resolver->>Resolver: Set templateContext via WithTemplateContext()
loop Resolve each local with dependencies
Resolver->>TemplateEngine: Execute template with merged context<br/>(locals + settings + vars + env)
TemplateEngine-->>Resolver: Resolved value
end
Resolver->>Resolver: Merge resolved locals with templateContext<br/>(locals take precedence)
Resolver-->>Client: Fully resolved locals map
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
🤖 Fix all issues with AI agents
In `@internal/exec/stack_processor_locals.go`:
- Around line 186-211: buildSectionTemplateContext currently replaces the entire
.settings/.vars/.env maps when a section provides them, losing global keys;
change it to shallow-merge the section maps onto the existing global maps
instead. In buildSectionTemplateContext, for each of cfg.SettingsSectionName,
cfg.VarsSectionName, and cfg.EnvSectionName: if the section map exists, get the
existing map from context (if any, ensure it's a map[string]any), copy its
key/value pairs into a new map, then copy/overwrite with keys from the section
map and assign that merged map back to context[...] so section keys override but
unspecified global keys remain. Use the function name
buildSectionTemplateContext and the symbols cfg.SettingsSectionName,
cfg.VarsSectionName, cfg.EnvSectionName to locate where to apply the merge.
In `@NOTICE`:
- Around line 1075-1077: The NOTICE file was edited manually; instead regenerate
it by running the provided script and committing the generated output: execute
./scripts/generate-notice.sh (or the repository's canonical generation command),
verify the NOTICE changes are as expected, and commit the updated NOTICE rather
than making manual edits—do not edit NOTICE by hand going forward.
🧹 Nitpick comments (2)
tests/cli_locals_test.go (1)
900-935: Consider strengthening the cross-file test assertion.This test verifies that cross-file access doesn't cause errors, but it doesn't explicitly verify that the cross-file pattern fails as expected. The test passes as long as
test-same-fileworks, but doesn't validate that a template referencing imported settings would produce<no value>or an error.For stronger coverage, consider adding a test case that explicitly attempts cross-file settings access and verifies the expected failure mode.
internal/exec/stack_processor_utils_test.go (1)
1726-1818: Consider table-driving the new context-access cases.The settings/vars/env/combined tests share the same setup and assertion pattern; a table-driven test would reduce duplication and make it easier to add cases. As per coding guidelines.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/exec/stack_processor_utils.go (1)
116-174: Prevent parent .settings/.vars/.env from leaking into imported files.Only locals are cleared from context, so settings/vars/env from a parent file can persist into imports and trigger template processing or cross-file access unexpectedly. Clear these keys alongside locals to preserve file-scoped behavior.
Suggested fix
if context != nil { delete(context, "locals") + delete(context, cfg.SettingsSectionName) + delete(context, cfg.VarsSectionName) + delete(context, cfg.EnvSectionName) }
🤖 Fix all issues with AI agents
In `@NOTICE`:
- Around line 1075-1077: The NOTICE file was modified manually and is out of
date; regenerate it using the project's generation script instead of editing
NOTICE directly: run ./scripts/generate-notice.sh to produce an updated NOTICE,
review and commit the generated output, and remove any manual changes
referencing entries like "github.com/bmatcuk/doublestar/v4" so CI will pass.
In `@website/docs/stacks/locals.mdx`:
- Line 229: The sentence "This architecture ensures that locals resolution is
fast, predictable, and free of cross-file dependencies" is missing a terminal
period; update the sentence in locals.mdx (the line containing that exact text)
to end with a period so it reads "...dependencies." to maintain consistent
punctuation.
🧹 Nitpick comments (1)
tests/cli_locals_test.go (1)
900-935: Consider strengthening the cross-file access test.The test comment mentions that cross-file access "would fail or return
<no value>", but the assertions only verify that same-file access works. For stronger validation of the file-scoped design, consider adding a fixture that explicitly attempts cross-file settings access and asserting the expected failure behavior (similar toTestLocalsNotInheritedwhich checks for<no value>).That said, if the intent is just to confirm the happy path works while cross-file scenarios are covered elsewhere, this is fine as-is.
github-actions
bot
added
size/xl
|
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. |
|
Warning Release Documentation RequiredThis PR is labeled
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@internal/exec/stack_processor_locals.go`:
- Around line 112-126: The YAML function processor in
createYamlFunctionProcessor currently returns raw errors and nil for missing
keys; update it to wrap any returned error and the missing-_value case with the
sentinel error defined in errors/errors.go (use the package-level sentinel, e.g.
errors.ErrSentinel), e.g., when ProcessCustomYamlTags returns err wrap it with
fmt.Errorf("%w: %v", errors.ErrSentinel, err) and when "_value" is absent return
errors.ErrSentinel (or wrap a descriptive message with the sentinel) from
createYamlFunctionProcessor so all failures conform to the project's
error-wrapping guideline.
In `@internal/exec/stack_processor_utils.go`:
- Around line 155-160: The code currently returns early when
len(extractResult.locals) == 0, which skips template context setup even if a
locals: {} section existed; change the logic to detect presence of the locals
section rather than its resolved size by adding a boolean flag on the extraction
result (e.g., ExtractResult.LocalsDeclared or a method like
ExtractResult.HasLocalsField) and use that here (and consistently in
ProcessBaseStackConfig) to decide whether to enable template processing; update
the extractor that populates extractResult to set this flag when the locals
block is present so template context is created even when resolve yields an
empty map.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pkg/locals/resolver.go`:
- Around line 466-470: Replace the raw fmt.Errorf return in the locals resolver
with a wrapped sentinel error so callers can errors.Is() it; import and use the
package-level sentinel (e.g., errors.ErrLocalResolution or add that sentinel in
errors/errors.go if missing) and wrap the detailed fmt.Errorf message with
fmt.Errorf("%w: ...", errors.ErrLocalResolution, ...) while keeping the existing
context (use r.getAvailableLocals(), r.getAvailableContext(), localName,
r.filePath and the original err) so the returned error both preserves details
and matches the static sentinel.
In `@tests/cli_locals_test.go`:
- Around line 1092-1093: Update the comment describing the output of
ExecuteDescribeLocals to accurately reflect that it returns root-level locals
plus per-section locals (terraform, helmfile, packer) rather than a components.*
tree; locate the comment near the test for ExecuteDescribeLocals in
cli_locals_test.go and replace the incorrect
"components.terraform.<component>.locals" description with a note that the
structure includes a top-level "locals:" and separate "terraform:", "helmfile:",
and "packer:" sections containing their respective locals.
- Reduced stacks/locals.mdx from 827 to 331 lines (60% cut)
- Consolidated Configuration Scopes (3 examples → 1 comprehensive example)
- Moved Processing Order diagram to <details> block ("How does processing work?")
- Deleted redundant "Complete Example" section, rely on "Try It" embed instead
- Removed duplicate explanations of file-scoped isolation across sections
- Moved debugging output examples to <details> block
- Condensed error handling section and best practices
- Removed "Scope Merging", "Limitations", "When to Use Each" sections as redundant
- Reduced examples/locals/README.md from 155 to 35 lines (77% cut)
- Aligned with standard example README pattern (demo-stacks/README.md style)
- Removed 60-line "Example Output" section
- Removed 40-line "Key Concepts" section duplicating main docs
- Fixed broken documentation link (/core-concepts/stacks/locals → /stacks/locals)
Locals is a minor feature with key gotchas. Documentation now appropriately sized.
Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@website/docs/stacks/locals.mdx`:
- Around line 167-169: Update the Docusaurus admonition title syntax by
replacing the bracketed title token ":::warning[Same-File Access Only]" with the
inline form ":::warning Same-File Access Only" so the admonition type and title
are separated by a space; locate the admonition that currently reads
":::warning[Same-File Access Only]" in locals.mdx and change it to the inline
title format for proper rendering.
osterman
approved these changes
Jan 21, 2026
|
Warning Release Documentation RequiredThis PR is labeled
|
|
These changes were released in v1.204.1-rc.4. |
aknysh
added a commit
that referenced
this pull request
Jan 28, 2026
Issue: #2032 After PR #1994, settings could no longer reference locals (regression from 1.204). This adds template processing for settings, vars, and env sections after locals are resolved, enabling bidirectional references between all sections. The fix ensures: - Locals can reference settings (resolved during locals processing) - Settings can reference locals (new template processing step) - Vars can reference both locals and processed settings - Env can reference locals, processed settings, and processed vars Added processTemplatesInSection() helper and comprehensive tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This was referenced Jan 29, 2026
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
.settings,.vars, and.envsections from the same YAML file during template resolution!env,!exec,!store,!terraform.state,!terraform.output)examples/localsexample demonstrating the featurewhy
.varsfrom the same fileYAML Functions in Locals
Locals support all Atmos YAML functions for dynamic value resolution:
!env!env API_ENDPOINT!exec!exec git rev-parse --short HEAD!store!store secrets/db .password!terraform.state!terraform.state vpc .vpc_id!terraform.output!terraform.output vpc .vpc_idExample:
Blog Posts
Locals Context Access (
2026-01-19-locals-context-access-and-design-patterns.mdx)YAML Functions in Locals (
2026-01-20-locals-yaml-functions.mdx)!env,!exec,!store,!terraform.state,!terraform.outputin localsError Handling Behavior
When a template reference cannot be resolved, Atmos produces a hard error with a clear message rather than silently returning an empty string or passing through the literal template syntax:
{{ .settings.foo }}whensettingsdoesn't existmap has no entry for key 'settings'{{ .settings.foo }}whensettingsexists but nofoomap has no entry for key 'foo'{{ .settings.foo }}when both existWorkarounds for optional values:
Example
A new
examples/localsexample demonstrates the feature:Output:
Summary of Changes
Code Changes:
pkg/locals/resolver.go:templateContextfield andWithTemplateContextmethod to pass settings/vars/env during resolutionYamlFunctionProcessorcallback type andWithYamlFunctionProcessormethod for YAML function supportresolveStringto detect and process YAML functions (strings starting with!)internal/exec/stack_processor_locals.go:localsResolveOptionsstruct andcreateYamlFunctionProcessorfor YAML function processingbuildSectionTemplateContextto merge section values with global values (not replace)mergeStringAnyMapshelper for shallow merginginternal/exec/stack_processor_utils.go:errors/errors.go: AddedErrLocalsYamlFunctionFailedsentinel errorTest Changes:
TestLocalsSettingsAccessSameFile,TestLocalsSettingsAccessDescribeStacks,TestLocalsSettingsAccessNotCrossFiletests/fixtures/scenarios/locals-settings-access/demonstrating same-file accesstests/fixtures/scenarios/locals-settings-cross-file/demonstrating cross-file access errorTestLocalsNestedSettingsAccess,TestLocalsEnvironmentVariableAccess,TestLocalsHelmfileSectionLocalstests/fixtures/scenarios/locals-advanced/for nested values and helmfile section localsTestExtractLocalsFromRawYAML_ContextAccess)TestMergeStringAnyMaps,TestBuildSectionTemplateContext_MergesBehavior)TestLocalsWithYamlFunctionsEnvfor YAML functions in localstests/fixtures/scenarios/locals-yaml-functions/for YAML function testingDocumentation Changes:
!env,!exec,!store,!terraform.state,!terraform.outputBlog Posts Added:
website/blog/2026-01-19-locals-context-access-and-design-patterns.mdxwebsite/blog/2026-01-20-locals-yaml-functions.mdxExample Added:
examples/locals/- Minimal example demonstrating locals feature:Dependency Updates:
google.golang.org/apifrom retracted v0.258.0 to v0.260.0Summary by CodeRabbit
New Features
Documentation
Tests
Examples
Chores
✏️ Tip: You can customize this high-level summary in your review settings.