fix: Resolve multiple regressions and reported issues#2035
fix: Resolve multiple regressions and reported issues#2035
Conversation
…tmpl files
Fix regression in 1.205 where templates using {{ .atmos_component }} or
{{ .atmos_stack }} fail in non-.tmpl files that have a locals section.
The locals feature (commit 6ae0a27) inadvertently triggered template
processing during import by adding settings/vars/env to context, making
len(context) > 0. This caused ProcessTmpl to fail on templates that
reference component-specific variables not available during import.
The fix tracks whether context was originally provided (from outside) vs
populated from file extraction. Templates are now only processed during
import when:
1. The file has a .tmpl extension, OR
2. Context was explicitly passed from outside
Adds test case and documentation for the regression.
Co-Authored-By: Claude <noreply@anthropic.com>
Helm now requires verification by default for plugins. The helm-diff plugin source does not support verification, so we skip it with --verify=false flag. Co-Authored-By: Claude <noreply@anthropic.com>
Two issues were fixed: 1. Missing AuthManager propagation in ProcessComponentConfig: The function only set AuthContext from the authManager, but did not store the AuthManager itself on configAndStacksInfo. This meant YAML functions like !terraform.state could not access the AuthManager for authentication. 2. Missing authentication setup in terraform shell: The ExecuteTerraformShell function was calling ProcessStacks with nil as the authManager parameter. This meant no authentication context was available when processing YAML functions during the shell command. The fix: - Set configAndStacksInfo.AuthManager = authManager in ProcessComponentConfig - Add Identity field to ShellOptions struct - Add --identity flag support to shell command - Create and authenticate AuthManager in ExecuteTerraformShell before ProcessStacks This enables YAML functions like !terraform.state to use authenticated credentials when running atmos terraform shell. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When yq returns a scalar value ending with colons (like AWS Secrets Manager
ARNs), the YAML parser was misinterpreting them as map keys with null values.
For example, "arn:aws:...:password::" became {"password:": null}.
This fix adds helper functions to detect and handle this edge case:
- isScalarString: pre-checks if the result should be returned as-is
- isMisinterpretedScalar: post-checks if YAML parsing created a false map
- isYAMLNullValue: checks if a YAML node represents null
- keyMatchesOriginalWithColon: checks if key matches original with trailing colon
Fixes #2031
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The generate varfile and generate backend commands now properly support JIT (Just-in-Time) vendored components. Previously, these commands failed when a component was configured with a source attribute because they skipped the JIT provisioning hooks. Changes: - Added ensureTerraformComponentExists helper for JIT provisioning check - Added tryJITProvision helper to provision component sources - Both commands now use these helpers to support JIT vendored components - Fixed writeBackendConfigFile to use constructTerraformComponentWorkingDir - Added tests for JIT vendored component path handling Fixes #2019 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Issue: #2018 The terraform two-word commands (providers lock, state list, workspace select) were not recognized when passed as a quoted single argument. This adds proper parsing for both quoted and separate argument forms. Changes: - Add helper functions for modular two-word command parsing - parseTwoWordCommand, parseQuotedTwoWordCommand, parseSeparateTwoWordCommand - processTerraformTwoWordCommand, processSingleCommand - Support all terraform two-word commands: providers, state, workspace, write - Add comprehensive unit tests for all helper functions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Issue: #2017 When "terraform shell" was selected from the Atmos interactive UI, it was being passed to the terraform executable which doesn't have a native "shell" command. This adds early interception of the "shell" subcommand in ExecuteTerraform() to route it to ExecuteTerraformShell(). Changes: - Add "shell" subcommand handling in ExecuteTerraform() - Convert ConfigAndStacksInfo to ShellOptions for shell execution - Add unit tests for shell options conversion - Add documentation for the fix Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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>
Add comprehensive unit tests for new functions introduced in this branch: - TestProcessTemplatesInSection: Tests template processing in sections - TestProcessTemplatesInSection_EdgeCases: Tests edge cases like nil, empty, lists - TestStoreAuthenticatedIdentity: Tests auth identity storage with mock AuthManager - TestShellOptionsValidation: Tests ShellOptions field validation - TestShellConfigWithWorkdirProvisioner: Tests workdir provisioner path handling - TestEnsureTerraformComponentExists: Tests component existence validation - TestTryJITProvision: Tests JIT provisioning behavior - TestVarfileOptions_Validation: Tests VarfileOptions field validation - TestVarfileOptions_ProcessingOptions: Tests ProcessingOptions embedding Coverage improvements: - processTemplatesInSection: 66.7% -> 80.0% - ensureTerraformComponentExists: 0% -> 80.0% - tryJITProvision: 0% -> 28.6% - storeAuthenticatedIdentity: 0% -> 100% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use filepath.Join() for platform-agnostic path construction in tests: - Replace hardcoded "/tmp/test.tfvars.json" with filepath.Join() - Replace hardcoded Unix paths in TestShellConfigWithWorkdirProvisioner This ensures tests pass on Windows where path separator is backslash. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move locals merging outside the OriginalComponentLocals guard to ensure
stack-level locals are available for template processing on every call
to ProcessComponentConfig
- Move spacelift_stack and atlantis_project computation before template
processing so they can be referenced in templates
- Update documentation with details of both fixes
Fixes templates like {{ .locals.namespace }} and {{ .spacelift_stack }}
returning <no value> in describe component output.
Co-Authored-By: Claude Opus 4.5 <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. |
|
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
|
📝 WalkthroughWalkthroughThis PR implements bidirectional template resolution between locals/settings/vars/env sections, adds Terraform shell UI support with identity-aware authentication, enables JIT vendoring for generate commands, fixes YAML parsing for strings ending with colons, and sets template processing as enabled by default. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI/UI
participant Shell as ExecuteTerraformShell
participant Auth as AuthManager
participant Stack as ProcessStacks
participant YAML as YAML Functions
CLI->>Shell: shellInfoFromOptions(component, stack, identity)
Shell->>Auth: createAndAuthenticateAuthManager(config, info)
Auth->>Auth: mergeGlobalAndComponentAuth()
Auth->>Auth: authenticateWithIdentity()
Auth->>Shell: return authManager
Shell->>Stack: ProcessStacks(info with authManager)
Stack->>YAML: !terraform.state/.../etc
YAML->>Auth: use authManager.AuthContext for credentials
YAML->>Stack: return resolved values
Stack->>Shell: return component config
Shell->>CLI: shell session ready
sequenceDiagram
participant Parser as YAML Parser
participant YQ as EvaluateYqExpression
participant Check as Scalar Checks
participant Result as Result Handler
Parser->>YQ: evaluate expression
YQ->>Check: isScalarString(result)?
alt Scalar String
Check->>Result: return original string
else YAML Parse Candidate
YQ->>YQ: unmarshal to YAML
YQ->>Check: isMisinterpretedScalar(node, original)?
alt Misinterpreted Colon
Check->>Result: return original string
else Valid YAML
YQ->>Result: return parsed structure
end
end
sequenceDiagram
participant Processor as StackProcessor
participant Locals as extractAndAddLocalsToContext
participant Template as processTemplatesInSection
participant Context as Context Builder
Processor->>Locals: extract file-scoped locals
Locals->>Context: build localsContext
Processor->>Template: processTemplatesInSection(settings, localsContext)
Template->>Context: build settingsContext (locals+settings)
Processor->>Template: processTemplatesInSection(vars, settingsContext)
Context->>Template: build varsContext (locals+settings+vars)
Processor->>Template: processTemplatesInSection(env, varsContext)
Template->>Processor: return resolved sections
Processor->>Processor: remove locals from import configs<br/>(file-scoped semantics)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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 full review please |
|
🧠 Learnings used✅ Actions performedFull review triggered. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2035 +/- ##
==========================================
+ Coverage 75.65% 75.76% +0.10%
==========================================
Files 793 793
Lines 73892 74135 +243
==========================================
+ Hits 55901 56165 +264
+ Misses 14498 14469 -29
- Partials 3493 3501 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughThis PR introduces Terraform shell subcommand support with authentication integration, bidirectional template resolution across locals/settings/vars sections, just-in-time component provisioning, YAML scalar handling for colon-ending strings, and refactored CLI parsing for Terraform two-word commands. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant TerraformShell as Terraform Shell<br/>Executor
participant AuthMgr as Auth Manager
participant Config as Component Config
participant Shell as Interactive Shell
User->>CLI: atmos terraform shell --identity=<id>
CLI->>TerraformShell: ExecuteTerraformShell(ShellOptions{Identity})
TerraformShell->>AuthMgr: createShellAuthManager(identity)
AuthMgr->>Config: Fetch merged auth config<br/>(global + component-level)
Config-->>AuthMgr: Merged auth config
AuthMgr->>AuthMgr: Authenticate with identity
AuthMgr-->>TerraformShell: Authenticated AuthManager
TerraformShell->>TerraformShell: Store AuthManager in ConfigAndStacksInfo
TerraformShell->>TerraformShell: ProcessStacks (with AuthManager)
TerraformShell->>Shell: Launch interactive shell<br/>(with environment)
Shell-->>User: Shell prompt
sequenceDiagram
participant User
participant VarfileCmd as Generate Varfile<br/>Command
participant JIT as JIT Provisioner
participant ComponentPath as Component Path
participant Varfile as Varfile<br/>Generator
User->>VarfileCmd: terraform generate varfile
VarfileCmd->>VarfileCmd: ProcessStacks (resolve config)
VarfileCmd->>VarfileCmd: ensureTerraformComponentExists()
VarfileCmd->>ComponentPath: Check if component path exists
ComponentPath-->>VarfileCmd: Path missing
VarfileCmd->>JIT: tryJITProvision(component source)
JIT->>JIT: AutoProvisionSource (5min timeout)
JIT-->>VarfileCmd: Provisioned with workdir path
VarfileCmd->>ComponentPath: Re-check component path
ComponentPath-->>VarfileCmd: Path exists
VarfileCmd->>Varfile: Generate with workdir path
Varfile-->>User: Varfile output
sequenceDiagram
participant Stack as Stack Processor
participant StackLocals as Stack-Level<br/>Locals
participant ComponentLocals as Component-Level<br/>Locals
participant Settings as Settings Section
participant Templates as Template<br/>Processing
Stack->>StackLocals: Extract file-scoped locals
Stack->>ComponentLocals: Extract component locals
Stack->>ComponentLocals: Preserve original keys
Stack->>StackLocals: Merge StackLocals + ComponentLocals<br/>(component precedence)
Stack->>Templates: Add merged locals to context
Stack->>Settings: Process settings templates<br/>(with merged locals)
Stack->>Settings: Process vars templates<br/>(with resolved settings)
Templates-->>Settings: Template resolution complete
Stack->>Stack: Filter output to original<br/>component locals only
Stack-->>Stack: Return ConfigAndStacksInfo<br/>(cleaned locals)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
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-1655:⚠️ Potential issue | 🔴 CriticalRegenerate the NOTICE file using the provided script.
The pipeline is failing because the NOTICE file is out of sync. Run
./scripts/generate-notice.shlocally and commit the result. Based on learnings, this file is programmatically generated and should not be manually edited.
Move the "terraform shell" dispatch from ExecuteTerraform (terraform.go) to the UI dispatcher (atmos.go), so ExecuteTerraform no longer needs to know about the shell subcommand. Both entry paths now call ExecuteTerraformShell directly: - CLI path: cmd/terraform/shell.go -> ExecuteTerraformShell - UI path: atmos.go -> ExecuteTerraformShell Extract testable helpers from inline logic: - shellInfoFromOptions: builds ConfigAndStacksInfo from ShellOptions - resolveWorkdirPath: returns workdir override or original path - shellOptionsForUI: builds ShellOptions for the UI dispatch path Replace tautological tests that re-implemented production logic inline with tests that call the actual extracted functions. Add blog post for templating enabled by default. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…comment Restore blog posts and documentation files that were inadvertently deleted or reverted by commit 6e3ad09 (splitting website updates to fix-issues-6). Add clarifying comment to utils_auth.go explaining why auth orchestration lives in internal/exec rather than pkg/auth (circular import constraint). Restored from main: - website/blog/2025-01-06-chdir-config-isolation.mdx - website/blog/2026-01-29-artifactory-store-fix.mdx - website/docs/cli/configuration/stores.mdx - website/docs/stacks/hooks.mdx - website/docs/tutorials/sharing-state/stores.mdx Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
||
| ## What Changed | ||
|
|
||
| Atmos now registers Viper-level defaults for all three template processing settings: |
There was a problem hiding this comment.
No, this is too technical. This needs to be user-facing, not developer-facing. Users have no idea what Viper is.
|
|
||
| ## Why This Changed | ||
|
|
||
| The [config isolation feature](/changelog/introducing-chdir-flag) (PR #1941) changed how Atmos discovers configuration files. When you use `--chdir` or have a local `atmos.yaml`, Atmos now respects that config in isolation rather than merging it with the repository root's `atmos.yaml`. |
There was a problem hiding this comment.
I don't know that this has anything to do with the stated title of this changelog entry. Maybe it belongs in a different changelog entry?
|
|
||
| The [config isolation feature](/changelog/introducing-chdir-flag) (PR #1941) changed how Atmos discovers configuration files. When you use `--chdir` or have a local `atmos.yaml`, Atmos now respects that config in isolation rather than merging it with the repository root's `atmos.yaml`. | ||
|
|
||
| This created an unintended side effect: if your local `atmos.yaml` didn't explicitly set `templates.settings.enabled: true`, the value defaulted to `false` (Go's zero value for booleans). Previously, the repository root's `atmos.yaml` would have been merged in as a fallback, providing the `true` value. |
There was a problem hiding this comment.
This seems to be masking the underlying problem. Changing the default means the underlying problem is still there, it's just by default working under this one circumstance. I don't understand the fix. Seems like curve fitting to me.
|
💥 This pull request now has conflicts. Could you fix it @aknysh? 🙏 |
what
localsandsettings(Atmos 1.205.0 regression){{ .atmos_component }}failuresspacelift_stackandatlantis_projecttemplate resolution orderterraform shellcommand routing when invoked from Atmos interactive UIgenerate varfileandgenerate backendAuthManagerpropagation to YAML functions interraform shellcommand--verify=false.terraformsymlinks breaking describe-affected testsutils_auth.goterraform shelldirectly from UI dispatcher without touchingExecuteTerraformfalsewhen config file doesn't explicitly configure templateswhy
1. Settings can't refer to locals anymore — Atmos 1.205.0 regression (#2032)
After PR #1994,
localscould referencesettingsbutsettingscould no longer referencelocals. The root cause was thatextractAndAddLocalsToContext()added raw settings/vars/env to the context without processing their templates first. Go templates don't recursively expand template strings in data values, so{{ .locals.stage }}inside a settings value remained as a literal string.Fix: Introduced a processing pipeline in
extractAndAddLocalsToContext():Added
processTemplatesInSection()helper that converts a section to YAML, processes templates, and parses the result back.Files:
internal/exec/stack_processor_utils.go,internal/exec/stack_processor_utils_test.go2. Template regression with
{{ .atmos_component }}in non-.tmplfiles (#2032)PR #1994 inadvertently caused template processing to trigger during imports for non-
.tmplfiles that hadsettings,vars, orenvsections. The locals feature populated the template context, makinglen(context) > 0, which triggered the template processing guard. Templates like{{ .atmos_component }}then failed because component context isn't available at import time.Fix: Track whether context was externally provided vs extracted from the file itself using
originalContextProvided. Only externally-provided context triggers template processing in non-.tmplfiles. Also persist resolved sections back intostackConfigMap, remove locals from import configs before merging (file-scoped isolation), and pass resolved locals throughProcessStackConfig.Files:
internal/exec/stack_processor_utils.go,internal/exec/stack_processor_process_stacks.go,internal/exec/stack_processor_utils_test.go3. Stack-level vs component-level locals handling (#2032)
Stack-level locals (for template resolution) were incorrectly appearing in final component output, while component-level locals (user-defined) were being lost. Additionally,
ProcessComponentConfigis called multiple times and map mutations polluted the globalstacksMapcache.Fix:
OriginalComponentLocalsonConfigAndStacksInfocomponentSectionbefore modification to prevent cache pollutionpostProcessTemplatesAndYamlFunctionsto keep only component-level keysExecuteDescribeStacks()for terraform, helmfile, and packerFiles:
internal/exec/utils.go,internal/exec/describe_stacks.go,pkg/schema/schema.go,tests/cli_locals_test.go4.
spacelift_stackandatlantis_projecttemplate resolution (#2032)Templates referencing
{{ .spacelift_stack }}and{{ .atlantis_project }}returned<no value>because these values were computed after template processing.Fix: Moved computation of
spacelift_stackandatlantis_projectbeforeProcessTmplWithDatasourcesinProcessStacks().Files:
internal/exec/utils.go5.
terraform shellnot working from Atmos UI (#2017)The interactive UI dispatches commands through
ExecuteTerraform(), which had no handler for the "shell" subcommand. Sinceterraform shellis an Atmos-only command (not a native terraform subcommand), it would fall through and attempt to executeterraform shellas a native command, which doesn't exist.Fix: Route
terraform shelldirectly toExecuteTerraformShell()from the UI dispatcher inatmos.go, bypassingExecuteTerraform()entirely. This keepsterraform.gofree of shell-specific logic. Both entry paths now callExecuteTerraformShelldirectly:cmd/terraform/shell.go→ExecuteTerraformShell()atmos.go→shellOptionsForUI()→ExecuteTerraformShell()Extracted testable helpers:
shellInfoFromOptions()(buildsConfigAndStacksInfofromShellOptions),resolveWorkdirPath()(returns workdir override or original path), andshellOptionsForUI()(buildsShellOptionsfor the UI dispatch).Files:
internal/exec/atmos.go,internal/exec/terraform_shell.go,internal/exec/terraform_shell_test.go6. JIT vendoring for
generate varfileandgenerate backend(#2019)These commands used
ProcessStacks()directly without triggering JIT provisioning hooks. Also,writeBackendConfigFilehardcoded path construction instead of usingconstructTerraformComponentWorkingDir().Fix: Added JIT provisioning support following the
ExecuteTerraform()pattern: check component path, detectsourceconfig, run auto-provisioning, use workdir path.Files:
internal/exec/terraform_generate_varfile.go,internal/exec/terraform_generate_backend.go,internal/exec/terraform_generate_varfile_test.go,internal/exec/path_utils_test.go7. YAML strings ending with colons parsed incorrectly (#2031)
EvaluateYqExpressionreturned unquoted strings thatyaml.Unmarshalmisinterpreted as map keys when they ended with colons (e.g., AWS ARNs likearn:aws:...:secret:password::).Fix: Added
isScalarString(pre-check) andisMisinterpretedScalar(post-check) helpers to detect and return such strings directly without YAML parsing.Files:
pkg/utils/yq_utils.go,pkg/utils/yq_utils_test.go8.
AuthManagerpropagation to YAML functions interraform shell!terraform.stateYAML function failed interraform shellandterraform plan --allbecauseAuthManagerwasn't stored onconfigAndStacksInfo, andExecuteTerraformShellpassednilas the authManager.Fix: Store
AuthManageronconfigAndStacksInfoinProcessComponentConfig. AddIdentityfield toShellOptionsand--identityflag to the shell command. Extract shared auth manager creation intoutils_auth.gowithcreateAndAuthenticateAuthManager(),getMergedAuthConfig(), andstoreAutoDetectedIdentity()— eliminating duplication betweenterraform.goandterraform_shell.go.Files:
internal/exec/utils.go,internal/exec/utils_auth.go,internal/exec/utils_auth_test.go,internal/exec/terraform_shell.go,pkg/terraform/options.go,cmd/terraform/shell.go9. Helm plugin install in Dockerfile
Added
--verify=falsetohelm plugin installcommand for Helm 4 compatibility.Files:
Dockerfile10. Dangling
.terraformsymlinks in describe-affected testsTestDescribeAffectedWith*tests failed locally due to a dangling symlink inexamples/secrets-masking/.terraform/providers/left by a previous test run.Fix: Added
.terraformto the copy skip filter alongside the existingnode_modulesskip.11. Template settings defaulting to
falsewhen config file doesn't explicitly configure templatesAfter PR #1941 (config isolation for
--chdir), fixtures with their ownatmos.yamlthat didn't specify template settings gottemplates.settings.enabled = falsebecause Viper zero-valued bools. Previously, the git root'satmos.yaml(which hadtemplates.settings.enabled: true) was merged in.Fix: Added Viper defaults for
templates.settings.enabled,templates.settings.sprig.enabled, andtemplates.settings.gomplate.enabledinsetDefaultConfiguration().Files:
pkg/config/load.goreferences
generate varfileandgenerate backenddon't work with JIT vendoring #2019!terraform.stateYAML Tag Incorrectly Parses String Values Ending with Colons #2031Summary by CodeRabbit
New Features
Bug Fixes
Chores