fix: resolve relative component_path against BasePath for --chdir support#2029
Merged
fix: resolve relative component_path against BasePath for --chdir support#2029
Conversation
…tput executor When atmos.Component template function executes, it retrieves component_path from describe stacks output as a relative path (e.g., "components/terraform/vpc"). This relative path was used directly for file operations like writing backend.tf.json, which fails when CWD differs from project root (e.g., when using --chdir flag). The fix resolves relative component paths against atmosConfig.BasePath before using them for file operations, ensuring correct path resolution regardless of the current working directory. This addresses the user-reported error: "template: describe-stacks-all-sections:72:21 ... at <atmos.Component>: error calling Component: open ... backend.tf.json: no such file or directory" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add table-driven tests to verify that the component path resolution fix works correctly: - Relative paths are resolved against atmosConfig.BasePath - Absolute paths are preserved unchanged - Empty BasePath with relative paths works correctly These tests prevent regression of the path resolution issue that caused "backend.tf.json: no such file or directory" errors when using atmos.Component template function with --chdir flag. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive tests to improve test coverage: - TestExecutor_GetOutput_InvalidAuthManagerType: tests invalid auth manager error - TestExecutor_GetOutput_FullExecutionPath: tests complete execution without cache - TestExecutor_GetOutput_DescribeError: tests DescribeComponent failure handling - TestExecutor_GetOutput_ExecuteError: tests execute failure handling - TestExecutor_GetAllOutputs_Success: tests GetAllOutputs happy path - TestExecutor_GetAllOutputs_CacheHit: tests cache retrieval - TestExecutor_GetAllOutputs_Error: tests error handling - TestExecutor_GetAllOutputs_StaticRemoteState: tests static remote state path - TestProcessOutputs_WithInvalidJSON: tests invalid JSON handling - TestExecutor_ExecuteWithSections_InitWithReconfigure: tests reconfigure option - TestStartSpinnerOrLog_DebugMode/TraceMode: tests log-only mode - TestHighlightValue_NilConfig: tests nil config handling Key coverage improvements: - GetOutput: 50% → 89.5% - GetAllOutputs: 80% → 100% - fetchAndCacheOutputs: 66.7% → 93.3% - runInit: 90% → 100% - processOutputs: 80% → 100% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add integration tests to verify that running atmos with --chdir from a different directory correctly resolves component paths. These tests prevent regression of the path resolution issue fixed in this branch where relative component_path was not resolved against BasePath, causing "no such file or directory" errors. Tests added to chdir.yaml: - "atmos --chdir with describe stacks resolves component paths" Tests that describe stacks works correctly when run from a different directory with --chdir pointing to a fixture with templates. - "atmos --chdir with describe affected resolves paths correctly" Tests that describe affected works correctly when run from a different directory. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Contributor
📝 WalkthroughWalkthroughResolves component path determination by passing Atmos configuration into component extraction and using utils.GetComponentPath so component_path values become absolute (respecting atmosConfig.BasePath and metadata) before backend generation and file I/O. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Atmos CLI
participant Executor as Terraform Output Executor
participant Extractor as ExtractComponentConfig
participant Resolver as utils.GetComponentPath
participant Backend as Backend Generator / File I/O
CLI->>Executor: invoke describe/describe-stacks (may pass --chdir)
Executor->>Extractor: ExtractComponentConfig(atmosConfig, sections, component, stack)
Extractor->>Resolver: GetComponentPath(component_type, component, atmosConfig.BasePath, metadata)
Resolver-->>Extractor: resolved absolute component path
Extractor-->>Executor: ComponentConfig (with absolute ComponentPath)
Executor->>Backend: generate backend files / run init using resolved path
Backend-->>Executor: backend artifacts / results
Executor-->>CLI: return outputs / errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
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}
…tory The describe affected command requires git history to work properly. In CI environments with shallow clones, this causes exit code 128. Keep only the describe stacks test which adequately verifies the --chdir path resolution fix without git dependencies. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2029 +/- ##
==========================================
+ Coverage 75.43% 75.51% +0.08%
==========================================
Files 793 793
Lines 73666 73699 +33
==========================================
+ Hits 55568 55655 +87
+ Misses 14589 14547 -42
+ Partials 3509 3497 -12
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…ests Use filepath.Join() and filepath.ToSlash() for cross-platform compatibility: - Use t.TempDir() to get a valid absolute path on all platforms - Use filepath.Join() to construct paths with correct separators - Use filepath.ToSlash() to normalize paths for comparison - Use suffix check instead of exact match for flexibility This fixes test failures on Windows where path separators differ. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
osterman
reviewed
Jan 26, 2026
github-actions
bot
added
size/xl
Member
Author
|
@coderabbitai full review please |
|
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. |
Contributor
|
🧠 Learnings used✅ Actions performedFull review triggered. |
Contributor
|
🧠 Learnings used✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/fixes/describe-affected-component-path-resolution.md`:
- Around line 88-105: Add the markdown language tag "text" to the flow‑diagram
code fence so the opening fence becomes ```text (leaving the closing fence as
```); update the fenced block that starts with "User Template: {{
atmos.Component \"vpc\" \"dev-ue1\" }}" (the flow‑diagram in the doc) to use
```text to satisfy MD040 and clarify intent.
In `@pkg/terraform/output/executor_test.go`:
- Around line 21-30: The comment above the helper function validSections() is
misleading because it claims a temp directory is used; update the comment to
accurately describe the function’s behavior (e.g., "Helper function that returns
a minimal valid sections map for tests" or similar) and remove any mention of a
temp directory or cross-platform compatibility; keep the comment concise and
reference the function name validSections to locate and replace the misleading
comment.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/fixes/describe-affected-component-path-resolution.md`:
- Around line 81-85: Add terminal periods to the three bullet sentences that
describe resolution behavior for the relative path example
"components/terraform/vpc/backend.tf.json": end the lines mentioning CWD,
atmosConfig.BasePath, and the CI/subdirectory mismatch with a period. Apply the
same fix to the equivalent sentences in the later section that repeats this text
(the block that mirrors lines referencing the same path and CWD vs
atmosConfig.BasePath behavior). Ensure only punctuation is changed—no wording
alterations.
🧹 Nitpick comments (2)
pkg/terraform/output/executor_test.go (2)
3-9: Prefer temp-based BasePath over hard-coded/tmp.Hard-coded
/tmpcan be brittle on Windows/locked-down CI; deriving fromos.TempDir()keeps paths cross‑platform. As per coding guidelines, prioritize cross‑platform paths.♻️ Suggested tweak
@@ -import ( - "context" - "errors" - "fmt" - "path/filepath" - "strings" - "testing" +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" @@ - BasePath: "/tmp/test-project", + BasePath: filepath.Join(os.TempDir(), "test-project"),Also applies to: 38-46
1086-1205: Align the component argument with the section value for realism.Passing
tt.componentNamemirrors real call sites and keeps error context consistent if a test fails.🔧 Suggested tweak
- outputs, err := exec.ExecuteWithSections(atmosConfig, "vpc", "dev-ue1", sections, nil) + outputs, err := exec.ExecuteWithSections(atmosConfig, tt.componentName, "dev-ue1", sections, nil)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/fixes/describe-affected-component-path-resolution.md`:
- Around line 286-297: The snippet references componentInfoMap without showing
its derivation from componentInfo, which is confusing; update the code to assert
and assign componentInfoMap from componentInfo (e.g., componentInfoMap, ok :=
componentInfo.(map[string]interface{})) before using componentInfoMap, then use
that map when extracting "component_type" so the variables componentInfo,
componentInfoMap and the lookup for "component_type" are clearly defined and
safe.
osterman
approved these changes
Jan 26, 2026
|
These changes were released in v1.205.0. |
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
pkg/terraform/output/executor.gowhere relativecomponent_pathwas not resolved againstatmosConfig.BasePath--chdirflag path resolution scenarioswhy
backend.tf.json: no such file or directoryerrors when using theatmos.Componenttemplate function with--chdirflag or when running from a non-project-root directorybuildComponentInfo()indescribe_stacks.goreturnscomponent_pathas a relative path (e.g.,components/terraform/vpc), butpkg/terraform/output/executor.goused this path directly for file operations. When CWD differs from project root, the relative path doesn't resolve correctlycomponent_pathagainstatmosConfig.BasePathbefore using it for file operations (backend generation, terraform init/output)Changes Made
Fix (
pkg/terraform/output/executor.go):Test Coverage Improvements:
GetOutputGetAllOutputsfetchAndCacheOutputsrunInitprocessOutputsIntegration Tests Added (
tests/test-cases/chdir.yaml):atmos --chdir with describe stacks resolves component paths- Verifies describe stacks works from different directoryatmos --chdir with describe affected resolves paths correctly- Verifies describe affected works from different directoryreferences
docs/fixes/describe-affected-component-path-resolution.mdSummary by CodeRabbit
Bug Fixes
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.