Skip to content

Commit 34ef473

Browse files
ostermanclaudeautofix-ci[bot]aknysh
authored
fix: Config isolation for --chdir flag (#1941)
* fix: Implement proper config isolation for --chdir flag When using --chdir to change to a directory with its own atmos.yaml, config loading now correctly uses ONLY that local config. Parent directory and git root searches are now properly treated as fallbacks and are skipped when local config exists. - Add hasLocalAtmosConfig() checks to readParentDirConfig() and readGitRootConfig() - Update PRD to clarify parent/git-root searches are fallbacks when local config exists - Add unit tests for config isolation behavior - Add CLI test verifying --chdir doesn't merge parent/repo-root configs - Update test framework to not override config when --chdir is used This fixes the issue where running `atmos --chdir path/to/project describe config` would unexpectedly include configuration from parent directories and the git root. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <[email protected]> * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) * docs: Add guidance on restoring previous config merge behavior Document that explicit imports can be used to restore the previous behavior where parent/repo-root configurations were merged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <[email protected]> * fix: Address CodeRabbit review feedback - Fix CLI test to use !not tag syntax instead of unsupported stdout_not field - Improve git-root skip test to actually exercise hasLocalAtmosConfig() gating by using TEST_GIT_ROOT with a config that would be loaded if not skipped - Add test case verifying git root IS loaded when CWD has no local config 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <[email protected]> * test: Enable snapshots for chdir config isolation test 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Haiku 4.5 <[email protected]> * test: Ignore github_token in chdir config isolation snapshot Add diff pattern to ignore the github_token line in snapshot comparison. This field varies by environment (present when GITHUB_TOKEN is set in the runner's environment, absent otherwise). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * update snapshots * update snapshots --------- Co-authored-by: Claude Haiku 4.5 <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: aknysh <[email protected]> Co-authored-by: Andriy Knysh <[email protected]>
1 parent 7c5190f commit 34ef473

File tree

9 files changed

+470
-9
lines changed

9 files changed

+470
-9
lines changed

‎docs/prd/git-root-discovery-default-behavior.md‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,12 +178,28 @@ YAML Function Processing:
178178
179179
### Config Search Order (Current)
180180
181+
Config is loaded from multiple sources. Sources are loaded in order, with later sources
182+
overriding earlier ones via Viper's merge behavior.
183+
184+
**IMPORTANT:** Parent directory and git root searches are **fallbacks** - they only run
185+
when the current working directory does NOT have any Atmos configuration indicator.
186+
181187
1. **Embedded config** - `pkg/config/atmos.yaml` (no YAML functions)
182188
2. **System directory** - `/usr/local/etc/atmos/atmos.yaml`
183189
3. **Home directory** - `~/.atmos/atmos.yaml`
184-
4. **Current working directory** - `./atmos.yaml`
185-
5. **Environment variable** - `ATMOS_CLI_CONFIG_PATH`
186-
6. **CLI argument** - `--config-dir` flag
190+
4. **Parent directory search** - Walk up from CWD (**fallback only - skipped if CWD has config**)
191+
5. **Git repository root** - `!repo-root` location (**fallback only - skipped if CWD has config**)
192+
6. **Current working directory** - `./atmos.yaml`
193+
7. **Environment variable** - `ATMOS_CLI_CONFIG_PATH`
194+
8. **CLI argument** - `--config-path` flag
195+
196+
**Atmos configuration indicators** (any means "local config exists"):
197+
- `atmos.yaml` or `.atmos.yaml`
198+
- `.atmos/` directory
199+
- `.atmos.d/` or `atmos.d/` directory
200+
201+
This ensures that `--chdir` to a directory with its own `atmos.yaml` uses ONLY that config,
202+
without merging parent/repo-root configs.
187203
188204
### BasePath Usage
189205

‎pkg/config/load.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ const (
4141

4242
var defaultHomeDirProvider = filesystem.NewOSHomeDirProvider()
4343

44+
// osGetwd wraps os.Getwd, allowing tests to simulate CWD errors.
45+
var osGetwd = os.Getwd
46+
4447
// mergedConfigFiles tracks all config files merged during a LoadConfig call.
4548
// This is used to extract case-sensitive map keys from all sources, not just the main config.
4649
// The slice is reset at the start of each LoadConfig call.
@@ -67,6 +70,8 @@ const (
6770
profileDelimiter = ","
6871
// AtmosCliConfigPathEnvVar is the environment variable name for CLI config path.
6972
AtmosCliConfigPathEnvVar = "ATMOS_CLI_CONFIG_PATH"
73+
// CwdKey is the log key for current working directory.
74+
cwdKey = "cwd"
7075
)
7176

7277
// parseProfilesFromOsArgs parses --profile flags from os.Args using pflag.
@@ -561,6 +566,7 @@ func readWorkDirConfigOnly(v *viper.Viper) error {
561566
}
562567

563568
// readGitRootConfig tries to load atmos.yaml from the git repository root.
569+
// This is a fallback search - it only runs when CWD does NOT have local Atmos config.
564570
func readGitRootConfig(v *viper.Viper) error {
565571
// Check if git root discovery is disabled.
566572
//nolint:forbidigo // ATMOS_GIT_ROOT_BASEPATH is bootstrap config, not application configuration.
@@ -575,6 +581,20 @@ func readGitRootConfig(v *viper.Viper) error {
575581
return nil
576582
}
577583

584+
// Skip git root search if CWD has local Atmos config.
585+
// This ensures --chdir to a directory with its own config uses only that config.
586+
cwd, err := osGetwd()
587+
if err != nil {
588+
log.Debug("Failed to get CWD for local config check", "error", err)
589+
} else {
590+
log.Debug("readGitRootConfig checking CWD for local config", cwdKey, cwd)
591+
if hasLocalAtmosConfig(cwd) {
592+
log.Debug("Skipping git root search (local config exists)", "path", cwd)
593+
return nil
594+
}
595+
log.Debug("No local config found, will search git root", cwdKey, cwd)
596+
}
597+
578598
gitRoot, err := u.ProcessTagGitRoot("!repo-root")
579599
if err != nil {
580600
log.Trace("Git root detection failed", "error", err)
@@ -608,6 +628,7 @@ func readGitRootConfig(v *viper.Viper) error {
608628
}
609629

610630
// readParentDirConfig searches parent directories for atmos.yaml (fallback).
631+
// This is a fallback search - it only runs when CWD does NOT have local Atmos config.
611632
func readParentDirConfig(v *viper.Viper) error {
612633
// If ATMOS_CLI_CONFIG_PATH is set, don't search parent directories.
613634
// This allows tests and users to explicitly control config discovery.
@@ -621,6 +642,16 @@ func readParentDirConfig(v *viper.Viper) error {
621642
return err
622643
}
623644

645+
log.Debug("readParentDirConfig checking CWD for local config", cwdKey, wd)
646+
647+
// Skip parent search if CWD has local Atmos config.
648+
// This ensures --chdir to a directory with its own config uses only that config.
649+
if hasLocalAtmosConfig(wd) {
650+
log.Debug("Skipping parent directory search (local config exists)", "path", wd)
651+
return nil
652+
}
653+
log.Debug("No local config found, will search parent directories", cwdKey, wd)
654+
624655
// Search parent directories for atmos.yaml.
625656
configDir := findAtmosConfigInParentDirs(wd)
626657
if configDir == "" {

‎pkg/config/load_test.go‎

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package config
22

33
import (
4+
"fmt"
45
"os"
56
"path/filepath"
67
"strings"
@@ -1036,6 +1037,206 @@ base_path: /test/parent-should-not-find
10361037
}
10371038
}
10381039

1040+
// TestReadParentDirConfig_SkipsWhenLocalConfigExists verifies that parent directory
1041+
// search is skipped when CWD has local Atmos config (per PRD constraint).
1042+
func TestReadParentDirConfig_SkipsWhenLocalConfigExists(t *testing.T) {
1043+
tests := []struct {
1044+
name string
1045+
setupDirs func(t *testing.T, tempDir string) string
1046+
expectParentLoad bool
1047+
}{
1048+
{
1049+
name: "skips parent search when CWD has atmos.yaml",
1050+
setupDirs: func(t *testing.T, tempDir string) string {
1051+
// Create child dir with its own config.
1052+
childDir := filepath.Join(tempDir, "child")
1053+
require.NoError(t, os.MkdirAll(childDir, 0o755))
1054+
1055+
// Create config in parent.
1056+
createTestConfig(t, tempDir, `base_path: /from/parent`)
1057+
1058+
// Create config in child.
1059+
createTestConfig(t, childDir, `base_path: /from/child`)
1060+
1061+
return childDir
1062+
},
1063+
expectParentLoad: false,
1064+
},
1065+
{
1066+
name: "skips parent search when CWD has .atmos.yaml",
1067+
setupDirs: func(t *testing.T, tempDir string) string {
1068+
childDir := filepath.Join(tempDir, "child")
1069+
require.NoError(t, os.MkdirAll(childDir, 0o755))
1070+
1071+
// Create config in parent.
1072+
createTestConfig(t, tempDir, `base_path: /from/parent`)
1073+
1074+
// Create hidden config in child.
1075+
err := os.WriteFile(filepath.Join(childDir, ".atmos.yaml"), []byte(`base_path: /from/child`), 0o644)
1076+
require.NoError(t, err)
1077+
1078+
return childDir
1079+
},
1080+
expectParentLoad: false,
1081+
},
1082+
{
1083+
name: "skips parent search when CWD has .atmos.d directory",
1084+
setupDirs: func(t *testing.T, tempDir string) string {
1085+
childDir := filepath.Join(tempDir, "child")
1086+
require.NoError(t, os.MkdirAll(childDir, 0o755))
1087+
1088+
// Create config in parent.
1089+
createTestConfig(t, tempDir, `base_path: /from/parent`)
1090+
1091+
// Create .atmos.d directory in child (no config file).
1092+
require.NoError(t, os.MkdirAll(filepath.Join(childDir, ".atmos.d"), 0o755))
1093+
1094+
return childDir
1095+
},
1096+
expectParentLoad: false,
1097+
},
1098+
{
1099+
name: "searches parent when CWD has no config",
1100+
setupDirs: func(t *testing.T, tempDir string) string {
1101+
childDir := filepath.Join(tempDir, "child")
1102+
require.NoError(t, os.MkdirAll(childDir, 0o755))
1103+
1104+
// Create config in parent only.
1105+
createTestConfig(t, tempDir, `base_path: /from/parent`)
1106+
1107+
return childDir
1108+
},
1109+
expectParentLoad: true,
1110+
},
1111+
}
1112+
1113+
for _, tt := range tests {
1114+
t.Run(tt.name, func(t *testing.T) {
1115+
tempDir := t.TempDir()
1116+
workDir := tt.setupDirs(t, tempDir)
1117+
1118+
t.Chdir(workDir)
1119+
1120+
v := viper.New()
1121+
v.SetConfigType("yaml")
1122+
1123+
err := readParentDirConfig(v)
1124+
require.NoError(t, err)
1125+
1126+
if tt.expectParentLoad {
1127+
assert.Equal(t, "/from/parent", v.GetString("base_path"),
1128+
"Should load parent config when CWD has no local config")
1129+
} else {
1130+
assert.Empty(t, v.GetString("base_path"),
1131+
"Should NOT load parent config when CWD has local config")
1132+
}
1133+
})
1134+
}
1135+
}
1136+
1137+
// TestReadGitRootConfig_SkipsWhenLocalConfigExists verifies that git root
1138+
// search is skipped when CWD has local Atmos config (per PRD constraint).
1139+
// This test uses TEST_GIT_ROOT to mock a git root with config that WOULD be loaded
1140+
// if not for the hasLocalAtmosConfig() gating.
1141+
func TestReadGitRootConfig_SkipsWhenLocalConfigExists(t *testing.T) {
1142+
tests := []struct {
1143+
name string
1144+
setupDirs func(t *testing.T, cwdDir, gitRootDir string)
1145+
expectGitRootLoad bool
1146+
}{
1147+
{
1148+
name: "skips git root search when CWD has atmos.yaml",
1149+
setupDirs: func(t *testing.T, cwdDir, gitRootDir string) {
1150+
// Create config at mock git root that WOULD be loaded.
1151+
createTestConfig(t, gitRootDir, `base_path: /from/git-root`)
1152+
// Create local config in CWD - should prevent git root loading.
1153+
createTestConfig(t, cwdDir, `base_path: /from/local`)
1154+
},
1155+
expectGitRootLoad: false,
1156+
},
1157+
{
1158+
name: "skips git root search when CWD has .atmos directory",
1159+
setupDirs: func(t *testing.T, cwdDir, gitRootDir string) {
1160+
createTestConfig(t, gitRootDir, `base_path: /from/git-root`)
1161+
require.NoError(t, os.MkdirAll(filepath.Join(cwdDir, ".atmos"), 0o755))
1162+
},
1163+
expectGitRootLoad: false,
1164+
},
1165+
{
1166+
name: "skips git root search when CWD has atmos.d directory",
1167+
setupDirs: func(t *testing.T, cwdDir, gitRootDir string) {
1168+
createTestConfig(t, gitRootDir, `base_path: /from/git-root`)
1169+
require.NoError(t, os.MkdirAll(filepath.Join(cwdDir, "atmos.d"), 0o755))
1170+
},
1171+
expectGitRootLoad: false,
1172+
},
1173+
{
1174+
name: "loads git root config when CWD has no local config",
1175+
setupDirs: func(t *testing.T, cwdDir, gitRootDir string) {
1176+
// Create config at mock git root.
1177+
createTestConfig(t, gitRootDir, `base_path: /from/git-root`)
1178+
// CWD has no local config - git root should be loaded.
1179+
},
1180+
expectGitRootLoad: true,
1181+
},
1182+
}
1183+
1184+
for _, tt := range tests {
1185+
t.Run(tt.name, func(t *testing.T) {
1186+
// Create separate directories for CWD and mock git root.
1187+
cwdDir := t.TempDir()
1188+
gitRootDir := t.TempDir()
1189+
1190+
tt.setupDirs(t, cwdDir, gitRootDir)
1191+
1192+
t.Chdir(cwdDir)
1193+
1194+
// Point TEST_GIT_ROOT to our mock git root with config.
1195+
t.Setenv("TEST_GIT_ROOT", gitRootDir)
1196+
1197+
v := viper.New()
1198+
v.SetConfigType("yaml")
1199+
1200+
err := readGitRootConfig(v)
1201+
require.NoError(t, err)
1202+
1203+
if tt.expectGitRootLoad {
1204+
assert.Equal(t, "/from/git-root", v.GetString("base_path"),
1205+
"Should load git root config when CWD has no local config")
1206+
} else {
1207+
assert.Empty(t, v.GetString("base_path"),
1208+
"Should NOT load git root config when CWD has local config")
1209+
}
1210+
})
1211+
}
1212+
}
1213+
1214+
// TestReadGitRootConfig_GetwdError verifies that readGitRootConfig gracefully handles
1215+
// os.Getwd failure by falling through to git root detection instead of failing.
1216+
func TestReadGitRootConfig_GetwdError(t *testing.T) {
1217+
// Override osGetwd to simulate failure.
1218+
originalGetwd := osGetwd
1219+
osGetwd = func() (string, error) {
1220+
return "", fmt.Errorf("simulated getwd failure")
1221+
}
1222+
t.Cleanup(func() { osGetwd = originalGetwd })
1223+
1224+
// Set up a mock git root with config.
1225+
gitRootDir := t.TempDir()
1226+
createTestConfig(t, gitRootDir, `base_path: /from/git-root`)
1227+
t.Setenv("TEST_GIT_ROOT", gitRootDir)
1228+
1229+
v := viper.New()
1230+
v.SetConfigType("yaml")
1231+
1232+
err := readGitRootConfig(v)
1233+
require.NoError(t, err)
1234+
1235+
// When os.Getwd fails, the function should still proceed and load git root config.
1236+
assert.Equal(t, "/from/git-root", v.GetString("base_path"),
1237+
"Should load git root config when os.Getwd fails (fallthrough behavior)")
1238+
}
1239+
10391240
// TestLoadConfig_DefaultConfigWithGitRootAtmosD tests that .atmos.d at git root is loaded
10401241
// even when no atmos.yaml config file is found (using default config).
10411242
func TestLoadConfig_DefaultConfigWithGitRootAtmosD(t *testing.T) {

‎tests/cli_test.go‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -912,13 +912,26 @@ func runCLICommandTest(t *testing.T, tc TestCase) {
912912
// Set ATMOS_CLI_CONFIG_PATH to ensure test isolation.
913913
// This forces Atmos to use the workdir's atmos.yaml instead of searching
914914
// up the directory tree or using ~/.config/atmos/atmos.yaml.
915-
atmosConfigPath := filepath.Join(absoluteWorkdir, "atmos.yaml")
916-
if _, err := os.Stat(atmosConfigPath); err == nil {
917-
if tc.Env == nil {
918-
tc.Env = make(map[string]string)
915+
// BUT: Skip this for tests that use --chdir/-C, since they need to test
916+
// config loading in the target directory.
917+
usesChdirFlag := false
918+
for _, arg := range tc.Args {
919+
if arg == "--chdir" || arg == "-C" || strings.HasPrefix(arg, "--chdir=") {
920+
usesChdirFlag = true
921+
break
919922
}
920-
tc.Env["ATMOS_CLI_CONFIG_PATH"] = absoluteWorkdir
921-
logger.Debug("Setting ATMOS_CLI_CONFIG_PATH for test isolation", "path", absoluteWorkdir)
923+
}
924+
if !usesChdirFlag {
925+
atmosConfigPath := filepath.Join(absoluteWorkdir, "atmos.yaml")
926+
if _, err := os.Stat(atmosConfigPath); err == nil {
927+
if tc.Env == nil {
928+
tc.Env = make(map[string]string)
929+
}
930+
tc.Env["ATMOS_CLI_CONFIG_PATH"] = absoluteWorkdir
931+
logger.Debug("Setting ATMOS_CLI_CONFIG_PATH for test isolation", "path", absoluteWorkdir)
932+
}
933+
} else {
934+
logger.Debug("Skipping ATMOS_CLI_CONFIG_PATH for --chdir test")
922935
}
923936
}
924937

‎tests/fixtures/scenarios/chdir-isolation/atmos.yaml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# Minimal config for testing --chdir config isolation.
2+
# This config has unique values that should NOT appear when --chdir'ing
3+
# to directories with their own config.
4+
base_path: "."
5+
6+
vendor:
7+
base_path: "chdir-isolation-vendor"
8+
9+
components:
10+
terraform:
11+
base_path: "chdir-isolation-components"
12+
command: terraform
13+
14+
stacks:
15+
base_path: "chdir-isolation-stacks"
16+
included_paths:
17+
- "**/*"
18+
name_template: "{{.vars.stage}}"

‎tests/snapshots/TestCLICommands_atmos_--chdir_config_isolation.stderr.golden‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)