diff --git a/.claude/skills/field-test/SKILL.md b/.claude/skills/field-test/SKILL.md index 47c06c40cb..4ee06f84c6 100644 --- a/.claude/skills/field-test/SKILL.md +++ b/.claude/skills/field-test/SKILL.md @@ -1,7 +1,7 @@ --- name: field-test -description: "Hands-on manual DX test pass of a feature or CLI command: read the real implementation and tests, hypothesize plausible user misunderstandings and misuse automated tests don't cover, build durable fixtures, execute for real against real state, and report ranked findings. Investigation only — never fixes anything found. Invoke on explicit requests like 'field test X' / 'do a DX test pass on X' / 'find vibe-coded slop in X'." -argument-hint: "Feature or command to test, e.g. 'atmos vendor pull'" +description: "Hands-on manual DX test pass of a feature or CLI command: read the real implementation and tests, hypothesize plausible user misunderstandings and misuse automated tests don't cover, build durable fixtures, execute for real against real state, and report ranked findings. Investigation only — never fixes anything found. Defaults to testing whatever the current branch changed vs its base branch when no explicit target is given. Invoke on explicit requests like 'field test X' / 'do a DX test pass on X' / 'find vibe-coded slop in X' / 'field test this branch'." +argument-hint: "Feature or command to test, e.g. 'atmos vendor pull' (omit to default to this branch's change)" metadata: copyright: Copyright Cloud Posse, LLC 2026 version: "1.0.0" @@ -10,8 +10,38 @@ metadata: # Field Test Hands-on, adversarial test pass of **`$ARGUMENTS`** (the feature/command named when this skill -was invoked, e.g. `atmos vendor pull`). If no target was given, ask which feature/command to test -before starting. +was invoked, e.g. `atmos vendor pull`). + +**If no target was given, default to the change introduced on the current branch** rather than +asking. Resolve the actual pull-request base branch when one is available +(`gh pr view --json baseRefName -q .baseRefName` for the current branch), falling back to the +repository's default branch (`gh repo view --json defaultBranchRef -q .defaultBranchRef.name`) +when no PR exists yet. Do NOT use the upstream tracking branch (`@{u}`) as the base — for a normal +feature branch that tracks `origin/`, diffing against its own upstream produces +an empty or near-empty diff, not the PR's actual changes, once the branch has been pushed. + +A branch NAME from `gh` (e.g. `main`) is not guaranteed to be a usable git ref in THIS checkout — +shallow clones, detached HEADs, and worktrees with a narrow fetch refspec can have the name +without the commits. Before running any diff, verify the base actually resolves here +(`git rev-parse --verify --quiet ^{commit}`), trying in order: `origin/`, +``, `origin/main`, `main` — take the first that verifies. If `gh` itself isn't +available or returns nothing, skip straight to the `origin/main`/`main` candidates. If NONE of +these resolve, stop and ask the user which base to diff against — do not run `git diff ...` +against an unverified ref and let it fail with a confusing git error. Once a real base is +confirmed, inspect the FULL set of +changes relative to that base — content, not just a file-list summary: `git diff ...HEAD` +(the full patch, not `--stat`, since `--stat` only shows file names and line counts, not what +those lines actually do) for committed history; `git diff HEAD` for any staged/unstaged changes +to tracked files not yet committed; and `git ls-files --others --exclude-standard` to enumerate +untracked files — `git status --porcelain` lists their paths too but never their content, and +neither `git diff HEAD` nor a `--stat` summary includes untracked files at all, so a brand-new +implementation file can otherwise go completely unread. Read the actual content of every +untracked file this turns up, the same as any diff hunk. Derive the test target from all of +that — the CLI command(s), flag(s), config option(s), or subsystem the changed files implement — +and state explicitly what you inferred and why before proceeding to Phase 1. Only fall back to +asking the user if no candidate base ref resolves at all, there's truly nothing changed (clean +worktree, base equals HEAD, no untracked files), or the changes span multiple unrelated features +with no coherent single target (ask which one to focus on, don't silently pick one). Goal: catch "vibe-coded slop" — behavior that looks fine in code review but breaks or misleads a real user — not to re-run what automated tests already cover. Anticipate plausible user @@ -27,10 +57,23 @@ The goal is a map of "documented or plausible usage" minus "already tested" = wh verification. This phase is broad, read-only research — delegate it to `Agent subagent_type: "Explore"` (1-3 agents in parallel, one per bullet below) rather than doing it all serially inline. -- **Implementation** — the actual code, not just its docs or the skill describing it. Per this - repo's conventions, business logic lives in narrow `pkg/` packages, not `internal/exec/` (being - phased out) — check both `cmd//` (thin call site) and the `pkg/` package(s) it - delegates to for the real logic and error paths. +When defaulting to the current branch (no explicit target given), scope every bullet below to the +target inferred from the branch diff — don't research the whole surrounding subsystem when the +branch only touched one corner of it. If the branch's changed files span more than one command or +package, treat each as a separate target to cover in Phase 2-4, prioritized by how much of the +diff each accounts for. + +- **Implementation** — the actual code, not just its docs or the skill describing it. Inspect + every changed production package identified by the diff — new business logic belongs in narrow + `pkg/` packages per this repo's conventions, but `internal/exec/` is still where a large amount + of existing logic lives during its ongoing migration, so a branch touching files there must + still be read, not skipped. Check both `cmd//` (thin call site) and whichever + `pkg/`/`internal/exec/` package(s) it delegates to for the real logic and error paths. When + defaulting from a branch diff, read the full diff content itself first (not just the post-change + files, and not just a `--stat` summary) — the diff shows what changed *from*, which is where a + regression or half-finished edge case would show up. Include untracked files + (`git ls-files --others --exclude-standard`) in this reading pass too — they never appear in any + diff at all. - **Docs and skills** — every relevant page under `website/docs/cli/commands/`, the matching `.claude/skills/atmos-*` skill(s) for the subsystem, and any README describing the feature. Note anything phrased with confidence you haven't independently confirmed against the code — docs and diff --git a/cmd/config/operations.go b/cmd/config/operations.go index 688295803a..7949f740d2 100644 --- a/cmd/config/operations.go +++ b/cmd/config/operations.go @@ -7,7 +7,9 @@ import ( cfg "github.com/cloudposse/atmos/pkg/config" "github.com/cloudposse/atmos/pkg/data" "github.com/cloudposse/atmos/pkg/perf" + "github.com/cloudposse/atmos/pkg/schema" "github.com/cloudposse/atmos/pkg/ui" + u "github.com/cloudposse/atmos/pkg/utils" atmosyaml "github.com/cloudposse/atmos/pkg/yaml" ) @@ -15,20 +17,36 @@ import ( var valueType string var configGetCmd = &cobra.Command{ - Use: "get ", - Short: "Read a value from atmos.yaml by dot-notation path", - Long: "Read a value from atmos.yaml using a dot-notation path (e.g. logs.level).", + Use: "get ", + Short: "Read a value from the effective Atmos configuration by dot-notation path", + Long: `Read a value using a dot-notation path (e.g. logs.level) from the effective, +fully-merged Atmos configuration for this invocation -- the same configuration +"terraform plan", "list stacks", etc. actually use, including every --config +file, --config-path directory, and profile applied on top of each other. This +can differ from what a single physical atmos.yaml file declares on its own +(cloudposse/atmos#2867): use "atmos config format" or read the file directly +to inspect one file's own declared value instead.`, Example: "atmos config get logs.level", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { defer perf.Track(atmosConfigPtr, "config.getRunE")() - file, err := resolveConfigFile(cmd) + // Reload rather than reuse atmosConfigPtr, mirroring configListCmd: this keeps `get` + // independently correct (and independently testable via RunE) even before root.go's + // PersistentPreRun has populated the package-level pointer. Safe to call with an empty + // ConfigAndStacksInfo{} -- LoadConfig now falls back to os.Args/env for + // --config/--config-path/--base-path itself (cloudposse/atmos#2868). + atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, false) + if err != nil { + return err + } + + effectiveYAML, err := u.ConvertToYAML(atmosConfig) if err != nil { return err } - value, err := atmosyaml.GetFile(file, args[0]) + value, err := atmosyaml.Get([]byte(effectiveYAML), args[0]) if err != nil { return err } @@ -140,12 +158,16 @@ func init() { } // resolveConfigFile picks the atmos.yaml to edit. The inherited persistent -// --config flag (first entry) acts as an explicit override; otherwise the file -// is discovered in the current directory or git root. +// --config flag acts as an explicit override when it names exactly one file; +// otherwise the file is discovered in the current directory or git root. func resolveConfigFile(cmd *cobra.Command) (string, error) { - override := "" - if cfgFiles, _ := cmd.Flags().GetStringSlice("config"); len(cfgFiles) > 0 { - override = cfgFiles[0] + cfgFiles, _ := cmd.Flags().GetStringSlice("config") + override, err := cfg.ResolveConfigOverride(cfgFiles) + if err != nil { + return "", errUtils.Build(errUtils.ErrInvalidArgumentError). + WithExplanation(err.Error()). + WithHint("Pass a single --config file to config set/delete/format, or edit the target file directly."). + Err() } file, err := cfg.ResolveEditableConfigFile(atmosConfigPtr, override) diff --git a/cmd/config/operations_test.go b/cmd/config/operations_test.go index c5835ebfc0..84091c34d5 100644 --- a/cmd/config/operations_test.go +++ b/cmd/config/operations_test.go @@ -1,15 +1,22 @@ package config import ( + "bytes" + stdio "io" "os" "path/filepath" + "strings" "testing" + ckerrors "github.com/cockroachdb/errors" "github.com/spf13/cobra" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" errUtils "github.com/cloudposse/atmos/errors" + "github.com/cloudposse/atmos/pkg/data" + iolib "github.com/cloudposse/atmos/pkg/io" "github.com/cloudposse/atmos/pkg/schema" atmosyaml "github.com/cloudposse/atmos/pkg/yaml" ) @@ -130,6 +137,29 @@ func TestResolveConfigFile_Error(t *testing.T) { require.ErrorIs(t, err, errUtils.ErrInvalidArgumentError) } +// TestResolveConfigFile_MultipleConfigFilesAmbiguous guards against a bug found during a +// field-test pass on cloudposse/atmos#2867/#2868: resolveConfigFile silently used only the +// FIRST --config file (cfgFiles[0]) when multiple were given, so `config set --config a,b +// logs.level X` reported success editing a.yaml while the actual effective value (what `config +// get` reports, and what every other atmos command uses) stayed unchanged whenever b.yaml also +// set that key -- a false success, not just a stale-value bug. +func TestResolveConfigFile_MultipleConfigFilesAmbiguous(t *testing.T) { + dir := t.TempDir() + fileA := filepath.Join(dir, "a.yaml") + fileB := filepath.Join(dir, "b.yaml") + require.NoError(t, os.WriteFile(fileA, []byte("settings:\n enabled: true\n"), 0o644)) + require.NoError(t, os.WriteFile(fileB, []byte("settings:\n enabled: false\n"), 0o644)) + + cmd := &cobra.Command{} + cmd.Flags().StringSlice("config", []string{fileA, fileB}, "") + + _, err := resolveConfigFile(cmd) + require.ErrorIs(t, err, errUtils.ErrInvalidArgumentError) + details := strings.Join(ckerrors.GetAllDetails(err), "\n") + assert.Contains(t, details, "a.yaml") + assert.Contains(t, details, "b.yaml") +} + func TestConfigGetCommand_MissingValue(t *testing.T) { dir := t.TempDir() file := filepath.Join(dir, "atmos.yaml") @@ -146,6 +176,86 @@ func TestConfigGetCommand_MissingValue(t *testing.T) { require.ErrorIs(t, err, atmosyaml.ErrYAMLPathNotFound) } +// configGetTestStreams is a minimal io.Streams implementation for capturing data output, +// mirroring configSchemaTestStreams in schema_test.go. +type configGetTestStreams struct { + stdin stdio.Reader + stdout *bytes.Buffer + stderr *bytes.Buffer +} + +func (ts *configGetTestStreams) Input() stdio.Reader { return ts.stdin } +func (ts *configGetTestStreams) Output() stdio.Writer { return ts.stdout } +func (ts *configGetTestStreams) Error() stdio.Writer { return ts.stderr } +func (ts *configGetTestStreams) RawOutput() stdio.Writer { return ts.stdout } +func (ts *configGetTestStreams) RawError() stdio.Writer { return ts.stderr } + +// TestConfigGetCommand_ReportsEffectiveMergedValue reproduces the "stale value" half of +// cloudposse/atmos#2867: `atmos config get` used to read only the FIRST --config file +// directly off disk (resolveConfigFile picked cfgFiles[0]), so a second --config file's +// override was invisible to `get` even though the rest of atmos (stack discovery, etc.) +// correctly used the merged value. `get` must report the same effective value everything +// else uses. +func TestConfigGetCommand_ReportsEffectiveMergedValue(t *testing.T) { + dir := t.TempDir() + mainFile := filepath.Join(dir, "main.yaml") + fragmentFile := filepath.Join(dir, "fragment.yaml") + + require.NoError(t, os.WriteFile(mainFile, []byte(` +base_path: "." +stacks: + base_path: "stacks" + included_paths: + - "deploy/**/*" +`), 0o644)) + require.NoError(t, os.WriteFile(fragmentFile, []byte(` +stacks: + included_paths: + - "deploy/**/*" + - "other/**/*" +`), 0o644)) + + streams := &configGetTestStreams{stdin: &bytes.Buffer{}, stdout: &bytes.Buffer{}, stderr: &bytes.Buffer{}} + ioCtx, err := iolib.NewContext(iolib.WithStreams(streams)) + require.NoError(t, err) + data.InitWriter(ioCtx) + t.Cleanup(data.Reset) + + viper.Reset() + t.Cleanup(viper.Reset) + + origArgs := os.Args + t.Cleanup(func() { os.Args = origArgs }) + os.Args = []string{"atmos", "--config", mainFile + "," + fragmentFile, "config", "get", "stacks.included_paths"} + + require.NoError(t, configGetCmd.RunE(configGetCmd, []string{"stacks.included_paths"})) + + output := streams.stdout.String() + assert.True(t, strings.Contains(output, "deploy/**/*"), "output should contain the first file's value: %s", output) + assert.True(t, strings.Contains(output, "other/**/*"), + "output must reflect the SECOND --config file's override, not just the first file's stale value: %s", output) +} + +// TestConfigGetCommand_InitCliConfigError proves configGetCmd.RunE surfaces a genuine +// InitCliConfig failure (a malformed --config file here) instead of panicking or masking it, +// since `get` now reloads the full effective config on every invocation rather than reading a +// single already-validated file (cloudposse/atmos#2867/#2868). +func TestConfigGetCommand_InitCliConfigError(t *testing.T) { + dir := t.TempDir() + badFile := filepath.Join(dir, "bad.yaml") + require.NoError(t, os.WriteFile(badFile, []byte("settings:\n enabled: [true\n"), 0o644)) // unterminated flow sequence + + viper.Reset() + t.Cleanup(viper.Reset) + + origArgs := os.Args + t.Cleanup(func() { os.Args = origArgs }) + os.Args = []string{"atmos", "--config", badFile, "config", "get", "settings.enabled"} + + err := configGetCmd.RunE(configGetCmd, []string{"settings.enabled"}) + require.Error(t, err) +} + func TestConfigSetCommand_TypeVariants(t *testing.T) { dir := t.TempDir() file := filepath.Join(dir, "atmos.yaml") diff --git a/internal/exec/describe_workflows_test.go b/internal/exec/describe_workflows_test.go index 4d38197635..05a2449745 100644 --- a/internal/exec/describe_workflows_test.go +++ b/internal/exec/describe_workflows_test.go @@ -87,6 +87,11 @@ workflows: // Update config with the correct base path config.BasePath = tmpDir config.Workflows.BasePath = "stacks/workflows" + // Refresh derived absolute paths (e.g. WorkflowsDirAbsolutePath) after mutating BasePath + // directly -- real callers only ever get a fresh AtmosConfiguration from InitCliConfig, + // which computes these together; a manual post-load field mutation must recompute them + // too or ExecuteDescribeWorkflows would resolve against the stale pre-mutation paths. + require.NoError(t, cfg.AtmosConfigAbsolutePaths(&config)) tests := []struct { name string @@ -120,7 +125,7 @@ workflows: }, }, wantErr: true, - errContains: "the workflow directory 'nonexistent' does not exist", + errContains: "workflow directory does not exist: 'nonexistent'", }, } @@ -190,6 +195,7 @@ workflows: config := initTestConfig(t) config.BasePath = tmpDir config.Workflows.BasePath = "stacks/workflows" + require.NoError(t, cfg.AtmosConfigAbsolutePaths(&config)) tests := []struct { name string @@ -287,6 +293,7 @@ workflows: config := initTestConfig(t) config.BasePath = tmpDir config.Workflows.BasePath = "stacks/workflows" + require.NoError(t, cfg.AtmosConfigAbsolutePaths(&config)) // Should continue processing and return valid workflows despite invalid file. listResult, _, _, err := ExecuteDescribeWorkflows(config) @@ -326,6 +333,7 @@ some_other_key: config := initTestConfig(t) config.BasePath = tmpDir config.Workflows.BasePath = "stacks/workflows" + require.NoError(t, cfg.AtmosConfigAbsolutePaths(&config)) // Should continue processing and return valid workflows. listResult, _, _, err := ExecuteDescribeWorkflows(config) @@ -344,6 +352,7 @@ func TestExecuteDescribeWorkflows_EmptyWorkflowsDirectory(t *testing.T) { config := initTestConfig(t) config.BasePath = tmpDir config.Workflows.BasePath = "stacks/workflows" + require.NoError(t, cfg.AtmosConfigAbsolutePaths(&config)) // Empty workflows directory. listResult, mapResult, allResult, err := ExecuteDescribeWorkflows(config) @@ -353,3 +362,29 @@ func TestExecuteDescribeWorkflows_EmptyWorkflowsDirectory(t *testing.T) { assert.Len(t, mapResult, 0) assert.Len(t, allResult, 0) } + +// TestExecuteDescribeWorkflows_PathLeak guards against a bug found during a field-test pass on +// cloudposse/atmos#2867/#2868: "the workflow directory '%s' does not exist" interpolated the +// resolved absolute workflowsDir directly, and the sibling "error reading the directory" message +// interpolated the raw, unresolved atmosConfig.Workflows.BasePath instead of workflowsDir (the +// directory actually searched) -- a copy/paste inconsistency with the sibling branch one line up. +// The pre-existing "nonexistent workflows directory" test above doesn't catch either bug: it +// leaves BasePath/WorkflowsDirAbsolutePath unset, so getWorkflowsDirToUse falls back to a bare +// relative path that was never absolute to begin with. +func TestExecuteDescribeWorkflows_PathLeak(t *testing.T) { + resolvedDir := resolvedTempDir(t) + + config := schema.AtmosConfiguration{ + BasePath: resolvedDir, + Workflows: schema.Workflows{ + BasePath: "nonexistent-workflows-dir", + }, + } + require.NoError(t, cfg.AtmosConfigAbsolutePaths(&config)) + + _, _, _, err := ExecuteDescribeWorkflows(config) + + require.Error(t, err) + assert.NotContains(t, err.Error(), resolvedDir, "error must not leak the machine-specific absolute directory") + assert.Contains(t, err.Error(), "nonexistent-workflows-dir") +} diff --git a/internal/exec/validate_component.go b/internal/exec/validate_component.go index f088f0b170..9238b3d97e 100644 --- a/internal/exec/validate_component.go +++ b/internal/exec/validate_component.go @@ -30,6 +30,20 @@ func getBasePathToUse(atmosConfig *schema.AtmosConfiguration) string { return atmosConfig.BasePath } +// getWorkflowsDirToUse returns the appropriate workflows directory for file resolution. It +// prefers the precomputed WorkflowsDirAbsolutePath (set by AtmosConfigAbsolutePaths, the same +// mechanism cloudposse/atmos#2864 uses for the top-level base_path), falling back to joining +// the raw BasePath/Workflows.BasePath for callers that construct an AtmosConfiguration by hand +// without running it through AtmosConfigAbsolutePaths first (e.g. tests). +func getWorkflowsDirToUse(atmosConfig *schema.AtmosConfiguration) string { + if atmosConfig.WorkflowsDirAbsolutePath != "" { + return atmosConfig.WorkflowsDirAbsolutePath + } + // u.JoinPath (unlike filepath.Join) returns an already-absolute Workflows.BasePath as-is + // instead of nesting it under BasePath. + return u.JoinPath(atmosConfig.BasePath, atmosConfig.Workflows.BasePath) +} + // enableProvenanceForRichOutput sets atmosConfig.TrackProvenance when the // resolved output format is "rich". Provenance is enabled only for the rich // invocation: it lets the command map JSON Schema fields back to the diff --git a/internal/exec/validate_schema.go b/internal/exec/validate_schema.go index a1441ce6b4..f0085fae93 100644 --- a/internal/exec/validate_schema.go +++ b/internal/exec/validate_schema.go @@ -344,11 +344,39 @@ func displayPath(file string) string { if err != nil { return file } - rel, err := filepath.Rel(cwd, file) - if err != nil || strings.HasPrefix(rel, "..") { + if rel, ok := relPath(cwd, file); ok { + return rel + } + // os.Getwd() preserves the logical $PWD-style path (e.g. macOS's /tmp), while + // config-derived absolute paths resolved through git-root discovery resolve symlinks + // internally (via go-git/go-billy's filepath.EvalSymlinks) -- so cwd and file can end up in + // different (logical vs. physical) forms of the same directory when a symlink is involved. + // Retry with both resolved to their canonical physical form; EvalSymlinks on an + // already-physical path is a no-op, so this only changes behavior for the mismatched case. + // Resolve the DIRECTORY, not file itself: file frequently doesn't exist yet (that's often + // exactly why displayPath is being called, e.g. "file not found" errors), and EvalSymlinks + // requires its target to exist. + resolvedCwd, cwdErr := filepath.EvalSymlinks(cwd) + resolvedFileDir, fileErr := filepath.EvalSymlinks(filepath.Dir(file)) + if cwdErr != nil || fileErr != nil { return file } - return rel + if rel, ok := relPath(resolvedCwd, filepath.Join(resolvedFileDir, filepath.Base(file))); ok { + return rel + } + return file +} + +// relPath returns file relative to cwd when that relative path stays within cwd, and whether it +// succeeded. Only a leading ".." PATH SEGMENT means "escapes cwd" -- a bare HasPrefix(rel, "..") +// check would also reject a legitimate in-directory name like "..vendor.yaml", leaking its +// absolute path. +func relPath(cwd, file string) (string, bool) { + rel, err := filepath.Rel(cwd, file) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", false + } + return rel, true } func (av *atmosValidatorExecutor) printValidation(schema string, files []string) (uint, error) { diff --git a/internal/exec/validate_schema_test.go b/internal/exec/validate_schema_test.go index d5e801454d..8f3d84e16a 100644 --- a/internal/exec/validate_schema_test.go +++ b/internal/exec/validate_schema_test.go @@ -272,6 +272,16 @@ func TestDisplayPath(t *testing.T) { file: "config.yaml", want: "config.yaml", }, + { + // A file named "..vendor.yaml" directly in cwd is IN-directory: filepath.Rel + // resolves it to "..vendor.yaml", which starts with ".." as a substring but is not + // a parent-directory escape (that would be "../vendor.yaml" or exactly ".."). A + // naive strings.HasPrefix(rel, "..") check would incorrectly reject this and leak + // the absolute path. + name: "in-directory filename starting with .. stays relative", + file: filepath.Join(cwd, "..vendor.yaml"), + want: "..vendor.yaml", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -280,6 +290,73 @@ func TestDisplayPath(t *testing.T) { } } +// TestDisplayPath_SymlinkedCWD guards against a bug found during a field-test pass on +// cloudposse/atmos#2867/#2868: when the working directory is reached through a symlink (e.g. +// macOS's /tmp -> /private/tmp) AND the shell's $PWD reflects the symlinked (logical) path, +// os.Getwd() returns that logical path (Go's documented $PWD shortcut) while config-derived +// absolute paths (BasePathAbsolute/VendorDirAbsolutePath/WorkflowsDirAbsolutePath, resolved via +// git-root discovery which internally calls filepath.EvalSymlinks) return the physical, +// resolved path -- so filepath.Rel between the two silently fails, and displayPath falls back +// to the full absolute path, defeating the fix in exactly the environments it was meant to help. +// +// Plain os.Chdir alone doesn't reproduce this (it doesn't touch $PWD), so $PWD is set explicitly +// here to model what a real shell `cd` into a symlinked directory does. +func TestDisplayPath_SymlinkedCWD(t *testing.T) { + tmpDir := t.TempDir() + realDir := filepath.Join(tmpDir, "realdir") + require.NoError(t, os.MkdirAll(realDir, 0o755)) + + symlinkPath := filepath.Join(tmpDir, "symlink") + if err := os.Symlink(realDir, symlinkPath); err != nil { + t.Skipf("Skipping symlink test: %v", err) + } + + resolvedRealDir, err := filepath.EvalSymlinks(realDir) + require.NoError(t, err) + + wd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, os.Chdir(wd)) }) + require.NoError(t, os.Chdir(symlinkPath)) + t.Setenv("PWD", symlinkPath) + + // file is built from the RESOLVED (physical) directory, as config-derived absolute paths + // are (via git-root discovery's filepath.EvalSymlinks), while os.Getwd() inside displayPath + // will see the symlinked (logical) form because $PWD matches it. + file := filepath.Join(resolvedRealDir, "vendor.yaml") + assert.Equal(t, "vendor.yaml", displayPath(file)) +} + +// TestDisplayPath_SymlinkedCWD_BothLogical guards against a regression the first version of the +// EvalSymlinks fix above introduced: when NO git-root discovery runs (e.g. base_path already +// absolute, or no .git found), config-derived absolute paths are computed via plain +// filepath.Abs, which -- like os.Getwd() -- respects the $PWD logical shortcut. So under a +// symlinked cwd (e.g. macOS's /tmp) with no git repo involved, BOTH cwd and file stay +// consistently in LOGICAL form and were already directly comparable before any EvalSymlinks +// normalization. A fix that unconditionally resolves only cwd (not file) breaks this +// previously-working case by making cwd physical while file stays logical. +func TestDisplayPath_SymlinkedCWD_BothLogical(t *testing.T) { + tmpDir := t.TempDir() + realDir := filepath.Join(tmpDir, "realdir") + require.NoError(t, os.MkdirAll(realDir, 0o755)) + + symlinkPath := filepath.Join(tmpDir, "symlink") + if err := os.Symlink(realDir, symlinkPath); err != nil { + t.Skipf("Skipping symlink test: %v", err) + } + + wd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, os.Chdir(wd)) }) + require.NoError(t, os.Chdir(symlinkPath)) + t.Setenv("PWD", symlinkPath) + + // file is built from the SAME logical (unresolved) symlink path as cwd, simulating + // filepath.Abs's $PWD-shortcut behavior in the no-git-root-discovery case. + file := filepath.Join(symlinkPath, "vendor.yaml") + assert.Equal(t, "vendor.yaml", displayPath(file)) +} + func TestBuiltinConfigSchemaMatchesIncludesExistingOptionalDirectories(t *testing.T) { project := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(project, "atmos.d"), 0o700)) diff --git a/internal/exec/vendor_utils.go b/internal/exec/vendor_utils.go index 04dc6618a6..d7c8ea4bc0 100644 --- a/internal/exec/vendor_utils.go +++ b/internal/exec/vendor_utils.go @@ -130,11 +130,26 @@ func ReadAndProcessVendorConfigFile( return vendorConfig, true, foundVendorConfigFile, nil } +// getVendorDirToUse returns the appropriate vendor directory for file resolution. It prefers +// the precomputed VendorDirAbsolutePath (set by AtmosConfigAbsolutePaths, the same mechanism +// cloudposse/atmos#2864 uses for the top-level base_path), falling back to joining the raw +// BasePath/Vendor.BasePath for callers that construct an AtmosConfiguration by hand without +// running it through AtmosConfigAbsolutePaths first (e.g. tests). Mirrors getWorkflowsDirToUse +// and getBasePathToUse (validate_component.go). +func getVendorDirToUse(atmosConfig *schema.AtmosConfiguration) string { + if atmosConfig.VendorDirAbsolutePath != "" { + return atmosConfig.VendorDirAbsolutePath + } + // u.JoinPath (unlike filepath.Join) returns an already-absolute Vendor.BasePath as-is + // instead of nesting it under BasePath. + return u.JoinPath(atmosConfig.BasePath, atmosConfig.Vendor.BasePath) +} + // Helper function to resolve the vendor config file path. func resolveVendorConfigFilePath(atmosConfig *schema.AtmosConfiguration, vendorConfigFile string, checkGlobalConfig bool) string { if checkGlobalConfig && atmosConfig.Vendor.BasePath != "" { if !filepath.IsAbs(atmosConfig.Vendor.BasePath) { - return filepath.Join(atmosConfig.BasePath, atmosConfig.Vendor.BasePath) + return getVendorDirToUse(atmosConfig) } return atmosConfig.Vendor.BasePath } @@ -142,7 +157,7 @@ func resolveVendorConfigFilePath(atmosConfig *schema.AtmosConfiguration, vendorC // Search for the vendor config file foundVendorConfigFile, fileExists := u.SearchConfigFile(vendorConfigFile) if !fileExists { - pathToVendorConfig := filepath.Join(atmosConfig.BasePath, vendorConfigFile) + pathToVendorConfig := filepath.Join(getBasePathToUse(atmosConfig), vendorConfigFile) foundVendorConfigFile, fileExists = u.SearchConfigFile(pathToVendorConfig) if !fileExists { return "" // File does not exist, but this is not an error @@ -159,7 +174,7 @@ func getConfigFiles(path string) ([]string, error) { return nil, ErrVendoringNotConfigured } if os.IsPermission(err) { - return nil, fmt.Errorf("%w '%s'. Please check the file permissions", ErrPermissionDenied, path) + return nil, fmt.Errorf("%w '%s'. Please check the file permissions", ErrPermissionDenied, displayPath(path)) } return nil, fmt.Errorf("An error occurred while accessing the vendoring configuration: %w", err) } @@ -171,7 +186,7 @@ func getConfigFiles(path string) ([]string, error) { } if len(matches) == 0 { - return nil, fmt.Errorf("%w '%s'", ErrNoYAMLConfigFiles, path) + return nil, fmt.Errorf("%w '%s'", ErrNoYAMLConfigFiles, displayPath(path)) } for i, match := range matches { matches[i] = filepath.Join(path, match) @@ -203,7 +218,7 @@ func mergeVendorConfigFiles(configFiles []string) (schema.AtmosVendorConfig, err source := currentConfig.Spec.Sources[i] if source.Component != "" { if sourceMap[source.Component] { - return vendorConfig, fmt.Errorf("%w '%s' found in config file '%s'", ErrDuplicateComponentsFound, source.Component, configFile) + return vendorConfig, fmt.Errorf("%w '%s' found in config file '%s'", ErrDuplicateComponentsFound, source.Component, displayPath(configFile)) } sourceMap[source.Component] = true } @@ -228,9 +243,11 @@ func ExecuteAtmosVendorInternal(params *executeVendorOptions) error { var err error vendorConfigFilePath := filepath.Dir(params.vendorConfigFileName) - logInitialMessage(params.vendorConfigFileName, params.tags) + // displayPath keeps the log message short and machine-independent; params.vendorConfigFileName + // itself stays untouched (and possibly absolute) for the actual file/source resolution below. + logInitialMessage(displayPath(params.vendorConfigFileName), params.tags) if len(params.atmosVendorSpec.Sources) == 0 && len(params.atmosVendorSpec.Imports) == 0 { - return fmt.Errorf("%w '%s'", ErrMissingVendorConfigDefinition, params.vendorConfigFileName) + return fmt.Errorf("%w '%s'", ErrMissingVendorConfigDefinition, displayPath(params.vendorConfigFileName)) } // Process imports and return all sources from all the imports and from `vendor.yaml`. sources, _, err := processVendorImports( @@ -245,7 +262,7 @@ func ExecuteAtmosVendorInternal(params *executeVendorOptions) error { } if len(sources) == 0 { - return fmt.Errorf("%w %s", ErrEmptySources, params.vendorConfigFileName) + return fmt.Errorf("%w %s", ErrEmptySources, displayPath(params.vendorConfigFileName)) } if err := validateTagsAndComponents(sources, params.vendorConfigFileName, params.component, params.tags); err != nil { @@ -289,7 +306,7 @@ func validateTagsAndComponents( if len(lo.Intersect(tags, componentTags)) == 0 { return fmt.Errorf("%w '%s' tagged with the tags %v", - ErrNoComponentsWithTags, vendorConfigFileName, tags) + ErrNoComponentsWithTags, displayPath(vendorConfigFileName), tags) } } @@ -299,12 +316,12 @@ func validateTagsAndComponents( if duplicates := lo.FindDuplicates(components); len(duplicates) > 0 { return fmt.Errorf("%w %v in the vendor config file '%s' and the imports", - ErrDuplicateComponents, duplicates, vendorConfigFileName) + ErrDuplicateComponents, duplicates, displayPath(vendorConfigFileName)) } if component != "" && !slices.Contains(components, component) { return fmt.Errorf("%w component '%s', file '%s'", - ErrComponentNotDefined, component, vendorConfigFileName) + ErrComponentNotDefined, component, displayPath(vendorConfigFileName)) } return nil @@ -575,8 +592,8 @@ func processVendorImports( return nil, nil, fmt.Errorf( "%w '%s' in the vendor config file '%s'. It was already imported in the import chain", ErrDuplicateImport, - imp, - vendorConfigFile, + displayPath(imp), + displayPath(vendorConfigFile), ) } @@ -588,11 +605,11 @@ func processVendorImports( } if slices.Contains(vendorConfig.Spec.Imports, imp) { - return nil, nil, fmt.Errorf("%w file '%s'", ErrVendorConfigSelfImport, imp) + return nil, nil, fmt.Errorf("%w file '%s'", ErrVendorConfigSelfImport, displayPath(imp)) } if len(vendorConfig.Spec.Sources) == 0 && len(vendorConfig.Spec.Imports) == 0 { - return nil, nil, fmt.Errorf("%w '%s'", ErrMissingVendorConfigDefinition, imp) + return nil, nil, fmt.Errorf("%w '%s'", ErrMissingVendorConfigDefinition, displayPath(imp)) } mergedSources, allImports, err = processVendorImports(atmosConfig, imp, vendorConfig.Spec.Imports, mergedSources, allImports) @@ -624,10 +641,10 @@ func validateSourceFields(s *schema.AtmosVendorSource, vendorConfigFileName stri s.File = vendorConfigFileName } if s.Source == "" { - return fmt.Errorf("%w `%s`", ErrSourceMissing, s.File) + return fmt.Errorf("%w `%s`", ErrSourceMissing, displayPath(s.File)) } if len(s.Targets) == 0 { - return fmt.Errorf("%w for source '%s' in file '%s'", ErrTargetsMissing, s.Source, s.File) + return fmt.Errorf("%w for source '%s' in file '%s'", ErrTargetsMissing, s.Source, displayPath(s.File)) } if err := version.ValidateVersionRangeConstraints(s.Version, s.Constraints); err != nil { return err diff --git a/internal/exec/vendor_utils_test.go b/internal/exec/vendor_utils_test.go index 8efd16701b..39c567d49b 100644 --- a/internal/exec/vendor_utils_test.go +++ b/internal/exec/vendor_utils_test.go @@ -12,6 +12,7 @@ import ( errUtils "github.com/cloudposse/atmos/errors" cfg "github.com/cloudposse/atmos/pkg/config" "github.com/cloudposse/atmos/pkg/schema" + u "github.com/cloudposse/atmos/pkg/utils" "github.com/cloudposse/atmos/pkg/vendoring/install" "github.com/cloudposse/atmos/pkg/vendoring/lockfile" ) @@ -1188,6 +1189,105 @@ func TestValidateTagsAndComponents(t *testing.T) { } } +// resolvedTempDir returns a fresh t.TempDir(), chdir'd into (matching testing.T.Chdir's real +// $PWD-setting behavior, which can leave $PWD as an unresolved/logical path -- e.g. macOS's +// /var/folders under the /var -> /private/var symlink), alongside its filepath.EvalSymlinks- +// resolved (physical) form. Config-derived absolute paths in production (VendorDirAbsolutePath, +// WorkflowsDirAbsolutePath) are always physical, resolved via git-root discovery -- so path-leak +// tests must build their file arguments from the resolved form to accurately reproduce the +// logical-cwd-vs-physical-file mismatch displayPath() has to handle. +func resolvedTempDir(t *testing.T) string { + t.Helper() + dir := t.TempDir() + t.Chdir(dir) + resolved, err := filepath.EvalSymlinks(dir) + require.NoError(t, err) + return resolved +} + +// TestGetVendorDirToUse covers both branches: the precomputed VendorDirAbsolutePath (set by +// AtmosConfigAbsolutePaths in real callers) taking precedence, and the raw BasePath/Vendor.BasePath +// join fallback for hand-built AtmosConfiguration values (e.g. in tests) that skip that step. +func TestGetVendorDirToUse(t *testing.T) { + t.Run("uses precomputed VendorDirAbsolutePath when set", func(t *testing.T) { + atmosConfig := &schema.AtmosConfiguration{ + BasePath: "/base", + VendorDirAbsolutePath: "/precomputed/vendor", + Vendor: schema.Vendor{BasePath: "vendor"}, + } + assert.Equal(t, "/precomputed/vendor", getVendorDirToUse(atmosConfig)) + }) + + t.Run("falls back to joining BasePath and Vendor.BasePath", func(t *testing.T) { + base := filepath.Join(string(filepath.Separator), "base") + atmosConfig := &schema.AtmosConfiguration{ + BasePath: base, + Vendor: schema.Vendor{BasePath: "vendor"}, + } + assert.Equal(t, u.JoinPath(base, "vendor"), getVendorDirToUse(atmosConfig)) + }) +} + +// TestResolveVendorConfigFilePath_CheckGlobalConfig covers resolveVendorConfigFilePath's +// checkGlobalConfig branch: an absolute Vendor.BasePath is returned as-is, while a relative one +// resolves via getVendorDirToUse (the precomputed-path case exercised here; the fallback-join +// case is already covered by TestGetVendorDirToUse above). +func TestResolveVendorConfigFilePath_CheckGlobalConfig(t *testing.T) { + t.Run("absolute Vendor.BasePath returned as-is", func(t *testing.T) { + // filepath.IsAbs uses platform semantics -- a hardcoded "/abs/vendor" string literal is + // absolute on POSIX but NOT on Windows (which needs a drive letter or UNC path), so this + // must use an OS-native absolute path (t.TempDir() already returns one) to actually + // exercise the intended branch on every platform. + absVendorDir := filepath.Join(t.TempDir(), "vendor") + atmosConfig := &schema.AtmosConfiguration{Vendor: schema.Vendor{BasePath: absVendorDir}} + got := resolveVendorConfigFilePath(atmosConfig, "vendor.yaml", true) + assert.Equal(t, absVendorDir, got) + }) + + t.Run("relative Vendor.BasePath resolves via getVendorDirToUse", func(t *testing.T) { + precomputed := filepath.Join(t.TempDir(), "precomputed-vendor") + atmosConfig := &schema.AtmosConfiguration{ + VendorDirAbsolutePath: precomputed, + Vendor: schema.Vendor{BasePath: "./vendor.yaml"}, + } + got := resolveVendorConfigFilePath(atmosConfig, "vendor.yaml", true) + assert.Equal(t, precomputed, got) + }) +} + +// TestValidateTagsAndComponents_PathLeak guards against a bug found during a field-test pass on +// cloudposse/atmos#2867/#2868: these error messages interpolated the raw (possibly absolute, +// machine-specific) vendorConfigFileName directly, right next to the "Vendoring from" log line +// that was already fixed with displayPath() in the same PR -- a classic half-fixed pattern. +func TestValidateTagsAndComponents_PathLeak(t *testing.T) { + resolvedDir := resolvedTempDir(t) + vendorConfigFileName := filepath.Join(resolvedDir, "vendor.yaml") + + sources := []schema.AtmosVendorSource{ + {Component: "vpc", Tags: []string{"network"}}, + } + + tests := []struct { + name string + sources []schema.AtmosVendorSource + component string + tags []string + }{ + {name: "ErrNoComponentsWithTags", sources: sources, tags: []string{"nonexistent"}}, + {name: "ErrDuplicateComponents", sources: []schema.AtmosVendorSource{{Component: "dup"}, {Component: "dup"}}}, + {name: "ErrComponentNotDefined", sources: sources, component: "missing"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateTagsAndComponents(tt.sources, vendorConfigFileName, tt.component, tt.tags) + require.Error(t, err) + assert.NotContains(t, err.Error(), resolvedDir, "error must not leak the machine-specific absolute directory") + assert.Contains(t, err.Error(), "vendor.yaml", "error should still name the file, just shortened") + }) + } +} + // TestFilterMaterializedVendorPackages_SkipsMaterializedPackage proves a package with an existing, // matching vendor lock receipt is filtered out (skipped), while a sibling package with no receipt // yet is kept pending. @@ -1301,3 +1401,138 @@ func TestExecuteAtmosVendorInternal_AllMaterialized_NoOp(t *testing.T) { require.NoError(t, err) assert.NoFileExists(t, filepath.Join(target, "extra.tf"), "an already-materialized source must be skipped, not re-copied") } + +// TestExecuteAtmosVendorInternal_PathLeak guards against a bug found during a field-test pass on +// cloudposse/atmos#2867/#2868: ErrMissingVendorConfigDefinition interpolated the raw (possibly +// absolute) vendorConfigFileName directly, unlike the "Vendoring from" log line one statement +// earlier in the same function, which was already fixed with displayPath(). +// +// ErrEmptySources (fmt.Errorf("%w %s", ErrEmptySources, displayPath(...)), a few lines below the +// case tested here) is NOT covered by an equivalent case: processVendorImports requires every +// import in the chain to have non-empty sources or imports, so any input that would make the +// final merged sources list empty hits ErrMissingVendorConfigDefinition somewhere in the +// recursion first (confirmed empirically) -- ErrEmptySources is unreachable via this function's +// public entry point given the current control flow. +func TestExecuteAtmosVendorInternal_PathLeak(t *testing.T) { + resolvedDir := resolvedTempDir(t) + vendorConfigFileName := filepath.Join(resolvedDir, "vendor.yaml") + atmosConfig := &schema.AtmosConfiguration{BasePath: resolvedDir} + + tests := []struct { + name string + opts *executeVendorOptions + }{ + { + name: "ErrMissingVendorConfigDefinition", + opts: &executeVendorOptions{ + vendorConfigFileName: vendorConfigFileName, + atmosConfig: atmosConfig, + atmosVendorSpec: schema.AtmosVendorSpec{}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ExecuteAtmosVendorInternal(tt.opts) + require.Error(t, err) + assert.NotContains(t, err.Error(), resolvedDir, "error must not leak the machine-specific absolute directory") + }) + } +} + +// TestGetConfigFiles_PathLeak guards against the same class of bug for ErrNoYAMLConfigFiles. +func TestGetConfigFiles_PathLeak(t *testing.T) { + resolvedDir := resolvedTempDir(t) + emptyDir := filepath.Join(resolvedDir, "empty") + require.NoError(t, os.MkdirAll(emptyDir, 0o755)) + + _, err := getConfigFiles(emptyDir) + + require.Error(t, err) + assert.ErrorIs(t, err, ErrNoYAMLConfigFiles) + assert.NotContains(t, err.Error(), resolvedDir, "error must not leak the machine-specific absolute directory") +} + +// TestValidateSourceFields_PathLeak guards against the same class of bug for ErrSourceMissing and +// ErrTargetsMissing, whose messages interpolate s.File (defaulted from vendorConfigFileName). +func TestValidateSourceFields_PathLeak(t *testing.T) { + resolvedDir := resolvedTempDir(t) + vendorConfigFileName := filepath.Join(resolvedDir, "vendor.yaml") + + t.Run("ErrSourceMissing", func(t *testing.T) { + err := validateSourceFields(&schema.AtmosVendorSource{}, vendorConfigFileName) + require.Error(t, err) + assert.ErrorIs(t, err, ErrSourceMissing) + assert.NotContains(t, err.Error(), resolvedDir, "error must not leak the machine-specific absolute directory") + }) + + t.Run("ErrTargetsMissing", func(t *testing.T) { + err := validateSourceFields(&schema.AtmosVendorSource{Source: "./somewhere"}, vendorConfigFileName) + require.Error(t, err) + assert.ErrorIs(t, err, ErrTargetsMissing) + assert.NotContains(t, err.Error(), resolvedDir, "error must not leak the machine-specific absolute directory") + }) +} + +// TestProcessVendorImports_PathLeak guards against the same class of bug for +// ErrVendorConfigSelfImport and ErrMissingVendorConfigDefinition (the import-chain variant). +func TestProcessVendorImports_PathLeak(t *testing.T) { + resolvedDir := resolvedTempDir(t) + atmosConfig := &schema.AtmosConfiguration{BasePath: resolvedDir} + + t.Run("ErrVendorConfigSelfImport", func(t *testing.T) { + importFile := filepath.Join(resolvedDir, "self-import.yaml") + content := "apiVersion: atmos/v1\nkind: AtmosVendorConfig\nspec:\n imports:\n - " + importFile + "\n" + require.NoError(t, os.WriteFile(importFile, []byte(content), 0o644)) + + _, _, err := processVendorImports(atmosConfig, filepath.Join(resolvedDir, "vendor.yaml"), []string{importFile}, nil, nil) + + require.Error(t, err) + assert.ErrorIs(t, err, ErrVendorConfigSelfImport) + assert.NotContains(t, err.Error(), resolvedDir, "error must not leak the machine-specific absolute directory") + }) + + t.Run("ErrMissingVendorConfigDefinition", func(t *testing.T) { + importFile := filepath.Join(resolvedDir, "empty-import.yaml") + require.NoError(t, os.WriteFile(importFile, []byte("apiVersion: atmos/v1\nkind: AtmosVendorConfig\nspec: {}\n"), 0o644)) + + _, _, err := processVendorImports(atmosConfig, filepath.Join(resolvedDir, "vendor.yaml"), []string{importFile}, nil, nil) + + require.Error(t, err) + assert.ErrorIs(t, err, ErrMissingVendorConfigDefinition) + assert.NotContains(t, err.Error(), resolvedDir, "error must not leak the machine-specific absolute directory") + }) + + t.Run("ErrDuplicateImport", func(t *testing.T) { + importFile := filepath.Join(resolvedDir, "dup-import.yaml") + require.NoError(t, os.WriteFile(importFile, []byte( + "apiVersion: atmos/v1\nkind: AtmosVendorConfig\nspec:\n sources:\n - component: vpc\n source: ./a\n", + ), 0o644)) + + // The same file listed twice: the second occurrence is already in allImports by the + // time it's processed, so it must be rejected as a duplicate rather than silently + // re-processed. + _, _, err := processVendorImports(atmosConfig, filepath.Join(resolvedDir, "vendor.yaml"), []string{importFile, importFile}, nil, nil) + + require.Error(t, err) + assert.ErrorIs(t, err, ErrDuplicateImport) + assert.NotContains(t, err.Error(), resolvedDir, "error must not leak the machine-specific absolute directory") + }) +} + +// TestMergeVendorConfigFiles_PathLeak guards against the same class of bug for +// ErrDuplicateComponentsFound. +func TestMergeVendorConfigFiles_PathLeak(t *testing.T) { + resolvedDir := resolvedTempDir(t) + configFile := filepath.Join(resolvedDir, "vendor.yaml") + content := "apiVersion: atmos/v1\nkind: AtmosVendorConfig\nspec:\n sources:\n - component: vpc\n source: ./a\n" + + " - component: vpc\n source: ./b\n" + require.NoError(t, os.WriteFile(configFile, []byte(content), 0o644)) + + _, err := mergeVendorConfigFiles([]string{configFile}) + + require.Error(t, err) + assert.ErrorIs(t, err, ErrDuplicateComponentsFound) + assert.NotContains(t, err.Error(), resolvedDir, "error must not leak the machine-specific absolute directory") +} diff --git a/internal/exec/workflow.go b/internal/exec/workflow.go index bc4a821b67..8245fa5bcb 100644 --- a/internal/exec/workflow.go +++ b/internal/exec/workflow.go @@ -142,7 +142,7 @@ func ExecuteWorkflowCmd(cmd *cobra.Command, args []string) error { if u.IsPathAbsolute(workflowFile) { workflowPath = workflowFile } else { - workflowPath = filepath.Join(atmosConfig.BasePath, atmosConfig.Workflows.BasePath, workflowFile) + workflowPath = filepath.Join(getWorkflowsDirToUse(&atmosConfig), workflowFile) } // If the workflow file is specified without an extension, use the default extension @@ -154,7 +154,7 @@ func ExecuteWorkflowCmd(cmd *cobra.Command, args []string) error { if !u.FileExists(workflowPath) { return errUtils.Build(errUtils.ErrWorkflowFileNotFound). - WithExplanationf("The workflow manifest file `%s` does not exist", filepath.ToSlash(workflowPath)). + WithExplanationf("The workflow manifest file `%s` does not exist", filepath.ToSlash(displayPath(workflowPath))). WithExitCode(1). Err() } @@ -175,7 +175,7 @@ func ExecuteWorkflowCmd(cmd *cobra.Command, args []string) error { if workflowManifest.Workflows == nil { return errUtils.Build(errUtils.ErrInvalidWorkflowManifest). - WithExplanationf("The workflow manifest `%s` must be a map with the top-level `workflows:` key", filepath.ToSlash(workflowPath)). + WithExplanationf("The workflow manifest `%s` must be a map with the top-level `workflows:` key", filepath.ToSlash(displayPath(workflowPath))). WithHint("Add a top-level 'workflows:' key to the manifest file"). WithExitCode(1). Err() diff --git a/internal/exec/workflow_utils.go b/internal/exec/workflow_utils.go index 6ac15ebc63..3ff8dc0301 100644 --- a/internal/exec/workflow_utils.go +++ b/internal/exec/workflow_utils.go @@ -944,8 +944,14 @@ func ExecuteWorkflow( stepErr := err if !errors.Is(err, errUtils.ErrInvalidWorkflowStepType) { stepErr = buildWorkflowStepError(err, &workflowStepErrorContext{ - WorkflowPath: workflowPath, - WorkflowBasePath: atmosConfig.Workflows.BasePath, + WorkflowPath: workflowPath, + // Must be the SAME anchor workflowPath was actually joined against + // (workflow.go), not the raw, always-relative atmosConfig.Workflows.BasePath + // -- otherwise the TrimPrefix below in buildWorkflowStepError silently fails + // to strip it whenever workflowPath ends up absolute (e.g. via the + // precomputed WorkflowsDirAbsolutePath), leaving the resume-command hint + // showing a garbled path instead of the plain workflow file name. + WorkflowBasePath: getWorkflowsDirToUse(&atmosConfig), Workflow: workflow, StepName: step.Name, Command: command, @@ -1098,23 +1104,29 @@ func ExecuteDescribeWorkflows( return nil, nil, nil, errUtils.ErrWorkflowBasePathNotConfigured } - // If `workflows.base_path` is a relative path, join it with `stacks.base_path` + // If `workflows.base_path` is a relative path, resolve it via getWorkflowsDirToUse + // (prefers the precomputed WorkflowsDirAbsolutePath over the raw, possibly still-relative + // atmosConfig.BasePath -- same bug shape cloudposse/atmos#2864 fixed for the top-level + // base_path itself). var workflowsDir string if u.IsPathAbsolute(atmosConfig.Workflows.BasePath) { workflowsDir = atmosConfig.Workflows.BasePath } else { - workflowsDir = filepath.Join(atmosConfig.BasePath, atmosConfig.Workflows.BasePath) + workflowsDir = getWorkflowsDirToUse(&atmosConfig) } isDirectory, err := u.IsDirectory(workflowsDir) if err != nil || !isDirectory { - return nil, nil, nil, fmt.Errorf("the workflow directory '%s' does not exist. Review 'workflows.base_path' in 'atmos.yaml'", workflowsDir) + return nil, nil, nil, fmt.Errorf("%w: '%s'. Review 'workflows.base_path' in 'atmos.yaml'", + errUtils.ErrWorkflowDirectoryDoesNotExist, displayPath(workflowsDir)) } files, err := u.GetAllYamlFilesInDir(workflowsDir) if err != nil { - return nil, nil, nil, fmt.Errorf("error reading the directory '%s' defined in 'workflows.base_path' in 'atmos.yaml': %v", - atmosConfig.Workflows.BasePath, err) + // Report workflowsDir (the directory actually searched), not the raw, possibly-relative + // atmosConfig.Workflows.BasePath, which can silently differ from where Atmos looked. + return nil, nil, nil, fmt.Errorf("%w: '%s' defined in 'workflows.base_path' in 'atmos.yaml': %w", + errUtils.ErrReadDirectory, displayPath(workflowsDir), err) } for _, f := range files { @@ -1122,7 +1134,7 @@ func ExecuteDescribeWorkflows( if u.IsPathAbsolute(atmosConfig.Workflows.BasePath) { workflowPath = filepath.Join(atmosConfig.Workflows.BasePath, f) } else { - workflowPath = filepath.Join(atmosConfig.BasePath, atmosConfig.Workflows.BasePath, f) + workflowPath = filepath.Join(getWorkflowsDirToUse(&atmosConfig), f) } fileContent, err := os.ReadFile(workflowPath) diff --git a/pkg/config/base_path_resolution_test.go b/pkg/config/base_path_resolution_test.go index 296de2521b..a9108d3505 100644 --- a/pkg/config/base_path_resolution_test.go +++ b/pkg/config/base_path_resolution_test.go @@ -739,8 +739,17 @@ func TestInitCliConfig_BasePathSource_SetForEnvVar(t *testing.T) { "BasePathSource should be 'runtime' when ATMOS_BASE_PATH env var is set") } -// TestFindAllStackConfigsInPathsForStack_ErrorWrapping verifies that when GetGlobMatches -// fails, the error is wrapped with the ErrFailedToFindImport sentinel. +// TestFindAllStackConfigsInPathsForStack_ErrorWrapping verifies that when every +// included_paths entry matches nothing (e.g. the whole stacks directory doesn't exist), +// the function still errors -- with ErrNoStackManifestsFound, not ErrFailedToFindImport. +// +// Prior to cloudposse/atmos#2867's fix, a single entry matching nothing surfaced +// ErrFailedToFindImport directly from inside the per-path loop, which also meant a SECOND, +// valid included_paths entry earlier in the list would have its already-found matches +// discarded by this same hard error. Now, an individual entry matching nothing is treated as +// "nothing here, keep looking" and only the aggregate "nothing matched at all" case (this +// test: the only entry present matches nothing) errors, with a sentinel describing that +// outcome directly instead of leaking the glob-matching implementation's own error identity. func TestFindAllStackConfigsInPathsForStack_ErrorWrapping(t *testing.T) { atmosConfig := schema.AtmosConfiguration{ StacksBaseAbsolutePath: filepath.Join(os.TempDir(), "nonexistent-stacks-dir-test"), @@ -759,11 +768,37 @@ func TestFindAllStackConfigsInPathsForStack_ErrorWrapping(t *testing.T) { require.Error(t, err) - assert.True(t, errors.Is(err, errUtils.ErrFailedToFindImport), - "Error should wrap ErrFailedToFindImport, got: %v", err) + assert.True(t, errors.Is(err, errUtils.ErrNoStackManifestsFound), + "Error should wrap ErrNoStackManifestsFound, got: %v", err) +} + +// TestFindAllStackConfigsInPathsForStack_GenuineGlobError verifies that a genuinely invalid +// glob pattern (not just "matched nothing") still aborts and surfaces the underlying error, +// rather than being tolerated the way an empty-match ErrFailedToFindImport now is post-#2867. +func TestFindAllStackConfigsInPathsForStack_GenuineGlobError(t *testing.T) { + atmosConfig := schema.AtmosConfiguration{ + StacksBaseAbsolutePath: filepath.Join(os.TempDir(), "nonexistent-stacks-dir-badpattern"), + } + + includeStackPaths := []string{ + filepath.Join(os.TempDir(), "nonexistent-stacks-dir-badpattern", "[invalid"), + } + + _, _, _, err := FindAllStackConfigsInPathsForStack( + atmosConfig, + "test-stack", + includeStackPaths, + nil, + ) + + require.Error(t, err) + assert.False(t, errors.Is(err, errUtils.ErrNoStackManifestsFound), + "a genuine glob syntax error must not be reported as ErrNoStackManifestsFound") } -// TestFindAllStackConfigsInPaths_ErrorWrapping verifies error wrapping in the non-stack variant. +// TestFindAllStackConfigsInPaths_ErrorWrapping verifies error wrapping in the non-stack +// variant. See TestFindAllStackConfigsInPathsForStack_ErrorWrapping above for why this is +// ErrNoStackManifestsFound rather than ErrFailedToFindImport post-#2867. func TestFindAllStackConfigsInPaths_ErrorWrapping(t *testing.T) { atmosConfig := schema.AtmosConfiguration{ StacksBaseAbsolutePath: filepath.Join(os.TempDir(), "nonexistent-stacks-dir-test2"), @@ -781,6 +816,28 @@ func TestFindAllStackConfigsInPaths_ErrorWrapping(t *testing.T) { require.Error(t, err) - assert.True(t, errors.Is(err, errUtils.ErrFailedToFindImport), - "Error should wrap ErrFailedToFindImport, got: %v", err) + assert.True(t, errors.Is(err, errUtils.ErrNoStackManifestsFound), + "Error should wrap ErrNoStackManifestsFound, got: %v", err) +} + +// TestFindAllStackConfigsInPaths_GenuineGlobError is the non-stack-variant counterpart of +// TestFindAllStackConfigsInPathsForStack_GenuineGlobError above. +func TestFindAllStackConfigsInPaths_GenuineGlobError(t *testing.T) { + atmosConfig := schema.AtmosConfiguration{ + StacksBaseAbsolutePath: filepath.Join(os.TempDir(), "nonexistent-stacks-dir-badpattern2"), + } + + includeStackPaths := []string{ + filepath.Join(os.TempDir(), "nonexistent-stacks-dir-badpattern2", "[invalid"), + } + + _, _, err := FindAllStackConfigsInPaths( + &atmosConfig, + includeStackPaths, + nil, + ) + + require.Error(t, err) + assert.False(t, errors.Is(err, errUtils.ErrNoStackManifestsFound), + "a genuine glob syntax error must not be reported as ErrNoStackManifestsFound") } diff --git a/pkg/config/config.go b/pkg/config/config.go index b1f324fcf3..1ea2a75ea9 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -524,6 +524,25 @@ func AtmosConfigAbsolutePaths(atmosConfig *schema.AtmosConfiguration) error { } atmosConfig.HelmDirAbsolutePath = helmDirAbsPath + // Convert Vendor base path to an absolute path. Consumers previously re-joined the raw + // (possibly still-relative) atmosConfig.BasePath at call time instead of using a + // precomputed absolute path -- the same bug shape #2864 fixed for the top-level + // base_path itself, just not yet applied here. + vendorBasePath := u.JoinPath(atmosBasePathAbs, atmosConfig.Vendor.BasePath) + vendorDirAbsPath, err := absPathOrError(vendorBasePath, "vendor base path") + if err != nil { + return err + } + atmosConfig.VendorDirAbsolutePath = vendorDirAbsPath + + // Convert Workflows base path to an absolute path (same rationale as Vendor above). + workflowsBasePath := u.JoinPath(atmosBasePathAbs, atmosConfig.Workflows.BasePath) + workflowsDirAbsPath, err := absPathOrError(workflowsBasePath, "workflows base path") + if err != nil { + return err + } + atmosConfig.WorkflowsDirAbsolutePath = workflowsDirAbsPath + return nil } diff --git a/pkg/config/config_edit.go b/pkg/config/config_edit.go index 1b788ac7ca..8b5f438110 100644 --- a/pkg/config/config_edit.go +++ b/pkg/config/config_edit.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/cloudposse/atmos/pkg/perf" "github.com/cloudposse/atmos/pkg/schema" @@ -14,6 +15,27 @@ import ( // ErrNoEditableConfig is returned when an editable atmos.yaml file cannot be located. var ErrNoEditableConfig = errors.New("could not locate an editable atmos.yaml; pass --config to target a specific file") +// ErrAmbiguousConfigFile is returned when a config-editing command (config set/delete/format, +// mcp config add/remove) is given more than one --config file. Unlike `config get`, which reads +// the fully-merged effective value across every --config file, these commands mutate exactly one +// concrete file on disk -- silently picking the first (or last) would edit a file the user may +// not have intended, while the actual effective config (what every other atmos command uses) +// stays unchanged whenever a later file also sets the same key (cloudposse/atmos#2867). +var ErrAmbiguousConfigFile = errors.New("multiple --config files given; specify exactly one file to edit") + +// ResolveConfigOverride validates that cfgFiles names at most one file, returning it (or "" if +// none), since config set/delete/format and mcp config add/remove edit a single concrete file +// and cannot safely guess which of several --config files to target. +func ResolveConfigOverride(cfgFiles []string) (string, error) { + if len(cfgFiles) > 1 { + return "", fmt.Errorf("%w: %s", ErrAmbiguousConfigFile, strings.Join(cfgFiles, ", ")) + } + if len(cfgFiles) == 1 { + return cfgFiles[0], nil + } + return "", nil +} + // configFileCandidates lists the config file names to probe in a directory, in // precedence order (atmos.yaml before the dotfile variant). var configFileCandidates = []string{AtmosConfigFileName, DotAtmosConfigFileName} diff --git a/pkg/config/config_edit_test.go b/pkg/config/config_edit_test.go index 6ec852a13f..45f30d724e 100644 --- a/pkg/config/config_edit_test.go +++ b/pkg/config/config_edit_test.go @@ -100,3 +100,31 @@ func TestResolveEditableConfigFile_CurrentDirectory(t *testing.T) { wantResolved, _ := filepath.EvalSymlinks(file) assert.Equal(t, wantResolved, gotResolved) } + +// TestResolveConfigOverride covers all three branches: zero, one, and multiple --config files. +func TestResolveConfigOverride(t *testing.T) { + tests := []struct { + name string + cfgFiles []string + want string + wantErr bool + }{ + {name: "no files", cfgFiles: nil, want: ""}, + {name: "single file", cfgFiles: []string{"a.yaml"}, want: "a.yaml"}, + {name: "multiple files are ambiguous", cfgFiles: []string{"a.yaml", "b.yaml"}, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ResolveConfigOverride(tt.cfgFiles) + if tt.wantErr { + require.ErrorIs(t, err, ErrAmbiguousConfigFile) + assert.Contains(t, err.Error(), "a.yaml") + assert.Contains(t, err.Error(), "b.yaml") + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index a0c58efb1a..0a2bdbc0b2 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -538,6 +538,27 @@ func TestAtmosConfigAbsolutePaths(t *testing.T) { assert.Equal(t, absPath, config.Components.Helmfile.BasePath) assert.Equal(t, absPath, config.Stacks.BasePath) }) + + // TestAtmosConfigAbsolutePaths_VendorAndWorkflows guards against the same bug shape + // cloudposse/atmos#2864 fixed for the top-level base_path: Vendor.BasePath and + // Workflows.BasePath were never centralized into an absolute field, so consumers + // re-joined the raw (possibly still-relative) atmosConfig.BasePath at call time instead. + t.Run("computes vendor and workflows absolute paths", func(t *testing.T) { + baseDir := filepath.Join(os.TempDir(), "atmos-vendor-workflows-test") + config := &schema.AtmosConfiguration{ + BasePath: baseDir, + Vendor: schema.Vendor{BasePath: "vendor.yaml"}, + Workflows: schema.Workflows{ + BasePath: "stacks/workflows", + }, + } + + err := AtmosConfigAbsolutePaths(config) + assert.NoError(t, err) + + assert.Equal(t, filepath.Join(baseDir, "vendor.yaml"), config.VendorDirAbsolutePath) + assert.Equal(t, filepath.Join(baseDir, "stacks", "workflows"), config.WorkflowsDirAbsolutePath) + }) } // Helper functions. diff --git a/pkg/config/import_base_path_test.go b/pkg/config/import_base_path_test.go index 3e69d07fc9..631327ead8 100644 --- a/pkg/config/import_base_path_test.go +++ b/pkg/config/import_base_path_test.go @@ -155,7 +155,8 @@ func TestMergeFiles_InheritedBasePathUsesDeclaringConfigDir(t *testing.T) { v := viper.New() v.SetConfigType(yamlType) - require.NoError(t, mergeFiles(v, []string{baseConfig, overlayConfig})) + _, err := mergeFiles(v, []string{baseConfig, overlayConfig}) + require.NoError(t, err) assert.Equal(t, 157, v.GetInt("settings.terminal.max_width")) } @@ -184,7 +185,8 @@ func mergeFilesImportedBasePathCase(t *testing.T, baseYAML, defaultsYAML, extraS v := viper.New() v.SetConfigType(yamlType) - require.NoError(t, mergeFiles(v, []string{baseConfig, overlayConfig})) + _, err := mergeFiles(v, []string{baseConfig, overlayConfig}) + require.NoError(t, err) assert.Equal(t, wantWidth, v.GetInt("settings.terminal.max_width")) } @@ -218,7 +220,8 @@ func TestMergeFiles_EmptyBasePathUsesImportingConfigDir(t *testing.T) { v := viper.New() v.SetConfigType(yamlType) - require.NoError(t, mergeFiles(v, []string{filepath.Join(baseConfigDir, "base.yaml"), overlayConfig})) + _, err := mergeFiles(v, []string{filepath.Join(baseConfigDir, "base.yaml"), overlayConfig}) + require.NoError(t, err) assert.Equal(t, 173, v.GetInt("settings.terminal.max_width")) } @@ -238,7 +241,8 @@ func TestMergeFiles_ImportMergeErrorIsNonFatal(t *testing.T) { v := viper.New() v.SetConfigType(yamlType) - require.NoError(t, mergeFiles(v, []string{cfg})) + _, err := mergeFiles(v, []string{cfg}) + require.NoError(t, err) assert.Equal(t, 181, v.GetInt("settings.terminal.max_width")) } diff --git a/pkg/config/load.go b/pkg/config/load.go index e7fe0ee97b..aa2e0e67bc 100644 --- a/pkg/config/load.go +++ b/pkg/config/load.go @@ -125,12 +125,17 @@ func ParseProfilesFromOsArgs(args []string) []string { return result } -// parseViperProfilesFromEnv handles Viper's quirky environment variable parsing for StringSlice. -// Viper does NOT parse comma-separated environment variables correctly: +// FixViperEnvStringSliceQuirk handles Viper's quirky environment variable parsing for +// StringSlice flags in general -- not just --profile/ATMOS_PROFILE, which is where this was +// first found and fixed. Viper does NOT parse comma-separated environment variables correctly: // - "dev,staging,prod" → []string{"dev,staging,prod"} (single element, NOT split) // - "dev staging prod" → []string{"dev", "staging", "prod"} (splits on whitespace) // - " dev , staging " → []string{"dev", ",", "staging"} (splits on whitespace, keeps commas!) -func parseViperProfilesFromEnv(profiles []string) []string { +// +// Callers should apply this only to values actually sourced from an environment variable (e.g. +// after confirming via os.LookupEnv) -- CLI-flag-sourced values are already parsed correctly by +// pflag/Cobra and must not be re-split. +func FixViperEnvStringSliceQuirk(profiles []string) []string { var parsed []string for _, p := range profiles { @@ -290,6 +295,29 @@ func getProfilesFromFallbacks() ([]string, string) { return nil, "" } +// getConfigSelectionFromFlagsOrEnv retrieves --config/--config-path/--base-path selection +// directly from os.Args/env, for use as a LoadConfig fallback when the caller's +// ConfigAndStacksInfo didn't carry a selection. +// +// Mirrors getProfilesFromFlagsOrEnv below. Dozens of call sites across the codebase call +// InitCliConfig with an empty schema.ConfigAndStacksInfo{}, which previously silently +// discarded whatever --config/--config-path/--base-path (or ATMOS_CONFIG/ATMOS_CONFIG_PATH/ +// ATMOS_BASE_PATH) selection cmd/root.go correctly parsed once at startup via +// EarlyConfigAndStacksInfoFromArgs -- breaking any internal re-invocation of InitCliConfig +// mid-command (cloudposse/atmos#2868, e.g. `atmos --config terraform plan` falling +// back to plain auto-discovery and failing with "failed to find import"). +// ParseConfigSelectionFromOsArgs/ConfigSelectionFromEnv are pure os.Args/os.Getenv parsers +// (unlike the profile fallback, they don't need a global-viper leg first, since --config/ +// --config-path/--base-path are never bound through pflag/viper the way --profile is), so +// this is safe to call unconditionally as a fallback -- it's the exact same mechanism +// EarlyConfigAndStacksInfoFromArgs already uses for the one call site that gets this right +// today. +func getConfigSelectionFromFlagsOrEnv() ConfigSelection { + sel := ParseConfigSelectionFromOsArgs(os.Args) + sel.applyFallbacks(ConfigSelectionFromEnv()) + return sel +} + // getProfilesFromFlagsOrEnv retrieves profiles from --profile flag or ATMOS_PROFILE env var. // This is a helper function to reduce nesting complexity in LoadConfig. // Returns profiles and source ("env" or "flag") for logging. @@ -315,7 +343,7 @@ func getProfilesFromFlagsOrEnv() ([]string, string) { // Environment variable path - needs special parsing for Viper quirks. if envSet && len(profiles) > 0 { - parsed := parseViperProfilesFromEnv(profiles) + parsed := FixViperEnvStringSliceQuirk(profiles) if len(parsed) > 0 { return parsed, "env" } @@ -360,54 +388,76 @@ func LoadConfig(configAndStacksInfo *schema.ConfigAndStacksInfo) (schema.AtmosCo if runtimeBasePath := resolveRuntimeBasePath(configAndStacksInfo); runtimeBasePath != "" { v.Set(runtimeBasePathOverrideKey, runtimeBasePath) } + // Fall back to --config/--config-path/--base-path parsed directly from os.Args/env when + // the caller passed a ConfigAndStacksInfo that didn't carry a selection at all. This + // mirrors the profile fallback below (getProfilesFromFlagsOrEnv) and fixes + // cloudposse/atmos#2868: many internal call sites across the codebase call + // InitCliConfig(schema.ConfigAndStacksInfo{}, ...) with an empty struct, which otherwise + // silently drops a --config/--config-path/--base-path selection that a prior, correctly- + // populated InitCliConfig call in the same process already honored. + if len(configAndStacksInfo.AtmosConfigFilesFromArg) == 0 && + len(configAndStacksInfo.AtmosConfigDirsFromArg) == 0 && + configAndStacksInfo.AtmosBasePath == "" { + if sel := getConfigSelectionFromFlagsOrEnv(); len(sel.Config) > 0 || len(sel.ConfigPath) > 0 || sel.BasePath != "" { + configAndStacksInfo.AtmosConfigFilesFromArg = sel.Config + configAndStacksInfo.AtmosConfigDirsFromArg = sel.ConfigPath + configAndStacksInfo.AtmosBasePath = sel.BasePath + log.Debug("Config selection loaded from os.Args/env fallback", + "config", sel.Config, "config_path", sel.ConfigPath, "base_path", sel.BasePath) + } + } + // Whether config was selected via --config/--config-path: merge it and fall through into + // the same profile-loading/edition/final-unmarshal tail every other config source uses + // below, instead of returning immediately (see mergeConfigFromCLIArgs' doc comment). if len(configAndStacksInfo.AtmosConfigFilesFromArg) > 0 || len(configAndStacksInfo.AtmosConfigDirsFromArg) > 0 { - err := loadConfigFromCLIArgs(v, configAndStacksInfo, &atmosConfig) + configPaths, profilesBasePathConfigDir, err := mergeConfigFromCLIArgs(v, configAndStacksInfo) if err != nil { return atmosConfig, err } - return atmosConfig, nil - } - - // Load configuration from different sources. - if err := loadConfigSources(v, configAndStacksInfo); err != nil { - return atmosConfig, err - } - // If no config file is used, fall back to the default CLI config. - if v.ConfigFileUsed() == "" { - log.Debug("'atmos.yaml' CLI config was not found", "paths", "system dir, home dir, current dir, parent dirs, ENV vars") - log.Debug("Refer to https://atmos.tools/cli/configuration for details on how to configure 'atmos.yaml'") - log.Debug("Using the default CLI config") - - if err := mergeDefaultConfig(v); err != nil { + atmosConfig.CliConfigPath = connectPaths(configPaths) + atmosConfig.ProfilesBasePathConfigDir = profilesBasePathConfigDir + } else { + // Load configuration from different sources. + if err := loadConfigSources(v, configAndStacksInfo); err != nil { return atmosConfig, err } + // If no config file is used, fall back to the default CLI config. + if v.ConfigFileUsed() == "" { + log.Debug("'atmos.yaml' CLI config was not found", "paths", "system dir, home dir, current dir, parent dirs, ENV vars") + log.Debug("Refer to https://atmos.tools/cli/configuration for details on how to configure 'atmos.yaml'") + log.Debug("Using the default CLI config") - // Also search git root for .atmos.d even with default config. - // This enables custom commands defined in .atmos.d at the repo root - // to work when running from any subdirectory. - gitRoot, err := u.ProcessTagGitRoot("!repo-root .") - if err == nil && gitRoot != "" && gitRoot != "." { - log.Debug("Loading .atmos.d from git root", "path", gitRoot) - if err := mergeDefaultImports(gitRoot, v); err != nil { - if !errors.Is(err, errUtils.ErrAtmosDirConfigNotFound) { - return atmosConfig, err + if err := mergeDefaultConfig(v); err != nil { + return atmosConfig, err + } + + // Also search git root for .atmos.d even with default config. + // This enables custom commands defined in .atmos.d at the repo root + // to work when running from any subdirectory. + gitRoot, err := u.ProcessTagGitRoot("!repo-root .") + if err == nil && gitRoot != "" && gitRoot != "." { + log.Debug("Loading .atmos.d from git root", "path", gitRoot) + if err := mergeDefaultImports(gitRoot, v); err != nil { + if !errors.Is(err, errUtils.ErrAtmosDirConfigNotFound) { + return atmosConfig, err + } + log.Trace("Failed to load .atmos.d from git root", "path", gitRoot, "error", err) + // Non-fatal: directory doesn't exist, continue with default config. } - log.Trace("Failed to load .atmos.d from git root", "path", gitRoot, "error", err) - // Non-fatal: directory doesn't exist, continue with default config. } } - } - if v.ConfigFileUsed() != "" { - // get dir of atmosConfigFilePath - atmosConfigDir := filepath.Dir(v.ConfigFileUsed()) - atmosConfig.CliConfigPath = atmosConfigDir - // Set the CLI config path in the atmosConfig struct - if !filepath.IsAbs(atmosConfig.CliConfigPath) { - absPath, err := filepath.Abs(atmosConfig.CliConfigPath) - if err != nil { - return atmosConfig, err + if v.ConfigFileUsed() != "" { + // get dir of atmosConfigFilePath + atmosConfigDir := filepath.Dir(v.ConfigFileUsed()) + atmosConfig.CliConfigPath = atmosConfigDir + // Set the CLI config path in the atmosConfig struct + if !filepath.IsAbs(atmosConfig.CliConfigPath) { + absPath, err := filepath.Abs(atmosConfig.CliConfigPath) + if err != nil { + return atmosConfig, err + } + atmosConfig.CliConfigPath = absPath } - atmosConfig.CliConfigPath = absPath } } setEnv(v) @@ -457,6 +507,10 @@ func LoadConfig(configAndStacksInfo *schema.ConfigAndStacksInfo) (schema.AtmosCo // This ensures relative profile paths resolve against the actual CLI config directory // rather than the current working directory. tempConfig.CliConfigPath = atmosConfig.CliConfigPath + // Same for the directory that declared profiles.base_path (if any), so + // discoverProfileLocations resolves it against the correct --config file's directory + // rather than always the first one (cloudposse/atmos#2867). + tempConfig.ProfilesBasePathConfigDir = atmosConfig.ProfilesBasePathConfigDir // Load each profile in order (left-to-right precedence). if err := loadProfiles(v, configAndStacksInfo.ProfilesFromArg, &tempConfig); err != nil { @@ -1625,6 +1679,37 @@ func importBasePathDeclaration(content []byte) (bool, string, error) { return false, "", nil } +// declaresProfilesBasePath reports whether the given config file content declares a top-level +// `profiles.base_path` key, mirroring importBasePathDeclaration's approach but walking one level +// deeper into the `profiles:` mapping. Used to track which specific --config file declared +// profiles.base_path when multiple files are given, so a relative value resolves against that +// file's directory instead of always the first --config file's (cloudposse/atmos#2867). +func declaresProfilesBasePath(content []byte) (bool, error) { + var root goyaml.Node + if err := goyaml.Unmarshal(content, &root); err != nil { + return false, err + } + if len(root.Content) == 0 || root.Content[0].Kind != goyaml.MappingNode { + return false, nil + } + for i := 0; i < len(root.Content[0].Content); i += 2 { + if root.Content[0].Content[i].Value != "profiles" { + continue + } + profilesNode := root.Content[0].Content[i+1] + if profilesNode.Kind != goyaml.MappingNode { + return false, nil + } + for j := 0; j < len(profilesNode.Content); j += 2 { + if profilesNode.Content[j].Value == "base_path" { + return true, nil + } + } + return false, nil + } + return false, nil +} + // parseBasePathDeclaration is a seam over importBasePathDeclaration. Call sites that // re-parse content Viper has already validated route through it so tests can inject a // parse failure and exercise the error-propagation path. diff --git a/pkg/config/load_config_args.go b/pkg/config/load_config_args.go index 677c75d2ae..2f68d931d5 100644 --- a/pkg/config/load_config_args.go +++ b/pkg/config/load_config_args.go @@ -12,19 +12,37 @@ import ( "github.com/cloudposse/atmos/pkg/schema" ) -// loadConfigFromCLIArgs handles the loading of configurations provided via --config-path and --config. -func loadConfigFromCLIArgs(v *viper.Viper, configAndStacksInfo *schema.ConfigAndStacksInfo, atmosConfig *schema.AtmosConfiguration) error { +// mergeConfigFromCLIArgs merges config sources selected via --config/--config-path into v, +// returning the directories that contributed configuration (for CliConfigPath assembly) and the +// directory of whichever --config file declared profiles.base_path, if any (for +// discoverProfileLocations to resolve it against the correct file's directory, not just the +// first --config file's -- cloudposse/atmos#2867). +// +// Split out from loadConfigFromCLIArgs so LoadConfig's main flow can merge the CLI-selected +// config and then fall through into the same profile-loading/edition/final-unmarshal tail +// every other config source uses, instead of returning immediately. Previously, using +// --config/--config-path meant LoadConfig returned right after loadConfigFromCLIArgs' own, +// separate unmarshal -- silently skipping profile loading entirely (--config and --profile +// could not be combined, cloudposse/atmos#2867/#2868 audit finding) along with the +// profiles.base_path/vendor-updater/container-runtime config bridging the shared tail +// performs for every other config source. +func mergeConfigFromCLIArgs(v *viper.Viper, configAndStacksInfo *schema.ConfigAndStacksInfo) ([]string, string, error) { log.Debug("loading config from command line arguments") configFilesArgs := configAndStacksInfo.AtmosConfigFilesFromArg configDirsArgs := configAndStacksInfo.AtmosConfigDirsFromArg var configPaths []string + profilesBasePathConfigDir := "" - // Merge all config from --config files + // Merge all config from --config files. --config-path directories are always merged AFTER + // --config files below, regardless of which flag appears later on the command line -- + // documented (website/docs/cli/configuration/configuration.mdx) and intentional, not a bug. if len(configFilesArgs) > 0 { - if err := mergeFiles(v, configFilesArgs); err != nil { - return err + dir, err := mergeFiles(v, configFilesArgs) + if err != nil { + return nil, "", err } + profilesBasePathConfigDir = dir for _, configFilePath := range configFilesArgs { configPaths = append(configPaths, filepath.Dir(configFilePath)) } @@ -34,7 +52,7 @@ func loadConfigFromCLIArgs(v *viper.Viper, configAndStacksInfo *schema.ConfigAnd if len(configDirsArgs) > 0 { paths, err := mergeConfigFromDirectories(v, configDirsArgs) if err != nil { - return err + return nil, "", err } configPaths = append(configPaths, paths...) } @@ -42,13 +60,26 @@ func loadConfigFromCLIArgs(v *viper.Viper, configAndStacksInfo *schema.ConfigAnd // Check if any config files were found from command line arguments if len(configPaths) == 0 { log.Debug("no config files found from command line arguments") - return fmt.Errorf("%w: no config files found from command line arguments (--config or --config-path)", errUtils.ErrAtmosArgConfigNotFound) + return nil, "", fmt.Errorf("%w: no config files found from command line arguments (--config or --config-path)", errUtils.ErrAtmosArgConfigNotFound) + } + + return configPaths, profilesBasePathConfigDir, nil +} + +// loadConfigFromCLIArgs handles the loading of configurations provided via --config-path and +// --config, unmarshaling directly into atmosConfig on its own (without profile support or the +// shared-tail config bridging LoadConfig's main flow performs -- see mergeConfigFromCLIArgs). +// Kept as a standalone entry point for direct callers/tests exercising --config in isolation. +func loadConfigFromCLIArgs(v *viper.Viper, configAndStacksInfo *schema.ConfigAndStacksInfo, atmosConfig *schema.AtmosConfiguration) error { + configPaths, profilesBasePathConfigDir, err := mergeConfigFromCLIArgs(v, configAndStacksInfo) + if err != nil { + return err } setEnv(v) // Apply the edition pin (if any) before unmarshaling, same as the main - // LoadConfig flow (this path returns early and skips that hook). + // LoadConfig flow (this path skips that hook otherwise). if err := applyEditionDefaults(v); err != nil { return err } @@ -79,33 +110,59 @@ func loadConfigFromCLIArgs(v *viper.Viper, configAndStacksInfo *schema.ConfigAnd } atmosConfig.CliConfigPath = connectPaths(configPaths) + atmosConfig.ProfilesBasePathConfigDir = profilesBasePathConfigDir return nil } -// mergeFiles merges config files from the provided paths. -func mergeFiles(v *viper.Viper, configFilePaths []string) error { +// trackConfigDirs inspects a single config file's raw content for declared base_path and +// profiles.base_path keys, returning the (possibly updated) basePathConfigDir and +// profilesBasePathConfigDir accumulators for mergeFiles' per-file loop. Split out from +// mergeFiles to keep its cognitive complexity down. +func trackConfigDirs(content []byte, configPath, configDir, basePathConfigDir, profilesBasePathConfigDir string) (string, string, error) { + declaresBasePath, _, err := importBasePathDeclaration(content) + if err != nil { + return "", "", fmt.Errorf("%w: %s: %w", errUtils.ErrMergeConfiguration, configPath, err) + } + if declaresBasePath || basePathConfigDir == "" { + basePathConfigDir = configDir + } + + declaresProfilesPath, err := declaresProfilesBasePath(content) + if err != nil { + return "", "", fmt.Errorf("%w: %s: %w", errUtils.ErrMergeConfiguration, configPath, err) + } + if declaresProfilesPath { + profilesBasePathConfigDir = configDir + } + + return basePathConfigDir, profilesBasePathConfigDir, nil +} + +// mergeFiles merges config files from the provided paths, returning the directory of the file +// that declared profiles.base_path (empty if none did), for discoverProfileLocations to resolve +// a relative profiles.base_path against the correct file's directory (cloudposse/atmos#2867) +// instead of always the first --config file's. +func mergeFiles(v *viper.Viper, configFilePaths []string) (string, error) { err := validatedIsFiles(configFilePaths) if err != nil { - return err + return "", err } basePathConfigDir := "" + profilesBasePathConfigDir := "" for _, configPath := range configFilePaths { configDir := filepath.Dir(configPath) content, readErr := os.ReadFile(configPath) if readErr != nil { - return fmt.Errorf("%w: %s: %w", errUtils.ErrReadConfig, configPath, readErr) - } - declaresBasePath, _, parseErr := importBasePathDeclaration(content) - if parseErr != nil { - return fmt.Errorf("%w: %s: %w", errUtils.ErrMergeConfiguration, configPath, parseErr) + return "", fmt.Errorf("%w: %s: %w", errUtils.ErrReadConfig, configPath, readErr) } - if declaresBasePath || basePathConfigDir == "" { - basePathConfigDir = configDir + basePathConfigDir, profilesBasePathConfigDir, err = trackConfigDirs(content, configPath, configDir, basePathConfigDir, profilesBasePathConfigDir) + if err != nil { + return "", err } err := mergeConfigFile(configPath, v) if err != nil { log.Debug("error loading config file", "path", configPath, "error", err) - return err + return "", err } log.Debug("config file merged", "path", configPath) if err := mergeDefaultImports(configPath, v); err != nil { @@ -123,7 +180,7 @@ func mergeFiles(v *viper.Viper, configFilePaths []string) error { basePathConfigDir = importBasePathDir } } - return nil + return profilesBasePathConfigDir, nil } // mergeConfigFromDirectories merges config files from the provided directories. diff --git a/pkg/config/load_error_paths_unix_test.go b/pkg/config/load_error_paths_unix_test.go index 94976efd0f..0e67d6d06c 100644 --- a/pkg/config/load_error_paths_unix_test.go +++ b/pkg/config/load_error_paths_unix_test.go @@ -76,7 +76,7 @@ func TestMergeFiles_ReadFileError(t *testing.T) { v := viper.New() v.SetConfigType("yaml") - err := mergeFiles(v, []string{cfg}) + _, err := mergeFiles(v, []string{cfg}) require.Error(t, err) assert.ErrorIs(t, err, errUtils.ErrReadConfig) } diff --git a/pkg/config/load_profile_test.go b/pkg/config/load_profile_test.go index c288298172..704c4a2889 100644 --- a/pkg/config/load_profile_test.go +++ b/pkg/config/load_profile_test.go @@ -208,7 +208,7 @@ func TestGetProfilesFromFallbacks_EmptyEnvVar(t *testing.T) { "source should be empty for empty ATMOS_PROFILE") } -// TestParseViperProfilesFromEnv_Quirks tests the parseViperProfilesFromEnv function +// TestParseViperProfilesFromEnv_Quirks tests the FixViperEnvStringSliceQuirk function // with various Viper-quirk inputs. func TestParseViperProfilesFromEnv_Quirks(t *testing.T) { tests := []struct { @@ -255,7 +255,7 @@ func TestParseViperProfilesFromEnv_Quirks(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := parseViperProfilesFromEnv(tt.input) + result := FixViperEnvStringSliceQuirk(tt.input) assert.Equal(t, tt.expected, result) }) } diff --git a/pkg/config/load_test.go b/pkg/config/load_test.go index 053aa0596e..f9d0072276 100644 --- a/pkg/config/load_test.go +++ b/pkg/config/load_test.go @@ -1891,7 +1891,7 @@ func TestParseViperProfilesFromEnv(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := parseViperProfilesFromEnv(tt.profiles) + result := FixViperEnvStringSliceQuirk(tt.profiles) assert.Equal(t, tt.expected, result) }) } diff --git a/pkg/config/multifile_array_merge_test.go b/pkg/config/multifile_array_merge_test.go new file mode 100644 index 0000000000..19f22537e4 --- /dev/null +++ b/pkg/config/multifile_array_merge_test.go @@ -0,0 +1,416 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/cloudposse/atmos/pkg/schema" +) + +// TestLoadConfigFromCLIArgs_ArrayFieldMergeAcrossFiles reproduces cloudposse/atmos#2867's +// exact three-scenario matrix: a second --config file setting the SAME, a SUPERSET, or a +// completely DISJOINT value for stacks.included_paths (an array-typed key) relative to the +// first file. Only `commands` gets a manual merge workaround in mergeConfigFile(); every +// other array-typed key -- including stacks.included_paths -- goes through plain +// v.MergeConfig(). This asserts what the FINAL merged atmosConfig.Stacks.IncludedPaths +// actually is for each case, to establish ground truth before designing a fix. +func TestLoadConfigFromCLIArgs_ArrayFieldMergeAcrossFiles(t *testing.T) { + tests := []struct { + name string + fragmentIncluded string + wantIncluded []string + }{ + { + name: "identical value", + fragmentIncluded: `["deploy/**/*"]`, + wantIncluded: []string{"deploy/**/*"}, + }, + { + name: "superset value (still contains original)", + fragmentIncluded: `["deploy/**/*", "other/**/*"]`, + wantIncluded: []string{"deploy/**/*", "other/**/*"}, + }, + { + name: "disjoint value", + fragmentIncluded: `["other/**/*"]`, + wantIncluded: []string{"other/**/*"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + mainFile := filepath.Join(tmpDir, "main.yaml") + fragmentFile := filepath.Join(tmpDir, "fragment.yaml") + + require.NoError(t, os.WriteFile(mainFile, []byte(` +base_path: "." +stacks: + base_path: "stacks" + included_paths: + - "deploy/**/*" +components: + terraform: + base_path: "components/terraform" +`), 0o644)) + + require.NoError(t, os.WriteFile(fragmentFile, []byte(` +stacks: + included_paths: `+tt.fragmentIncluded+` +`), 0o644)) + + v := viper.New() + v.SetConfigType("yaml") + + configAndStacksInfo := &schema.ConfigAndStacksInfo{ + AtmosConfigFilesFromArg: []string{mainFile, fragmentFile}, + } + + var atmosConfig schema.AtmosConfiguration + err := loadConfigFromCLIArgs(v, configAndStacksInfo, &atmosConfig) + require.NoError(t, err) + + assert.Equal(t, tt.wantIncluded, atmosConfig.Stacks.IncludedPaths, + "the second --config file's value for stacks.included_paths must be what atmos "+ + "actually uses -- last file wins, matching the documented/expected 'later file "+ + "overrides earlier' semantics") + }) + } +} + +// TestLoadConfigFromCLIArgs_ArrayFieldMerge_IntermediateStates instruments each stage of +// mergeFiles (the function --config a.yaml,b.yaml actually goes through) to pinpoint exactly +// where a superset stacks.included_paths value from the second file diverges from what ends +// up in the final unmarshaled atmosConfig -- since static tracing of viper's own MergeConfig +// suggests a bare merge call should already fully replace (not revert) the slice. +func TestLoadConfigFromCLIArgs_ArrayFieldMerge_IntermediateStates(t *testing.T) { + tmpDir := t.TempDir() + mainFile := filepath.Join(tmpDir, "main.yaml") + fragmentFile := filepath.Join(tmpDir, "fragment.yaml") + + require.NoError(t, os.WriteFile(mainFile, []byte(` +base_path: "." +stacks: + base_path: "stacks" + included_paths: + - "deploy/**/*" +components: + terraform: + base_path: "components/terraform" +`), 0o644)) + + require.NoError(t, os.WriteFile(fragmentFile, []byte(` +stacks: + included_paths: + - "deploy/**/*" + - "other/**/*" +`), 0o644)) + + v := viper.New() + v.SetConfigType("yaml") + setDefaultConfiguration(v) + require.NoError(t, loadEmbeddedConfig(v)) + + // Stage 1: after merging main.yaml alone. + require.NoError(t, mergeConfigFile(mainFile, v)) + require.Equal(t, []interface{}{"deploy/**/*"}, v.Get("stacks.included_paths"), + "stage 1 (after main.yaml): sanity check on the starting value") + + // Stage 2: after merging fragment.yaml on top (mergeConfigFile only, no mergeImports yet). + require.NoError(t, mergeConfigFile(fragmentFile, v)) + afterMergeConfigFile := v.Get("stacks.included_paths") + t.Logf("stage 2 (after mergeConfigFile(fragment.yaml)): %#v", afterMergeConfigFile) + + // Stage 3: after mergeImports runs (a no-op for files with no `import:` key, per + // processConfigImportsWithFSAndBasePathSource's early return -- confirming that, + // or finding it ISN'T a no-op, is the point of this checkpoint). + _, err := mergeImports(v, tmpDir, "", "") + require.NoError(t, err) + afterMergeImports := v.Get("stacks.included_paths") + t.Logf("stage 3 (after mergeImports): %#v", afterMergeImports) + + // Stage 4: after the final v.Unmarshal into the typed struct (what loadConfigFromCLIArgs + // itself does last). + var atmosConfig schema.AtmosConfiguration + require.NoError(t, v.Unmarshal(&atmosConfig, atmosDecodeHook())) + t.Logf("stage 4 (after v.Unmarshal): %#v", atmosConfig.Stacks.IncludedPaths) + + assert.Equal(t, []string{"deploy/**/*", "other/**/*"}, atmosConfig.Stacks.IncludedPaths, + "final unmarshaled value must match the superset fragment.yaml declared") +} + +// TestInitCliConfig_ConfigMultiFileArraySupersetUsedDuringDiscovery reproduces the full +// end-to-end symptom from cloudposse/atmos#2867: `atmos --config main.yaml,fragment.yaml +// list stacks` reports "No stacks found" when fragment.yaml's stacks.included_paths is a +// superset of main.yaml's, even though the superset still contains the glob that covers the +// only stack manifest present. +func TestInitCliConfig_ConfigMultiFileArraySupersetUsedDuringDiscovery(t *testing.T) { + setupTestAdapters() + + tempDir := t.TempDir() + t.Chdir(tempDir) + t.Setenv("TEST_GIT_ROOT", tempDir) + + viper.Reset() + t.Cleanup(viper.Reset) + + require.NoError(t, os.MkdirAll(filepath.Join(tempDir, "stacks", "deploy"), 0o755)) + + mainFile := filepath.Join(tempDir, "main.yaml") + fragmentFile := filepath.Join(tempDir, "fragment.yaml") + + require.NoError(t, os.WriteFile(mainFile, []byte(` +base_path: '' +components: + terraform: + base_path: components/terraform +stacks: + base_path: stacks + included_paths: + - deploy/**/* + name_pattern: '{stage}' +`), 0o644)) + + require.NoError(t, os.WriteFile(fragmentFile, []byte(` +stacks: + included_paths: + - deploy/**/* + - other/**/* +`), 0o644)) + + require.NoError(t, os.WriteFile(filepath.Join(tempDir, "stacks", "deploy", "dev.yaml"), []byte(` +vars: + stage: dev +components: + terraform: + my-component: + vars: {} +`), 0o644)) + + configAndStacksInfo := schema.ConfigAndStacksInfo{ + AtmosConfigFilesFromArg: []string{mainFile, fragmentFile}, + } + atmosConfig, err := InitCliConfig(configAndStacksInfo, true) + require.NoError(t, err, + "list stacks should discover dev.yaml under the superset included_paths, which still "+ + "contains the original deploy/**/* glob") + assert.NotEmpty(t, atmosConfig.StackConfigFilesAbsolutePaths, + "stack discovery must find dev.yaml -- it's covered by deploy/**/*, present in both files") +} + +// TestInitCliConfig_SingleFileMultipleIncludedPathsOneEmpty reproduces the actual root cause +// behind cloudposse/atmos#2867's "No stacks found" symptom: it is NOT a config-file-merge bug +// (see TestLoadConfigFromCLIArgs_ArrayFieldMergeAcrossFiles above, which proves the merge +// itself is correct) -- it reproduces with a single atmos.yaml and no --config merging at +// all. When stacks.included_paths has multiple glob entries and one of them currently +// matches zero files (e.g. its directory doesn't exist yet), FindAllStackConfigsInPaths +// aborts discovery entirely instead of just skipping that one empty entry, discarding +// matches already found via the other entries. +func TestInitCliConfig_SingleFileMultipleIncludedPathsOneEmpty(t *testing.T) { + setupTestAdapters() + tempDir := t.TempDir() + t.Chdir(tempDir) + t.Setenv("TEST_GIT_ROOT", tempDir) + viper.Reset() + t.Cleanup(viper.Reset) + + require.NoError(t, os.MkdirAll(filepath.Join(tempDir, "stacks", "deploy"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tempDir, "atmos.yaml"), []byte(` +base_path: '' +components: + terraform: + base_path: components/terraform +stacks: + base_path: stacks + included_paths: + - deploy/**/* + - other/**/* + name_pattern: '{stage}' +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(tempDir, "stacks", "deploy", "dev.yaml"), []byte(` +vars: + stage: dev +components: + terraform: + my-component: + vars: {} +`), 0o644)) + + configAndStacksInfo := schema.ConfigAndStacksInfo{} + atmosConfig, err := InitCliConfig(configAndStacksInfo, true) + require.NoError(t, err, + "a single atmos.yaml with one included_paths entry that currently matches nothing "+ + "must not abort discovery of manifests covered by the other entries") + assert.NotEmpty(t, atmosConfig.StackConfigFilesAbsolutePaths) +} + +// TestInitCliConfig_ProfileAppliedOnTopOfConfigFlag reproduces a related bug found +// during the #2867/#2868 audit: loadConfigFromCLIArgs (the --config/--config-path path) +// returns immediately after merging the CLI-selected files, without ever reaching +// LoadConfig's profile-loading block. This means --config and --profile currently cannot be +// combined -- profile settings are silently ignored whenever --config/--config-path is used. +func TestInitCliConfig_ProfileAppliedOnTopOfConfigFlag(t *testing.T) { + setupTestAdapters() + + tempDir := t.TempDir() + t.Chdir(tempDir) + t.Setenv("TEST_GIT_ROOT", tempDir) + if orig, ok := os.LookupEnv("ATMOS_PROFILE"); ok { + require.NoError(t, os.Unsetenv("ATMOS_PROFILE")) + t.Cleanup(func() { require.NoError(t, os.Setenv("ATMOS_PROFILE", orig)) }) + } + + viper.Reset() + t.Cleanup(viper.Reset) + + require.NoError(t, os.MkdirAll(filepath.Join(tempDir, "profiles", "test"), 0o755)) + + mainFile := filepath.Join(tempDir, "main.yaml") + require.NoError(t, os.WriteFile(mainFile, []byte(` +base_path: . +profiles: + base_path: profiles +stacks: + base_path: stacks + included_paths: + - "deploy/**/*" +`), 0o644)) + + // The profile overrides stacks.base_path to a directory the base config never mentions. + require.NoError(t, os.WriteFile(filepath.Join(tempDir, "profiles", "test", "atmos.yaml"), []byte(` +stacks: + base_path: profile-stacks +`), 0o644)) + + configAndStacksInfo := schema.ConfigAndStacksInfo{ + AtmosConfigFilesFromArg: []string{mainFile}, + ProfilesFromArg: []string{"test"}, + } + atmosConfig, err := InitCliConfig(configAndStacksInfo, false) + require.NoError(t, err) + + assert.Equal(t, "profile-stacks", atmosConfig.Stacks.BasePath, + "the profile's stacks.base_path override must be applied on top of the --config-selected "+ + "base, matching the documented precedence in docs/prd/atmos-profiles.md") +} + +// TestInitCliConfig_ProfilesBasePathResolvesAgainstDeclaringFile reproduces a bug found during a +// field-test pass on cloudposse/atmos#2867/#2868: discoverProfileLocations resolved a relative +// `profiles.base_path` against the FIRST --config file's directory, regardless of which file +// actually declared it. Here, only the SECOND --config file (in a different directory) declares +// profiles.base_path, and the profile directory only exists relative to that second file's +// directory -- so the profile must still be found. +func TestInitCliConfig_ProfilesBasePathResolvesAgainstDeclaringFile(t *testing.T) { + setupTestAdapters() + + tempDir := t.TempDir() + t.Chdir(tempDir) + t.Setenv("TEST_GIT_ROOT", tempDir) + if orig, ok := os.LookupEnv("ATMOS_PROFILE"); ok { + require.NoError(t, os.Unsetenv("ATMOS_PROFILE")) + t.Cleanup(func() { require.NoError(t, os.Setenv("ATMOS_PROFILE", orig)) }) + } + + viper.Reset() + t.Cleanup(viper.Reset) + + mainFile := filepath.Join(tempDir, "main.yaml") + require.NoError(t, os.WriteFile(mainFile, []byte(` +base_path: . +stacks: + base_path: stacks + included_paths: + - "deploy/**/*" +`), 0o644)) + + // fragment.yaml lives in a DIFFERENT directory than main.yaml, and is the SECOND --config + // file. Only it declares profiles.base_path, relative to ITS OWN directory. + fragmentDir := filepath.Join(tempDir, "region-overrides") + require.NoError(t, os.MkdirAll(fragmentDir, 0o755)) + fragmentFile := filepath.Join(fragmentDir, "fragment.yaml") + require.NoError(t, os.WriteFile(fragmentFile, []byte(` +profiles: + base_path: ./custom-profiles +`), 0o644)) + + // The profile directory only exists relative to fragmentDir, NOT relative to tempDir (where + // main.yaml, the first --config file, lives) -- proving resolution uses the declaring file's + // directory, not just the first --config file's directory. + profileDir := filepath.Join(fragmentDir, "custom-profiles", "test") + require.NoError(t, os.MkdirAll(profileDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(profileDir, "atmos.yaml"), []byte(` +stacks: + base_path: profile-stacks +`), 0o644)) + + configAndStacksInfo := schema.ConfigAndStacksInfo{ + AtmosConfigFilesFromArg: []string{mainFile, fragmentFile}, + ProfilesFromArg: []string{"test"}, + } + atmosConfig, err := InitCliConfig(configAndStacksInfo, false) + require.NoError(t, err) + + assert.Equal(t, "profile-stacks", atmosConfig.Stacks.BasePath, + "the profile must be found relative to fragment.yaml's directory (where profiles.base_path "+ + "was actually declared), not main.yaml's directory (just the first --config file)") +} + +// TestDeclaresProfilesBasePath covers declaresProfilesBasePath's branches directly: malformed +// YAML, no top-level mapping, no profiles key, profiles present but not itself a mapping, +// profiles a mapping without base_path, and the true-positive case. +func TestDeclaresProfilesBasePath(t *testing.T) { + tests := []struct { + name string + content string + want bool + wantErr bool + }{ + { + name: "malformed YAML returns error", + content: "profiles:\n base_path: [unterminated\n", + wantErr: true, + }, + { + name: "empty content has no top-level mapping", + content: "", + want: false, + }, + { + name: "no profiles key at all", + content: "logs:\n level: Info\n", + want: false, + }, + { + name: "profiles key present but not a mapping", + content: "profiles: not-a-mapping\n", + want: false, + }, + { + name: "profiles is a mapping without base_path", + content: "profiles:\n default: developer\n", + want: false, + }, + { + name: "profiles declares base_path", + content: "profiles:\n base_path: ./custom-profiles\n", + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := declaresProfilesBasePath([]byte(tt.content)) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/config/profiles.go b/pkg/config/profiles.go index 84287d3dd5..e84417c08c 100644 --- a/pkg/config/profiles.go +++ b/pkg/config/profiles.go @@ -24,6 +24,29 @@ type ProfileLocation struct { Precedence int // Lower number = higher precedence. } +// splitCliConfigPath splits atmosConfig.CliConfigPath into its individual contributor +// directories. CliConfigPath is normally a single directory, but connectPaths +// (load_config_args.go) joins multiple --config/--config-path sources into "dirA;dirB;" when +// more than one contributed. An empty CliConfigPath (no config file was used) still yields a +// single "" entry so callers keep resolving relative to the working directory, matching prior +// single-directory behavior. +func splitCliConfigPath(cliConfigPath string) []string { + if cliConfigPath == "" { + return []string{""} + } + parts := strings.Split(cliConfigPath, ";") + dirs := make([]string, 0, len(parts)) + for _, p := range parts { + if p != "" { + dirs = append(dirs, p) + } + } + if len(dirs) == 0 { + return []string{""} + } + return dirs +} + // discoverProfileLocations returns all possible profile locations in precedence order. // Precedence (highest to lowest): // 1. Configurable (profiles.base_path in atmos.yaml). @@ -33,16 +56,33 @@ type ProfileLocation struct { func discoverProfileLocations(atmosConfig *schema.AtmosConfiguration) ([]ProfileLocation, error) { var locations []ProfileLocation - // Use CliConfigPath as base directory (it contains the directory of atmos.yaml). - baseDir := atmosConfig.CliConfigPath - - // 1. Configurable base_path (highest precedence). + // CliConfigPath is usually a single directory, but connectPaths (load_config_args.go) + // joins multiple --config/--config-path contributors into "dirA;dirB;" when more than one + // source was selected. Treating that joined string as a single directory produced a + // nonexistent path like "dirA;dirB;/.atmos/profiles", silently breaking profile discovery + // whenever --config selected more than one file. Search each contributor directory instead. + baseDirs := splitCliConfigPath(atmosConfig.CliConfigPath) + primaryDir := "" + if len(baseDirs) > 0 { + primaryDir = baseDirs[0] + } + + // 1. Configurable base_path (highest precedence), resolved from whichever --config file + // actually declared profiles.base_path (ProfilesBasePathConfigDir), falling back to the + // primary directory for single-file/non-CLI-arg config sources where that isn't tracked. + // Resolving against primaryDir unconditionally previously broke this whenever + // profiles.base_path was declared in a --config file OTHER than the first + // (cloudposse/atmos#2867). if atmosConfig.Profiles.BasePath != "" { basePath := atmosConfig.Profiles.BasePath - // If relative, resolve from atmos.yaml directory. + // If relative, resolve from the declaring file's directory. if !filepath.IsAbs(basePath) { - basePath = filepath.Join(baseDir, basePath) + resolveDir := atmosConfig.ProfilesBasePathConfigDir + if resolveDir == "" { + resolveDir = primaryDir + } + basePath = filepath.Join(resolveDir, basePath) } locations = append(locations, ProfileLocation{ @@ -52,13 +92,14 @@ func discoverProfileLocations(atmosConfig *schema.AtmosConfiguration) ([]Profile }) } - // 2. Project-local hidden profiles. - projectHiddenPath := filepath.Join(baseDir, ".atmos", "profiles") - locations = append(locations, ProfileLocation{ - Path: projectHiddenPath, - Type: "project-hidden", - Precedence: 2, - }) + // 2. Project-local hidden profiles -- one per contributor directory. + for _, baseDir := range baseDirs { + locations = append(locations, ProfileLocation{ + Path: filepath.Join(baseDir, ".atmos", "profiles"), + Type: "project-hidden", + Precedence: 2, + }) + } // 3. XDG user profiles. xdgPath, err := xdg.GetXDGConfigDir("profiles", 0o755) @@ -70,13 +111,14 @@ func discoverProfileLocations(atmosConfig *schema.AtmosConfiguration) ([]Profile }) } - // 4. Project-local non-hidden profiles (lowest precedence). - projectPath := filepath.Join(baseDir, "profiles") - locations = append(locations, ProfileLocation{ - Path: projectPath, - Type: "project", - Precedence: 4, - }) + // 4. Project-local non-hidden profiles (lowest precedence) -- one per contributor directory. + for _, baseDir := range baseDirs { + locations = append(locations, ProfileLocation{ + Path: filepath.Join(baseDir, "profiles"), + Type: "project", + Precedence: 4, + }) + } return locations, nil } diff --git a/pkg/config/profiles_test.go b/pkg/config/profiles_test.go index 3c93485689..58816e842f 100644 --- a/pkg/config/profiles_test.go +++ b/pkg/config/profiles_test.go @@ -74,6 +74,62 @@ func TestDiscoverProfileLocations(t *testing.T) { } } +// TestDiscoverProfileLocations_MultipleConfigDirs guards against a bug found during the +// #2867/#2868 audit: connectPaths (load_config_args.go) joins multiple --config/--config-path +// contributors into "dirA;dirB;", and discoverProfileLocations previously treated that joined +// string as a single directory, producing a nonexistent path like "dirA;dirB;/.atmos/profiles" +// and silently finding no profiles whenever --config selected more than one file. +func TestDiscoverProfileLocations_MultipleConfigDirs(t *testing.T) { + dirA := filepath.Join(string(filepath.Separator), "test", "dirA") + dirB := filepath.Join(string(filepath.Separator), "test", "dirB") + atmosConfig := schema.AtmosConfiguration{ + CliConfigPath: dirA + ";" + dirB + ";", + } + + locations, err := discoverProfileLocations(&atmosConfig) + require.NoError(t, err) + + var projectHidden, project []string + for _, loc := range locations { + switch loc.Type { + case "project-hidden": + projectHidden = append(projectHidden, loc.Path) + case "project": + project = append(project, loc.Path) + } + } + + assert.ElementsMatch(t, []string{ + filepath.Join(dirA, ".atmos", "profiles"), + filepath.Join(dirB, ".atmos", "profiles"), + }, projectHidden) + assert.ElementsMatch(t, []string{ + filepath.Join(dirA, "profiles"), + filepath.Join(dirB, "profiles"), + }, project) +} + +// TestSplitCliConfigPath covers splitCliConfigPath directly, including the empty-input +// fallback that preserves prior single-directory (cwd-relative) behavior. +func TestSplitCliConfigPath(t *testing.T) { + tests := []struct { + name string + input string + expected []string + }{ + {name: "empty", input: "", expected: []string{""}}, + {name: "single", input: "/test/config", expected: []string{"/test/config"}}, + {name: "multiple", input: "/test/dirA;/test/dirB;", expected: []string{"/test/dirA", "/test/dirB"}}, + {name: "separators only, no content", input: ";;;", expected: []string{""}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, splitCliConfigPath(tt.input)) + }) + } +} + // TestFindProfileDirectory tests profile directory lookup across locations. func TestFindProfileDirectory(t *testing.T) { // Create temporary test directories. diff --git a/pkg/config/utils.go b/pkg/config/utils.go index b2144f4ce4..8aef8bc841 100644 --- a/pkg/config/utils.go +++ b/pkg/config/utils.go @@ -56,7 +56,15 @@ func FindAllStackConfigsInPathsForStack( // (like permission issues or invalid path) versus simply no matching files. if len(allMatches) == 0 { _, err := u.GetGlobMatches(patterns[0]) - if err != nil { + // GetGlobMatches itself reports "pattern matched nothing" (e.g. a + // stacks.included_paths entry whose directory doesn't exist yet) by wrapping + // ErrFailedToFindImport -- that's expected and must not abort discovery for the + // *other* entries in includeStackPaths (cloudposse/atmos#2867: a second, + // currently-empty included_paths entry was hard-failing the whole lookup, even + // though an earlier entry already matched real stack manifests). Only a + // different underlying error (invalid pattern syntax, permission denied, etc.) + // is a genuine error worth aborting for. + if err != nil && !errors.Is(err, errUtils.ErrFailedToFindImport) { return nil, nil, false, errUtils.Build(err). WithHintf("Verify `stacks.base_path` in `atmos.yaml` points to the correct directory"). WithHint("Check that the stacks directory exists and contains stack configuration files"). @@ -64,8 +72,8 @@ func FindAllStackConfigsInPathsForStack( WithContext("stacks_base_path", atmosConfig.StacksBaseAbsolutePath). Err() } - // If there's no error but still no matches, we continue to the next path - // This happens when the pattern is valid but no files match it + // Either no error, or the pattern was simply valid-but-empty: continue to the + // next included_paths entry rather than aborting the whole lookup. continue } @@ -154,7 +162,11 @@ func FindAllStackConfigsInPaths( // (like permission issues or invalid path) versus simply no matching files. if len(allMatches) == 0 { _, err := u.GetGlobMatches(patterns[0]) - if err != nil { + // See the identical comment in FindAllStackConfigsInPathsForStack above: + // GetGlobMatches wraps ErrFailedToFindImport for a valid-but-empty pattern, and + // that must not abort discovery for the other includeStackPaths entries + // (cloudposse/atmos#2867). + if err != nil && !errors.Is(err, errUtils.ErrFailedToFindImport) { return nil, nil, errUtils.Build(err). WithHintf("Verify `stacks.base_path` in `atmos.yaml` points to the correct directory"). WithHint("Check that the stacks directory exists and contains stack configuration files"). @@ -162,8 +174,8 @@ func FindAllStackConfigsInPaths( WithContext("stacks_base_path", atmosConfig.StacksBaseAbsolutePath). Err() } - // If there's no error but still no matches, we continue to the next path - // This happens when the pattern is valid but no files match it + // Either no error, or the pattern was simply valid-but-empty: continue to the + // next included_paths entry rather than aborting the whole lookup. continue } @@ -190,6 +202,10 @@ func FindAllStackConfigsInPaths( } } + if len(absolutePaths) == 0 { + return nil, nil, fmt.Errorf("%w in the paths %v", errUtils.ErrNoStackManifestsFound, includeStackPaths) + } + return absolutePaths, relativePaths, nil } diff --git a/pkg/datafetcher/schema/atmos/config/1.0.json b/pkg/datafetcher/schema/atmos/config/1.0.json index d45854e87a..95062d9322 100644 --- a/pkg/datafetcher/schema/atmos/config/1.0.json +++ b/pkg/datafetcher/schema/atmos/config/1.0.json @@ -15904,6 +15904,26 @@ } ] }, + "vendorDirAbsolutePath": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, + "workflowsDirAbsolutePath": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, "version": { "anyOf": [ { diff --git a/pkg/flags/global_registry.go b/pkg/flags/global_registry.go index e1f9f203bd..c70580a50d 100644 --- a/pkg/flags/global_registry.go +++ b/pkg/flags/global_registry.go @@ -1,6 +1,8 @@ package flags import ( + "os" + "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" @@ -39,8 +41,8 @@ func ParseGlobalFlags(cmd *cobra.Command, v *viper.Viper) global.Flags { // Working directory and path configuration. Chdir: v.GetString("chdir"), BasePath: v.GetString("base-path"), - Config: v.GetStringSlice("config"), - ConfigPath: v.GetStringSlice("config-path"), + Config: stringSliceFromViperOrEnv(v, "config", "ATMOS_CONFIG"), + ConfigPath: stringSliceFromViperOrEnv(v, "config-path", "ATMOS_CONFIG_PATH"), // Logging configuration. LogsLevel: v.GetString("logs-level"), @@ -89,6 +91,24 @@ func ParseGlobalFlags(cmd *cobra.Command, v *viper.Viper) global.Flags { } } +// stringSliceFromViperOrEnv reads a StringSlice flag from Viper, correcting for Viper's +// comma-splitting quirk (see cfg.FixViperEnvStringSliceQuirk) when the value came from one of +// the given environment variables rather than the CLI flag itself. CLI-flag-sourced values are +// already parsed correctly by pflag/Cobra and must not be re-split. +// +// This is currently scoped to "config"/"config-path" (cloudposse/atmos#2867/#2868); other +// StringSlice+EnvVar flags (e.g. "skill"/ATMOS_SKILL) share the same latent Viper quirk but are +// deliberately left as a known follow-up rather than fixed here. +func stringSliceFromViperOrEnv(v *viper.Viper, key string, envVars ...string) []string { + values := v.GetStringSlice(key) + for _, envVar := range envVars { + if _, ok := os.LookupEnv(envVar); ok { + return cfg.FixViperEnvStringSliceQuirk(values) + } + } + return values +} + func lookupCommandFlag(cmd *cobra.Command, name string) (*pflag.Flag, bool) { if cmd == nil { return nil, false diff --git a/pkg/flags/global_registry_test.go b/pkg/flags/global_registry_test.go index 3bbaade124..2146712605 100644 --- a/pkg/flags/global_registry_test.go +++ b/pkg/flags/global_registry_test.go @@ -604,6 +604,72 @@ func TestParseGlobalFlags_SkillFlag(t *testing.T) { }) } +// TestParseGlobalFlags_ConfigEnvVarCommaSplit guards against a bug found during a field-test +// pass on cloudposse/atmos#2867/#2868: ATMOS_CONFIG/ATMOS_CONFIG_PATH with multiple +// comma-separated files worked for commands hitting pkg/config's own os.Args/env fallback (e.g. +// `atmos config get`), but broke every command reading these flags through this canonical +// ParseGlobalFlags path (`atmos list stacks`, `atmos auth list`, etc.) with a "file not found: +// 'a.yaml,b.yaml'" error -- because Viper's env-sourced GetStringSlice splits on whitespace, not +// commas (the same quirk already fixed for --profile/ATMOS_PROFILE, just not applied here). +func TestParseGlobalFlags_ConfigEnvVarCommaSplit(t *testing.T) { + t.Run("CLI flag with multiple files is unaffected", func(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + v := viper.New() + parser := NewGlobalOptionsBuilder().Build() + parser.RegisterFlags(cmd) + _ = parser.BindToViper(v) + + v.Set("config", []string{"a.yaml", "b.yaml"}) + + flags := ParseGlobalFlags(cmd, v) + assert.Equal(t, []string{"a.yaml", "b.yaml"}, flags.Config) + }) + + t.Run("ATMOS_CONFIG with multiple comma-separated files splits correctly", func(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + v := viper.New() + parser := NewGlobalOptionsBuilder().Build() + parser.RegisterFlags(cmd) + _ = parser.BindToViper(v) + + t.Setenv("ATMOS_CONFIG", "a.yaml,b.yaml") + _ = v.BindEnv("config", "ATMOS_CONFIG") + + flags := ParseGlobalFlags(cmd, v) + assert.Equal(t, []string{"a.yaml", "b.yaml"}, flags.Config, + "ATMOS_CONFIG should split on commas like the --config CLI flag does") + }) + + t.Run("ATMOS_CONFIG_PATH with multiple comma-separated dirs splits correctly", func(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + v := viper.New() + parser := NewGlobalOptionsBuilder().Build() + parser.RegisterFlags(cmd) + _ = parser.BindToViper(v) + + t.Setenv("ATMOS_CONFIG_PATH", "dirA,dirB") + _ = v.BindEnv("config-path", "ATMOS_CONFIG_PATH") + + flags := ParseGlobalFlags(cmd, v) + assert.Equal(t, []string{"dirA", "dirB"}, flags.ConfigPath, + "ATMOS_CONFIG_PATH should split on commas like the --config-path CLI flag does") + }) + + t.Run("ATMOS_CONFIG with a single file is unaffected", func(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + v := viper.New() + parser := NewGlobalOptionsBuilder().Build() + parser.RegisterFlags(cmd) + _ = parser.BindToViper(v) + + t.Setenv("ATMOS_CONFIG", "a.yaml") + _ = v.BindEnv("config", "ATMOS_CONFIG") + + flags := ParseGlobalFlags(cmd, v) + assert.Equal(t, []string{"a.yaml"}, flags.Config) + }) +} + // TestParseGlobalFlags_SettingsListMergeStrategyFlag verifies that the // --settings-list-merge-strategy global flag is registered (so Cobra accepts it) // and that its value flows through Viper, env var, and the default empty state. diff --git a/pkg/mcp/config/config.go b/pkg/mcp/config/config.go index 8001b54866..76c73ff7b5 100644 --- a/pkg/mcp/config/config.go +++ b/pkg/mcp/config/config.go @@ -242,10 +242,15 @@ func ParseHeaderPairs(pairs []string) (map[string]string, error) { func ResolveFile(cmd *cobra.Command, atmosConfig *schema.AtmosConfiguration) (string, error) { defer perf.Track(atmosConfig, "mcpconfig.ResolveFile")() - override := "" - if cfgFiles, _ := cmd.Flags().GetStringSlice("config"); len(cfgFiles) > 0 { - override = cfgFiles[0] + cfgFiles, _ := cmd.Flags().GetStringSlice("config") + override, err := pkgconfig.ResolveConfigOverride(cfgFiles) + if err != nil { + return "", errUtils.Build(errUtils.ErrInvalidArgumentError). + WithExplanation(err.Error()). + WithHint("Pass a single --config file, or edit the target file directly."). + Err() } + file, err := pkgconfig.ResolveEditableConfigFile(atmosConfig, override) if err != nil { return "", errUtils.Build(errUtils.ErrInvalidArgumentError). diff --git a/pkg/mcp/config/config_test.go b/pkg/mcp/config/config_test.go index 0d028173be..183f0e2954 100644 --- a/pkg/mcp/config/config_test.go +++ b/pkg/mcp/config/config_test.go @@ -3,12 +3,15 @@ package config import ( "os" "path/filepath" + "strings" "testing" + ckerrors "github.com/cockroachdb/errors" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + errUtils "github.com/cloudposse/atmos/errors" "github.com/cloudposse/atmos/pkg/schema" ) @@ -273,3 +276,24 @@ func TestResolveFile(t *testing.T) { require.NoError(t, err) assert.Equal(t, file, resolved) } + +// TestResolveFile_MultipleConfigFilesAmbiguous guards against the same bug fixed in +// cmd/config's resolveConfigFile (cloudposse/atmos#2867/#2868): ResolveFile silently used only +// the FIRST --config file when multiple were given, so `mcp client add --config a,b` could +// silently edit the wrong file. +func TestResolveFile_MultipleConfigFilesAmbiguous(t *testing.T) { + dir := t.TempDir() + fileA := filepath.Join(dir, "a.yaml") + fileB := filepath.Join(dir, "b.yaml") + require.NoError(t, os.WriteFile(fileA, []byte("base_path: \"./\"\n"), 0o600)) + require.NoError(t, os.WriteFile(fileB, []byte("base_path: \"./\"\n"), 0o600)) + + cmd := &cobra.Command{} + cmd.Flags().StringSlice("config", []string{fileA, fileB}, "") + + _, err := ResolveFile(cmd, &schema.AtmosConfiguration{}) + require.ErrorIs(t, err, errUtils.ErrInvalidArgumentError) + details := strings.Join(ckerrors.GetAllDetails(err), "\n") + assert.Contains(t, details, "a.yaml") + assert.Contains(t, details, "b.yaml") +} diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 2076fc85ca..edd350ad92 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -132,6 +132,8 @@ type AtmosConfiguration struct { AnsibleDirAbsolutePath string `yaml:"ansibleDirAbsolutePath,omitempty" json:"ansibleDirAbsolutePath,omitempty" mapstructure:"ansibleDirAbsolutePath"` KubernetesDirAbsolutePath string `yaml:"kubernetesDirAbsolutePath,omitempty" json:"kubernetesDirAbsolutePath,omitempty" mapstructure:"kubernetesDirAbsolutePath"` HelmDirAbsolutePath string `yaml:"helmDirAbsolutePath,omitempty" json:"helmDirAbsolutePath,omitempty" mapstructure:"helmDirAbsolutePath"` + VendorDirAbsolutePath string `yaml:"vendorDirAbsolutePath,omitempty" json:"vendorDirAbsolutePath,omitempty" mapstructure:"vendorDirAbsolutePath"` + WorkflowsDirAbsolutePath string `yaml:"workflowsDirAbsolutePath,omitempty" json:"workflowsDirAbsolutePath,omitempty" mapstructure:"workflowsDirAbsolutePath"` StackConfigFilesRelativePaths []string `yaml:"stackConfigFilesRelativePaths,omitempty" json:"stackConfigFilesRelativePaths,omitempty" mapstructure:"stackConfigFilesRelativePaths"` StackConfigFilesAbsolutePaths []string `yaml:"stackConfigFilesAbsolutePaths,omitempty" json:"stackConfigFilesAbsolutePaths,omitempty" mapstructure:"stackConfigFilesAbsolutePaths"` StackType string `yaml:"stackType,omitempty" json:"StackType,omitempty" mapstructure:"stackType"` @@ -145,22 +147,28 @@ type AtmosConfiguration struct { // SecretsAuth carries the auth-context resolver and effective default identity for cloud-KMS // SOPS providers (sops/aws-kms, sops/gcp-kms, sops/azure-kv). It is transient (never serialized) // and populated alongside the store auth resolver in the `atmos secret` and terraform code paths. - SecretsAuth *store.SecretsAuthContext `yaml:"-" json:"-" mapstructure:"-"` - CliConfigPath string `yaml:"cli_config_path" json:"cli_config_path,omitempty" mapstructure:"cli_config_path"` - Import []string `yaml:"import" json:"import" mapstructure:"import"` - Docs Docs `yaml:"docs,omitempty" json:"docs,omitempty" mapstructure:"docs"` - Auth AuthConfig `yaml:"auth,omitempty" json:"auth,omitempty" mapstructure:"auth"` - Container ContainerConfig `yaml:"container,omitempty" json:"container,omitempty" mapstructure:"container"` - Compositions map[string]Composition `yaml:"compositions,omitempty" json:"compositions,omitempty" mapstructure:"compositions"` - Env map[string]string `yaml:"env,omitempty" json:"env,omitempty" mapstructure:"-"` // mapstructure:"-" avoids collision with Command.Env []CommandEnv. - CaseMaps *casemap.CaseMaps `yaml:"-" json:"-" mapstructure:"-"` // Stores original case for YAML map keys (Viper lowercases them). - Profiler profiler.Config `yaml:"profiler,omitempty" json:"profiler,omitempty" mapstructure:"profiler"` - TrackProvenance bool `yaml:"track_provenance,omitempty" json:"track_provenance,omitempty" mapstructure:"track_provenance"` - Toolchain Toolchain `yaml:"toolchain,omitempty" json:"toolchain,omitempty" mapstructure:"toolchain"` - Git GitConfig `yaml:"git,omitempty" json:"git,omitempty" mapstructure:"git"` - Devcontainer map[string]any `yaml:"devcontainer,omitempty" json:"devcontainer,omitempty" mapstructure:"devcontainer"` - Profiles ProfilesConfig `yaml:"profiles,omitempty" json:"profiles,omitempty" mapstructure:"profiles"` - Metadata ConfigMetadata `yaml:"metadata,omitempty" json:"metadata,omitempty" mapstructure:"metadata"` + SecretsAuth *store.SecretsAuthContext `yaml:"-" json:"-" mapstructure:"-"` + CliConfigPath string `yaml:"cli_config_path" json:"cli_config_path,omitempty" mapstructure:"cli_config_path"` + // ProfilesBasePathConfigDir is the directory of the --config file that declared + // profiles.base_path, used to resolve a relative profiles.base_path correctly when multiple + // --config files are given (cloudposse/atmos#2867: previously always resolved against the + // FIRST --config file's directory regardless of which file actually declared it). Transient, + // populated during LoadConfig, never serialized. + ProfilesBasePathConfigDir string `yaml:"-" json:"-" mapstructure:"-"` + Import []string `yaml:"import" json:"import" mapstructure:"import"` + Docs Docs `yaml:"docs,omitempty" json:"docs,omitempty" mapstructure:"docs"` + Auth AuthConfig `yaml:"auth,omitempty" json:"auth,omitempty" mapstructure:"auth"` + Container ContainerConfig `yaml:"container,omitempty" json:"container,omitempty" mapstructure:"container"` + Compositions map[string]Composition `yaml:"compositions,omitempty" json:"compositions,omitempty" mapstructure:"compositions"` + Env map[string]string `yaml:"env,omitempty" json:"env,omitempty" mapstructure:"-"` // mapstructure:"-" avoids collision with Command.Env []CommandEnv. + CaseMaps *casemap.CaseMaps `yaml:"-" json:"-" mapstructure:"-"` // Stores original case for YAML map keys (Viper lowercases them). + Profiler profiler.Config `yaml:"profiler,omitempty" json:"profiler,omitempty" mapstructure:"profiler"` + TrackProvenance bool `yaml:"track_provenance,omitempty" json:"track_provenance,omitempty" mapstructure:"track_provenance"` + Toolchain Toolchain `yaml:"toolchain,omitempty" json:"toolchain,omitempty" mapstructure:"toolchain"` + Git GitConfig `yaml:"git,omitempty" json:"git,omitempty" mapstructure:"git"` + Devcontainer map[string]any `yaml:"devcontainer,omitempty" json:"devcontainer,omitempty" mapstructure:"devcontainer"` + Profiles ProfilesConfig `yaml:"profiles,omitempty" json:"profiles,omitempty" mapstructure:"profiles"` + Metadata ConfigMetadata `yaml:"metadata,omitempty" json:"metadata,omitempty" mapstructure:"metadata"` // List holds command-specific list configurations (list.components, list.instances, list.stacks). List TopLevelListConfig `yaml:"list,omitempty" json:"list,omitempty" mapstructure:"list"` CI CIConfig `yaml:"ci,omitempty" json:"ci,omitempty" mapstructure:"ci"` diff --git a/pkg/vendoring/resolve.go b/pkg/vendoring/resolve.go index ab3d237288..2cb611ab7e 100644 --- a/pkg/vendoring/resolve.go +++ b/pkg/vendoring/resolve.go @@ -160,9 +160,12 @@ func VendorFilePresent(override string) (string, bool) { } if atmosConfig.Vendor.BasePath != "" { + // Use the precomputed absolute path (AtmosConfigAbsolutePaths) rather than re-joining + // the raw, possibly still-relative atmosConfig.BasePath here -- same bug shape + // cloudposse/atmos#2864 fixed for the top-level base_path itself. vendorPath := atmosConfig.Vendor.BasePath if !filepath.IsAbs(vendorPath) { - vendorPath = filepath.Join(atmosConfig.BasePath, vendorPath) + vendorPath = atmosConfig.VendorDirAbsolutePath } if _, statErr := os.Stat(vendorPath); statErr == nil { return vendorPath, true @@ -173,7 +176,7 @@ func VendorFilePresent(override string) (string, bool) { if found, ok := u.SearchConfigFile(DefaultVendorFile); ok { return found, true } - if found, ok := u.SearchConfigFile(filepath.Join(atmosConfig.BasePath, DefaultVendorFile)); ok { + if found, ok := u.SearchConfigFile(filepath.Join(atmosConfig.BasePathAbsolute, DefaultVendorFile)); ok { return found, true } return "", false diff --git a/tests/snapshots/TestCLICommands_atmos_--chdir_config_isolation.stdout.golden b/tests/snapshots/TestCLICommands_atmos_--chdir_config_isolation.stdout.golden index 4f16bc101f..2d931155cd 100644 --- a/tests/snapshots/TestCLICommands_atmos_--chdir_config_isolation.stdout.golden +++ b/tests/snapshots/TestCLICommands_atmos_--chdir_config_isolation.stdout.golden @@ -109,6 +109,8 @@ packerDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/chdir-iso ansibleDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/chdir-isolation kubernetesDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/chdir-isolation helmDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/chdir-isolation +vendorDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/chdir-isolation/chdir-isolation-vendor +workflowsDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/chdir-isolation default: false cli_config_path: /absolute/path/to/repo/tests/fixtures/scenarios/chdir-isolation import: [] diff --git a/tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden b/tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden index 6b838b8326..9d2d2499ad 100644 --- a/tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden +++ b/tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden @@ -258,6 +258,8 @@ "ansibleDirAbsolutePath": "/absolute/path/to/repo/examples/demo-stacks", "kubernetesDirAbsolutePath": "/absolute/path/to/repo/examples/demo-stacks", "helmDirAbsolutePath": "/absolute/path/to/repo/examples/demo-stacks", + "vendorDirAbsolutePath": "/absolute/path/to/repo/examples/demo-stacks", + "workflowsDirAbsolutePath": "/absolute/path/to/repo/examples/demo-stacks", "default": false, "version": { "check": {}, diff --git a/tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden b/tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden index 139211f970..4c9a1c6ef8 100644 --- a/tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden +++ b/tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden @@ -110,6 +110,8 @@ packerDirAbsolutePath: /absolute/path/to/repo/examples/demo-stacks ansibleDirAbsolutePath: /absolute/path/to/repo/examples/demo-stacks kubernetesDirAbsolutePath: /absolute/path/to/repo/examples/demo-stacks helmDirAbsolutePath: /absolute/path/to/repo/examples/demo-stacks +vendorDirAbsolutePath: /absolute/path/to/repo/examples/demo-stacks +workflowsDirAbsolutePath: /absolute/path/to/repo/examples/demo-stacks default: false cli_config_path: /absolute/path/to/repo/examples/demo-stacks import: [] diff --git a/tests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden b/tests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden index 30ef837dcd..5639be0d75 100644 --- a/tests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden +++ b/tests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden @@ -126,6 +126,8 @@ packerDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-cli ansibleDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-cli-imports kubernetesDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-cli-imports helmDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-cli-imports +vendorDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-cli-imports/vendor.yaml +workflowsDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-cli-imports default: false cli_config_path: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-cli-imports import: diff --git a/tests/snapshots/TestCLICommands_atmos_describe_configuration.stdout.golden b/tests/snapshots/TestCLICommands_atmos_describe_configuration.stdout.golden index 103f6cb879..82390abdb6 100644 --- a/tests/snapshots/TestCLICommands_atmos_describe_configuration.stdout.golden +++ b/tests/snapshots/TestCLICommands_atmos_describe_configuration.stdout.golden @@ -124,6 +124,8 @@ packerDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-con ansibleDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-configuration kubernetesDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-configuration helmDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-configuration +vendorDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-configuration +workflowsDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-configuration default: false cli_config_path: /absolute/path/to/repo/tests/fixtures/scenarios/atmos-configuration import: [] diff --git a/tests/snapshots/TestCLICommands_indentation.stdout.golden b/tests/snapshots/TestCLICommands_indentation.stdout.golden index 2105d0f1bd..b48127a11a 100644 --- a/tests/snapshots/TestCLICommands_indentation.stdout.golden +++ b/tests/snapshots/TestCLICommands_indentation.stdout.golden @@ -108,6 +108,8 @@ packerDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/indentati ansibleDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/indentation kubernetesDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/indentation helmDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/indentation +vendorDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/indentation +workflowsDirAbsolutePath: /absolute/path/to/repo/tests/fixtures/scenarios/indentation default: false cli_config_path: /absolute/path/to/repo/tests/fixtures/scenarios/indentation import: [] diff --git a/tests/snapshots/TestCLICommands_secrets-masking_describe_config.stdout.golden b/tests/snapshots/TestCLICommands_secrets-masking_describe_config.stdout.golden index 4ba5cc1f48..135f50104d 100644 --- a/tests/snapshots/TestCLICommands_secrets-masking_describe_config.stdout.golden +++ b/tests/snapshots/TestCLICommands_secrets-masking_describe_config.stdout.golden @@ -267,6 +267,8 @@ "ansibleDirAbsolutePath": "/absolute/path/to/repo/examples/secrets-masking", "kubernetesDirAbsolutePath": "/absolute/path/to/repo/examples/secrets-masking", "helmDirAbsolutePath": "/absolute/path/to/repo/examples/secrets-masking", + "vendorDirAbsolutePath": "/absolute/path/to/repo/examples/secrets-masking", + "workflowsDirAbsolutePath": "/absolute/path/to/repo/examples/secrets-masking", "default": false, "version": { "check": {}, diff --git a/website/docs/cli/commands/config/config-delete.mdx b/website/docs/cli/commands/config/config-delete.mdx index b98964b531..c6d0e2b356 100644 --- a/website/docs/cli/commands/config/config-delete.mdx +++ b/website/docs/cli/commands/config/config-delete.mdx @@ -40,6 +40,6 @@ atmos config del logs.file ## Flags
-
`--config` (string, inherited)
-
Target a specific `atmos.yaml` file instead of the one discovered in the current directory or git root.
+
`--config` (string slice, inherited)
+
Target a specific `atmos.yaml` file instead of the one discovered in the current directory or git root. Must name exactly one file: `config delete` edits a single concrete file on disk, so passing more than one `--config` value is rejected as ambiguous.
diff --git a/website/docs/cli/commands/config/config-format.mdx b/website/docs/cli/commands/config/config-format.mdx index 41dee68f2b..1991be99c9 100644 --- a/website/docs/cli/commands/config/config-format.mdx +++ b/website/docs/cli/commands/config/config-format.mdx @@ -33,6 +33,6 @@ atmos --config ./config/atmos.yaml config format ## Flags
-
`--config` (string, inherited)
-
Target a specific `atmos.yaml` file instead of the one discovered in the current directory or git root.
+
`--config` (string slice, inherited)
+
Target a specific `atmos.yaml` file instead of the one discovered in the current directory or git root. Must name exactly one file: `config format` edits a single concrete file on disk, so passing more than one `--config` value is rejected as ambiguous.
diff --git a/website/docs/cli/commands/config/config-get.mdx b/website/docs/cli/commands/config/config-get.mdx index 8cdc3eda05..95d72edc86 100644 --- a/website/docs/cli/commands/config/config-get.mdx +++ b/website/docs/cli/commands/config/config-get.mdx @@ -3,17 +3,23 @@ title: atmos config get sidebar_label: get sidebar_class_name: command id: config-get -description: Read a value from atmos.yaml by dot-notation path. +description: Read a value from the effective Atmos configuration by dot-notation path. --- import Intro from '@site/src/components/Intro' import CastPlayer from '@site/src/components/CastPlayer' -Read a value from your `atmos.yaml` configuration using a dot-notation path. +Read a value from the effective, fully-merged Atmos configuration using a dot-notation path. +`atmos config get` reports the same configuration Atmos actually uses for this invocation — +every `--config` file, `--config-path` directory, and profile merged together in precedence +order — not just what a single physical `atmos.yaml` file declares on its own. If you need to +inspect or edit one file's own declared value directly, use `atmos config format` or open the +file. + ## Usage ```shell @@ -26,6 +32,7 @@ atmos config get atmos config get logs.level atmos config get components.terraform.base_path atmos config get logs.level --config ./atmos.yaml +atmos config get stacks.included_paths --config ./main.yaml,./overrides.yaml ``` ## Arguments @@ -38,6 +45,6 @@ atmos config get logs.level --config ./atmos.yaml ## Flags
-
`--config` (string, inherited)
-
Target a specific `atmos.yaml` file instead of the one discovered in the current directory or git root.
+
`--config` (string slice, inherited)
+
Select one or more `atmos.yaml` files instead of the location discovered in the current directory or git root. When multiple files are given, later files override earlier ones and the merged result is what `get` reports.
diff --git a/website/docs/cli/commands/config/config-set.mdx b/website/docs/cli/commands/config/config-set.mdx index b3daf62725..a8f7bc2d30 100644 --- a/website/docs/cli/commands/config/config-set.mdx +++ b/website/docs/cli/commands/config/config-set.mdx @@ -62,8 +62,8 @@ atmos config set logs.level Trace --config ./atmos.yaml
`--type` (string, default: inferred from the schema, falling back to `string`)
How to interpret ``: `string`, `int`, `bool`, `float`, `null`, or `yaml` (a raw YAML/yq literal inserted verbatim). Overrides the type Atmos would otherwise infer from ``.
-
`--config` (string, inherited)
-
Target a specific `atmos.yaml` file instead of the one discovered in the current directory or git root.
+
`--config` (string slice, inherited)
+
Target a specific `atmos.yaml` file instead of the one discovered in the current directory or git root. Must name exactly one file: `config set` edits a single concrete file on disk, so passing more than one `--config` value is rejected as ambiguous.
:::note diff --git a/website/docs/cli/configuration/configuration.mdx b/website/docs/cli/configuration/configuration.mdx index e490eb3f39..3ea20dc483 100644 --- a/website/docs/cli/configuration/configuration.mdx +++ b/website/docs/cli/configuration/configuration.mdx @@ -29,14 +29,16 @@ Think of this file as where you bootstrap the settings of your project. If you'l Atmos discovers configuration from multiple sources in the following precedence order (highest to lowest priority): -1. **Command-line flags** (`--config`, `--config-path`) -2. **Environment variable** (`ATMOS_CLI_CONFIG_PATH`) -3. **Profiles** (`--profile` or `ATMOS_PROFILE`) — Named configuration overrides applied on top of base config -4. **Current directory** (`./atmos.yaml`) - CWD only, no parent search -5. **Git repository root** (`repo-root/atmos.yaml`) - if in a git repository -6. **Parent directory search** - walks up from CWD looking for `atmos.yaml` -7. **Home directory** (`~/.atmos/atmos.yaml`) -8. **System directory** (`/usr/local/etc/atmos/atmos.yaml` on Linux, `%LOCALAPPDATA%/atmos/atmos.yaml` on Windows) +1. **Command-line flags** (`--config`, `--config-path`) — if a flag is set, it always wins over + its environment variable equivalent below, even when both are present +2. **Environment variable equivalents of the flags above** (`ATMOS_CONFIG`, `ATMOS_CONFIG_PATH`) +3. **Environment variable** (`ATMOS_CLI_CONFIG_PATH`) +4. **Profiles** (`--profile` or `ATMOS_PROFILE`) — Named configuration overrides applied on top of base config +5. **Current directory** (`./atmos.yaml`) - CWD only, no parent search +6. **Git repository root** (`repo-root/atmos.yaml`) - if in a git repository +7. **Parent directory search** - walks up from CWD looking for `atmos.yaml` +8. **Home directory** (`~/.atmos/atmos.yaml`) +9. **System directory** (`/usr/local/etc/atmos/atmos.yaml` on Linux, `%LOCALAPPDATA%/atmos/atmos.yaml` on Windows) Each configuration file discovered is deep-merged with the preceding configurations. Separately, Atmos may also auto-import configuration fragments from `.atmos.d/`. The git repository root is special: even when `atmos.yaml` is found elsewhere, Atmos always checks for `.atmos.d/` at the repo root to load shared fragments like custom commands. @@ -72,7 +74,14 @@ atmos --config /path/to/config1.yaml --config /path/to/config2.yaml \ --config-path /path/first/config/ --config-path /path/second/config/ ... ``` -Configurations are deep-merged in the order provided, with later configurations overriding earlier ones. +Configurations are deep-merged in the order provided, with later configurations overriding earlier ones. `--config-path` directories are always merged after — and therefore override — `--config` files, regardless of the order the flags appear on the command line. + +Both flags also accept comma-separated values via environment variables — `ATMOS_CONFIG` for `--config`, `ATMOS_CONFIG_PATH` for `--config-path`: + +```bash +export ATMOS_CONFIG=/path/to/config1.yaml,/path/to/config2.yaml +atmos describe config +``` ### Profiles diff --git a/website/docs/cli/configuration/profiles.mdx b/website/docs/cli/configuration/profiles.mdx index a56aef8a63..4f7df7b8fd 100644 --- a/website/docs/cli/configuration/profiles.mdx +++ b/website/docs/cli/configuration/profiles.mdx @@ -58,6 +58,11 @@ Atmos resolves the active profile using this precedence (highest first): 3. `profiles.default` in the base `atmos.yaml` 4. No profile +Profiles apply on top of whatever base configuration was selected — including a base loaded +via [`--config` or `--config-path`](/cli/configuration) — so +`atmos --config ./main.yaml --profile ci ...` merges the `ci` profile over `main.yaml`, the +same way it would over an auto-discovered `atmos.yaml`. + ### Pinning a Profile Per Project You can pin a profile for a whole project by setting `ATMOS_PROFILE` in a committed dotenv file and including it in the base `atmos.yaml` [`env`](/cli/configuration/env) section: