Fix: Enable auto-import of provisioned identities from profiles#1865
Fix: Enable auto-import of provisioned identities from profiles#1865
Conversation
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughInjects provisioned identities after profile loading (non-fatal on failure), processes per-file imports before merging, re-applies user config and profiles for precedence, preserves original-case provisioned identities via IdentityCaseMap, and exposes GetIdentityDisplayName on AuthManager used by formatters. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI
participant ConfigLoader
participant FS as FileSystem/Imports
participant Profiles
participant XDG as XDG Cache
participant AuthMgr
participant Formatter
User->>CLI: run command (load config)
CLI->>ConfigLoader: LoadConfig()
ConfigLoader->>FS: scan directory (detect files)
FS-->>ConfigLoader: file list (processFileImportsIfPresent)
ConfigLoader->>Profiles: load and merge profiles
Profiles-->>ConfigLoader: merged profiles + user config
ConfigLoader->>XDG: injectProvisionedIdentitiesPostLoad()
Note right of XDG: read per-provider cache YAML\nextract auth.identities (preserve case)
XDG-->>ConfigLoader: populate IdentityCaseMap (non-fatal)
ConfigLoader->>ConfigLoader: reapplyUserConfigForPrecedence()
ConfigLoader->>ConfigLoader: reapply profiles if specified
ConfigLoader->>ConfigLoader: preserveProvisionedIdentityCase()
ConfigLoader-->>CLI: final AtmosConfiguration
CLI->>AuthMgr: ResolveIdentity(...) / GetIdentityDisplayName(key)
AuthMgr-->>CLI: display name (original case or fallback)
CLI->>Formatter: format list/tree (uses GetIdentityDisplayName)
Formatter-->>User: rendered output with preserved case
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📝 WalkthroughWalkthroughThe changes extend the config loading pipeline with post-load provisioned identity injection, user config precedence reapplication, and import processing hooks. Functions enable provisioned identities to load after profiles, ensure user config overrides provisioned data, and process import directives within individual config files with non-fatal error handling. Changes
Sequence DiagramsequenceDiagram
participant Client
participant LoadConfig
participant ProfileLoader
participant ProvisionedLoader
participant UserConfig
participant ImportProcessor
Client->>LoadConfig: LoadConfig()
LoadConfig->>ProfileLoader: Load & apply profiles
ProfileLoader-->>LoadConfig: Profiles applied
LoadConfig->>ProvisionedLoader: injectProvisionedIdentitiesPostLoad()
ProvisionedLoader->>ProvisionedLoader: Read provisioned-identities.yaml
ProvisionedLoader-->>LoadConfig: Identities injected (non-fatal)
LoadConfig->>ProfileLoader: reapplyProfilesToEnforceConfigPrecedence()
ProfileLoader-->>LoadConfig: Profiles re-applied
LoadConfig->>UserConfig: reapplyUserConfigForPrecedence()
UserConfig->>UserConfig: Re-merge main config file
UserConfig-->>LoadConfig: User config precedence restored
LoadConfig->>ImportProcessor: processFileImportsIfPresent()
ImportProcessor->>ImportProcessor: Parse & apply imports
ImportProcessor-->>LoadConfig: Imports processed (non-fatal)
LoadConfig-->>Client: Final merged config
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/config/load.go (1)
842-890: Process imports before merging the file to match documented precedence semantics.
loadAtmosConfigsFromDirectory()currently merges the file first, then processes its imports. Since Viper'sMergeConfig()overwrites existing keys, imports end up overwriting the file's values—backwards from the documented behavior inmergeConfig()("importing file always takes precedence over imported files"). This is especially problematic for profile files.Swap the order: process imports first, then merge the file itself.
for _, filePath := range foundPaths { + // Process any imports in the file first so the file itself can override them. + // This matches mergeConfig() precedence semantics. + if err := processFileImportsIfPresent(filePath, filepath.Dir(filePath), dst); err != nil { + log.Debug("Failed to process imports in file", "file", filePath, "error", err) + // Non-fatal: continue loading even if import processing fails. + } + if err := mergeConfigFile(filePath, dst); err != nil { return fmt.Errorf("%w: failed to load configuration file from %s: %s: %w", errUtils.ErrParseFile, source, filePath, err) } - - // Process any imports in the file. - // This enables import: directives to work in profile files. - if err := processFileImportsIfPresent(filePath, filepath.Dir(filePath), dst); err != nil { - log.Debug("Failed to process imports in file", "file", filePath, "error", err) - // Non-fatal: continue loading even if import processing fails. - } log.Trace("Loaded configuration file", "path", filePath, "source", source) }
🧹 Nitpick comments (2)
pkg/config/load_test.go (2)
1039-1158: Small test cleanup: remove (or assert on) the unusedexpectIdentitiesLoadfield.
Right now it’s set per case but never read, which makes the table harder to scan.
1189-1257: Add coverage for “user override via imported config” (not inline) to validate precedence re-application.
Given the new “re-apply user config” step, a key case is:atmos.yamlimports another file that defines/overridesauth.identities.*; provisioned injection should not permanently win over those imported user values.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
pkg/config/load.go(4 hunks)pkg/config/load_test.go(1 hunks)pkg/config/profiles_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: All comments must end with periods (enforced bygodotlinter)
NEVER delete existing comments without a very strong reason. Comments document why/how/what/where. Preserve helpful comments, update to match code, refactor for clarity, add context when modifying.
Organize imports in three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages. Maintain aliases:cfg,log,u,errUtils
Adddefer perf.Track(atmosConfig, "pkg.FuncName")()with a blank line to all public functions. Usenilif no atmosConfig parameter is available.
Define interfaces for all major functionality. Use dependency injection for testability. Generate mocks withgo.uber.org/mock/mockgen. Avoid integration tests by mocking external dependencies.
Use//go:generatedirectives withgo.uber.org/mock/mockgenfor mock generation. Never creat...
Files:
pkg/config/load_test.gopkg/config/profiles_test.gopkg/config/load.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
**/*_test.go: Prefer unit tests with mocks over integration tests. Use interfaces and dependency injection for testability. Generate mocks withgo.uber.org/mock/mockgen. Use table-driven tests for comprehensive coverage. Target >80% coverage.
Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable using DI. No coverage theater. Remove always-skipped tests. Table-driven tests need real scenarios. Useassert.ErrorIs(err, ErrSentinel)for our/stdlib errors.
Tests must call actual production code, never duplicate logic.
Uset.Skipf("reason")with clear context for test skipping. CLI tests auto-build temp binaries.
Never use pipe redirection (2>&1,| head,| tail) when running tests. Piping breaks TTY detection → ASCII fallback → wrong snapshots.
Tests skip gracefully with helpers fromtests/test_preconditions.go.
Files:
pkg/config/load_test.gopkg/config/profiles_test.go
pkg/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
pkg/**/*.go: For complex domains with multiple operations, use Service-Oriented Architecture with one Service struct per domain, provider interfaces for classes of problems, default implementations, mock implementations, and dependency injection for testability.
Follow multi-provider registry pattern: 1) Define interface in dedicated package, 2) Implement per provider, 3) Register implementations, 4) Generate mocks.
Files:
pkg/config/load_test.gopkg/config/profiles_test.gopkg/config/load.go
🧠 Learnings (19)
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-12-10T23:12:15.805Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T23:12:15.805Z
Learning: Applies to **/*_test.go : Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable using DI. No coverage theater. Remove always-skipped tests. Table-driven tests need real scenarios. Use `assert.ErrorIs(err, ErrSentinel)` for our/stdlib errors.
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-10-02T19:17:51.630Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1504
File: pkg/profiler/profiler.go:20-31
Timestamp: 2025-10-02T19:17:51.630Z
Learning: In pkg/profiler/profiler.go, profiler-specific errors (ErrUnsupportedProfileType, ErrStartCPUProfile, ErrStartTraceProfile, ErrCreateProfileFile) must remain local and cannot be moved to errors/errors.go due to an import cycle: pkg/profiler → errors → pkg/schema → pkg/profiler. This is a valid exception to the centralized errors policy.
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-12-10T23:12:15.805Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T23:12:15.805Z
Learning: Applies to **/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` with a blank line to all public functions. Use `nil` if no atmosConfig parameter is available.
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-12-10T23:12:15.805Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T23:12:15.805Z
Learning: Applies to **/*_test.go : Tests skip gracefully with helpers from `tests/test_preconditions.go`.
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-12-10T23:12:15.805Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T23:12:15.805Z
Learning: Applies to **/*.go : Use functional options pattern for configuration instead of many parameters. Provides defaults and extensibility without breaking changes.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-12-10T23:12:15.805Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T23:12:15.805Z
Learning: Applies to cmd/**/*.go : Configuration loading precedence: CLI flags → ENV vars → config files → defaults (use Viper).
Applied to files:
pkg/config/load.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-02-09T18:43:53.902Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 808
File: pkg/config/loader.go:141-145
Timestamp: 2025-02-09T18:43:53.902Z
Learning: In the Atmos configuration loading process, errors during individual config file loading/merging should be logged but not propagate up to break the entire process. This design choice enables resilient partial configuration loading.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-12-10T23:12:15.805Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T23:12:15.805Z
Learning: Applies to cmd/**/*.go : Use `viper.BindEnv("ATMOS_VAR", "ATMOS_VAR", "FALLBACK")` - ATMOS_ prefix required for environment variables.
Applied to files:
pkg/config/load.go
📚 Learning: 2024-11-19T23:00:45.899Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.
Applied to files:
pkg/config/load.go
🧬 Code graph analysis (1)
pkg/config/load.go (3)
pkg/schema/schema.go (1)
AtmosConfiguration(53-96)pkg/xdg/xdg.go (1)
GetXDGCacheDir(33-35)pkg/auth/provisioning/writer.go (1)
ProvisionedFileName(26-26)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (1)
pkg/config/profiles_test.go (1)
489-568: Nice coverage forprocessFileImportsIfPresenthappy-path / no-op / missing import.
This aligns with the “non-fatal import processing” design.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (70.50%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1865 +/- ##
==========================================
+ Coverage 73.33% 73.48% +0.15%
==========================================
Files 564 566 +2
Lines 54336 54732 +396
==========================================
+ Hits 39847 40221 +374
Misses 11625 11625
- Partials 2864 2886 +22
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
pkg/config/load.go (2)
1027-1035: Guard correctly respectsauto_provision_identitiessetting.The nil check combined with the boolean check ensures cached identities are only loaded when explicitly enabled. This addresses the previous review concern.
1053-1075: Imports are re-processed for proper precedence.Using
mergeConfig(v, dir, name, true)ensures that overrides in user's imported files also take precedence over provisioned identities. This addresses the previous review concern.pkg/config/profiles_test.go (1)
393-500: Precedence semantics are correctly tested.The test at lines 439-447 verifies that importing file takes precedence over imported files (expects 120, not 80). The subdirectory placement at line 438 (
importSubdir: "imports") prevents double-loading via directory scan.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
pkg/config/load.go(5 hunks)pkg/config/load_test.go(1 hunks)pkg/config/profiles_test.go(1 hunks)tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.golden(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: Comment style: All comments must end with periods (enforced bygodotlinter)
Import organization: Three groups separated by blank lines (stdlib, 3rd-party, Atmos packages), sorted alphabetically within groups, maintain aliases:cfg,log,u,errUtils
Performance tracking: Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions. Usenilif no atmosConfig param
Error handling: ALL user-facing errors MUST use ErrorBuilder with sentinel errors fromerrors/errors.go. ALWAYS useerrors.Is()for checking, NEVER string comparison. ALWAYS useassert.ErrorIs()in tests, NEVERassert.Contains(err.Error(), ...)
Mock generation: Usego.uber.org/mock/mockgenwith//go:generatedirectives. Never manual mocks
File organization: Keep files small and focused (<600 lines). One cmd/impl per file. Co-locate tests. Never use `//revive:disable:file-lengt...
Files:
pkg/config/profiles_test.gopkg/config/load.gopkg/config/load_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
**/*_test.go: Test skipping conventions: Uset.Skipf("reason")with clear context. CLI tests auto-build temp binaries
Golden snapshots: NEVER manually edit golden snapshot files - Always use-regenerate-snapshotsflag. Snapshots capture exact output including invisible formatting (lipgloss padding, ANSI codes, trailing whitespace). Different environments produce different output. Never use pipe redirection (2>&1,| head,| tail) when running tests as piping breaks TTY detection
Files:
pkg/config/profiles_test.gopkg/config/load_test.go
🧠 Learnings (46)
📓 Common learnings
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1085
File: pkg/config/imports.go:68-75
Timestamp: 2025-03-17T18:41:08.831Z
Learning: In the Atmos configuration import system, errors during config file merging are logged at debug level and the process continues with other imports rather than failing completely, prioritizing resilience over strict correctness.
Learnt from: haitham911
Repo: cloudposse/atmos PR: 808
File: pkg/config/loader.go:141-145
Timestamp: 2025-02-09T18:43:53.902Z
Learning: In the Atmos configuration loading process, errors during individual config file loading/merging should be logged but not propagate up to break the entire process. This design choice enables resilient partial configuration loading.
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1085
File: pkg/config/load.go:219-221
Timestamp: 2025-02-24T22:46:39.744Z
Learning: In the Atmos configuration system, imports from atmos.d are optional. When import errors occur, they should be logged at debug level and the process should continue, rather than failing completely.
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/http_client_test.go:3-10
Timestamp: 2025-09-10T21:17:55.273Z
Learning: In the cloudposse/atmos repository, imports should never be changed as per samtholiya's coding guidelines.
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos-cli-imports/atmos.yaml:8-8
Timestamp: 2025-01-25T15:21:40.413Z
Learning: In Atmos, when a directory is specified for configuration loading (e.g., in the `import` section of atmos.yaml), all files within that directory should be treated as Atmos configurations. Do not suggest restricting file extensions in directory-based glob patterns.
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-10-02T19:17:51.630Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1504
File: pkg/profiler/profiler.go:20-31
Timestamp: 2025-10-02T19:17:51.630Z
Learning: In pkg/profiler/profiler.go, profiler-specific errors (ErrUnsupportedProfileType, ErrStartCPUProfile, ErrStartTraceProfile, ErrCreateProfileFile) must remain local and cannot be moved to errors/errors.go due to an import cycle: pkg/profiler → errors → pkg/schema → pkg/profiler. This is a valid exception to the centralized errors policy.
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to **/*.go : Import organization: Three groups separated by blank lines (stdlib, 3rd-party, Atmos packages), sorted alphabetically within groups, maintain aliases: `cfg`, `log`, `u`, `errUtils`
Applied to files:
pkg/config/profiles_test.gopkg/config/load.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to **/*.go : Configuration loading: Precedence: CLI flags → ENV vars → config files → defaults (use Viper)
Applied to files:
pkg/config/profiles_test.gopkg/config/load.go
📚 Learning: 2024-11-19T23:00:45.899Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.
Applied to files:
pkg/config/profiles_test.gopkg/config/load.go
📚 Learning: 2025-09-24T20:45:40.401Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: tests/fixtures/scenarios/atmos-auth/stacks/deploy/nonprod.yaml:3-4
Timestamp: 2025-09-24T20:45:40.401Z
Learning: In Atmos stack files, the correct syntax for importing other stack files is `import:` (singular), not `imports:` (plural). All stack files in the Atmos codebase consistently use `import:` followed by a list of paths to import.
Applied to files:
pkg/config/profiles_test.gotests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.golden
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
pkg/config/profiles_test.gopkg/config/load.go
📚 Learning: 2025-09-10T21:17:55.273Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/http_client_test.go:3-10
Timestamp: 2025-09-10T21:17:55.273Z
Learning: In the cloudposse/atmos repository, imports should never be changed as per samtholiya's coding guidelines.
Applied to files:
pkg/config/profiles_test.gopkg/config/load.gotests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.golden
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
pkg/config/profiles_test.gotests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
pkg/config/profiles_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.
Applied to files:
pkg/config/profiles_test.gotests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/config/load.gotests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2024-12-25T20:28:19.618Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/workflow_utils.go:167-169
Timestamp: 2024-12-25T20:28:19.618Z
Learning: The user plans to revert the change from `path.Join` to `filepath.Join` in this PR due to testing gaps and will open a new PR to safely handle the migration without breaking `main`.
Applied to files:
pkg/config/load.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
pkg/config/load.gotests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
pkg/config/load.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
pkg/config/load.gotests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.golden
📚 Learning: 2024-10-22T23:00:20.627Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:131-141
Timestamp: 2024-10-22T23:00:20.627Z
Learning: In the `ReadAndProcessVendorConfigFile` function in `internal/exec/vendor_utils.go`, the existence of the vendor config file is already checked, so additional file existence checks may be unnecessary.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-03-17T18:41:08.831Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1085
File: pkg/config/imports.go:68-75
Timestamp: 2025-03-17T18:41:08.831Z
Learning: In the Atmos configuration import system, errors during config file merging are logged at debug level and the process continues with other imports rather than failing completely, prioritizing resilience over strict correctness.
Applied to files:
pkg/config/load.gotests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2025-01-17T00:18:57.769Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
Applied to files:
pkg/config/load.go
📚 Learning: 2024-11-22T12:38:33.132Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.
Applied to files:
pkg/config/load.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to **/*.go : Environment variables: Use `viper.BindEnv("ATMOS_VAR", "ATMOS_VAR", "FALLBACK")` - ATMOS_ prefix required
Applied to files:
pkg/config/load.go
📚 Learning: 2025-02-09T18:43:53.902Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 808
File: pkg/config/loader.go:141-145
Timestamp: 2025-02-09T18:43:53.902Z
Learning: In the Atmos configuration loading process, errors during individual config file loading/merging should be logged but not propagate up to break the entire process. This design choice enables resilient partial configuration loading.
Applied to files:
pkg/config/load.gotests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Applied to files:
pkg/config/load.go
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2025-02-14T23:12:38.030Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1061
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:8-8
Timestamp: 2025-02-14T23:12:38.030Z
Learning: Test snapshots in the Atmos project, particularly for dry run scenarios, may be updated during the development process, and temporary inconsistencies in their content should not be flagged as issues.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Test data: Use fixtures in `tests/test-cases/`: `atmos.yaml`, `stacks/`, `components/`. NEVER modify `tests/test-cases/` or `tests/testdata/` unless explicitly instructed. Golden snapshots are sensitive to minor changes
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2025-04-03T10:57:04.602Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1091
File: pkg/config/load_config_args.go:61-82
Timestamp: 2025-04-03T10:57:04.602Z
Learning: In the Atmos configuration system, errors in `mergeDefaultImports` and `mergeImports` functions are intentionally only logged as debug messages without returning errors, as they are considered non-critical. Only errors in critical functions like `mergeConfigFile` are returned to halt execution.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.golden
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.golden
📚 Learning: 2025-02-03T06:00:11.419Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/describe_config.go:20-20
Timestamp: 2025-02-03T06:00:11.419Z
Learning: The `describe config` command should use `PrintErrorMarkdownAndExit` with empty title and suggestion for consistency with other commands.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.golden
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2025-02-24T22:46:39.744Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1085
File: pkg/config/load.go:219-221
Timestamp: 2025-02-24T22:46:39.744Z
Learning: In the Atmos configuration system, imports from atmos.d are optional. When import errors occur, they should be logged at debug level and the process should continue, rather than failing completely.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2024-11-25T17:17:15.703Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.golden
📚 Learning: 2025-01-25T15:21:40.413Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos-cli-imports/atmos.yaml:8-8
Timestamp: 2025-01-25T15:21:40.413Z
Learning: In Atmos, when a directory is specified for configuration loading (e.g., in the `import` section of atmos.yaml), all files within that directory should be treated as Atmos configurations. Do not suggest restricting file extensions in directory-based glob patterns.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.golden
📚 Learning: 2025-02-06T13:38:07.216Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 984
File: internal/exec/copy_glob.go:0-0
Timestamp: 2025-02-06T13:38:07.216Z
Learning: The `u.LogTrace` function in the `cloudposse/atmos` repository accepts `atmosConfig` as its first parameter, followed by the message string.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.golden
📚 Learning: 2024-10-23T22:11:41.077Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/path_utils.go:145-146
Timestamp: 2024-10-23T22:11:41.077Z
Learning: In the `atmos` project, the preference is to print relative paths in log messages instead of full paths.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.golden
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Adding new CLI command: 1) Create `cmd/[command]/` with CommandProvider interface, 2) Add blank import to `cmd/root.go`: `_ "github.com/cloudposse/atmos/cmd/mycommand"`, 3) Implement in `internal/exec/mycommand.go`, 4) Add tests in `cmd/mycommand/mycommand_test.go`, 5) Create Docusaurus docs in `website/docs/cli/commands/<command>/<subcommand>.mdx`, 6) Build website: `cd website && npm run build`. See `docs/developing-atmos-commands.md` and `docs/prd/command-registry-pattern.md`
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.goldentests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
📚 Learning: 2025-04-03T11:00:27.980Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1091
File: pkg/config/imports.go:0-0
Timestamp: 2025-04-03T11:00:27.980Z
Learning: The Atmos configuration system intentionally supports both HTTP and HTTPS protocols for remote imports, as confirmed by the project maintainers. This is implemented in the `isRemoteImport` and `processRemoteImport` functions in pkg/config/imports.go.
Applied to files:
tests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.golden
📚 Learning: 2025-02-03T05:57:18.407Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/cmd_utils.go:121-148
Timestamp: 2025-02-03T05:57:18.407Z
Learning: The Atmos CLI should fail fast (exit) when encountering configuration errors, including command alias configuration issues, to prevent undefined behavior. Use LogErrorAndExit instead of returning errors in such cases.
Applied to files:
tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
🧬 Code graph analysis (1)
pkg/config/load.go (3)
pkg/schema/schema.go (1)
AtmosConfiguration(53-96)pkg/xdg/xdg.go (1)
GetXDGCacheDir(33-35)pkg/auth/provisioning/writer.go (1)
ProvisionedFileName(26-26)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (9)
tests/snapshots/TestCLICommands_atmos_describe_configuration.stderr.golden (1)
1-24: Snapshot reflects increased config loading iterations.The additional "Loaded configuration directory" entries align with the new post-load provisioning and reapplication logic. Config is now loaded multiple times to support precedence semantics.
tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden (1)
7-32: Trace logs capture new provisioning workflow.The new trace messages document the post-load provisioning checks and user config reapplication. This provides good observability into the config loading sequence.
pkg/config/load.go (3)
275-303: Post-load provisioning flow is well-structured.The sequence is correct:
- Inject provisioned identities after profiles
- Re-apply user config for precedence
- Re-apply profiles if specified
Non-fatal error handling maintains resilience per the project's design philosophy. Based on learnings, errors during config file merging should be logged and continue rather than fail completely.
869-899: Clean helper for import processing.Good separation of concerns. The early return when no imports exist avoids unnecessary work.
837-858: Import-first processing maintains correct precedence.The comment at lines 842-844 is a helpful warning. Users should place import files in subdirectories to avoid double-loading.
pkg/config/profiles_test.go (1)
502-581: Good coverage for import processing edge cases.Tests cover no imports, valid imports, and missing import files. The missing import file test correctly expects non-fatal behavior per project resilience guidelines.
pkg/config/load_test.go (2)
1039-1164: Comprehensive test coverage for post-load injection.Table-driven tests cover all key scenarios including the auto_provision_identities guard (lines 1139-1163). Good use of
t.TempDir()andt.Setenv()for test isolation.
1219-1288: End-to-end precedence test validates the complete flow.This test correctly demonstrates that:
- Provisioned identities merge with existing config
- User config reapplication restores user-defined values
- Auto-provisioned identities not overridden by user persist
Solid integration test for the new functionality.
tests/snapshots/TestCLICommands_atmos_describe_config_imports.stderr.golden (1)
7-7: Snapshot correctly captures the new multi-stage processing flow.The additional
processConfigImportslog entries align with the PR's new post-load provisioning and profile import handling. These duplicate messages appear to be intentional trace points showing the import resolution happening at multiple stages.Worth confirming: if these represent separate logical stages (initial load, profile imports, post-load injection), the duplication makes sense. If they indicate redundant work, there might be room for optimization. Based on the AI summary mentioning "probe points", this looks intentional.
Also applies to: 9-9, 17-17, 21-21, 23-23
github-actions
bot
added
size/xl
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
mergify
bot
added
conflict
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
pkg/config/load_test.go (2)
1040-1189: injectProvisionedIdentitiesPostLoad tests cover key provisioning scenarios well.The table-driven cases exercise no providers, providers without caches, enabled/disabled
auto_provision_identities, and multi-provider behavior against the XDG cache layout, which is what this path needs. One tiny nit: theexpectIdentitiesLoadfield isn’t used anywhere and could be dropped or wired into assertions later.
1760-1791: XDG cache directory failure behavior is clearly specified.
TestPreserveProvisionedIdentityCase_XDGCacheDirErrorcorrectly simulates a file blocking directory creation and asserts that an error is returned, which ensures we don’t silently ignore fundamental cache-dir setup failures. If you later introduce a sentinel error for this path, consider switching toassert.ErrorIsto align with the repo’s preferred error-checking pattern.pkg/auth/manager.go (1)
492-504: Consider normalizing the key inside GetIdentityDisplayName for robustness.Today callers pass lowercase identity keys, so the lookup works. To make this more future-proof if a caller ever passes a mixed-case name, you could normalize inside the helper and still return the original input as a fallback:
-func (m *manager) GetIdentityDisplayName(lowercaseKey string) string { - defer perf.Track(nil, "auth.Manager.GetIdentityDisplayName")() - - if m.config.IdentityCaseMap != nil { - if originalName, exists := m.config.IdentityCaseMap[lowercaseKey]; exists { - return originalName - } - } - return lowercaseKey -} +func (m *manager) GetIdentityDisplayName(identityKey string) string { + defer perf.Track(nil, "auth.Manager.GetIdentityDisplayName")() + + key := strings.ToLower(identityKey) + if m.config.IdentityCaseMap != nil { + if originalName, exists := m.config.IdentityCaseMap[key]; exists { + return originalName + } + } + return identityKey +}This keeps current behavior while relaxing assumptions about caller input.
pkg/auth/list/formatter_tree.go (1)
440-445: Using displayName from AuthManager is a good improvement; consider a tiny fallback.Pulling
displayNameviaauthManager.GetIdentityDisplayName(name)and using it consistently in the title keeps UI aligned with original identity casing while preserving behavior whenauthManageris nil. IfGetIdentityDisplayNamecan ever return"", you might optionally fall back tonamehere to avoid blank titles, but that is purely defensive.Also applies to: 460-463
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
cmd/auth_console_test.go(2 hunks)internal/exec/terraform_output_utils.go(1 hunks)pkg/auth/hooks_test.go(1 hunks)pkg/auth/list/formatter_table.go(2 hunks)pkg/auth/list/formatter_tree.go(2 hunks)pkg/auth/manager.go(1 hunks)pkg/auth/manager_case_test.go(2 hunks)pkg/auth/providers/aws/saml_test.go(1 hunks)pkg/auth/types/interfaces.go(1 hunks)pkg/auth/types/mock_interfaces.go(2 hunks)pkg/config/load_test.go(4 hunks)pkg/config/provisioned_identity_case.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/auth/manager_case_test.go
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: Comment style: All comments must end with periods (enforced bygodotlinter)
Import organization: Three groups separated by blank lines (stdlib, 3rd-party, Atmos packages), sorted alphabetically within groups, maintain aliases:cfg,log,u,errUtils
Performance tracking: Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions. Usenilif no atmosConfig param
Error handling: ALL user-facing errors MUST use ErrorBuilder with sentinel errors fromerrors/errors.go. ALWAYS useerrors.Is()for checking, NEVER string comparison. ALWAYS useassert.ErrorIs()in tests, NEVERassert.Contains(err.Error(), ...)
Mock generation: Usego.uber.org/mock/mockgenwith//go:generatedirectives. Never manual mocks
File organization: Keep files small and focused (<600 lines). One cmd/impl per file. Co-locate tests. Never use `//revive:disable:file-lengt...
Files:
pkg/auth/hooks_test.gointernal/exec/terraform_output_utils.gopkg/auth/manager.gopkg/auth/list/formatter_table.gopkg/auth/providers/aws/saml_test.gopkg/auth/types/interfaces.gocmd/auth_console_test.gopkg/auth/types/mock_interfaces.gopkg/auth/list/formatter_tree.gopkg/config/provisioned_identity_case.gopkg/config/load_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
**/*_test.go: Test skipping conventions: Uset.Skipf("reason")with clear context. CLI tests auto-build temp binaries
Golden snapshots: NEVER manually edit golden snapshot files - Always use-regenerate-snapshotsflag. Snapshots capture exact output including invisible formatting (lipgloss padding, ANSI codes, trailing whitespace). Different environments produce different output. Never use pipe redirection (2>&1,| head,| tail) when running tests as piping breaks TTY detection
Files:
pkg/auth/hooks_test.gopkg/auth/providers/aws/saml_test.gocmd/auth_console_test.gopkg/config/load_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
Files:
cmd/auth_console_test.go
cmd/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
cmd/**/*_test.go: Testing: Usecmd.NewTestKit(t)for cmd tests to auto-clean RootCmd state (flags, args)
Test isolation: ALWAYS usecmd.NewTestKit(t)for cmd tests to auto-clean RootCmd state (flags, args). Required for any test touching RootCmd
Files:
cmd/auth_console_test.go
🧠 Learnings (31)
📚 Learning: 2025-12-10T18:32:43.260Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:43.260Z
Learning: In cmd subpackages, tests should avoid using NewTestKit(t) unless tests actually interact with RootCmd (e.g., execute commands via RootCmd or modify RootCmd state). For structural tests that only verify command structure/flags without touching RootCmd, TestKit cleanup is unnecessary.
Applied to files:
cmd/auth_console_test.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.
Applied to files:
cmd/auth_console_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
Applied to files:
pkg/auth/types/mock_interfaces.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Interface-driven design: Define interfaces for all major functionality. Use dependency injection for testability. Generate mocks with `go.uber.org/mock/mockgen`. Avoid integration tests by mocking external dependencies
Applied to files:
pkg/auth/types/mock_interfaces.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Multi-provider registry: Follow registry pattern: 1) Define interface in dedicated package, 2) Implement per provider, 3) Register implementations, 4) Generate mocks. Example: `pkg/store/` with AWS SSM, Azure Key Vault, Google Secret Manager
Applied to files:
pkg/auth/types/mock_interfaces.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Testing strategy: Prefer unit tests with mocks over integration tests. Use interfaces + dependency injection for testability. Generate mocks with `go.uber.org/mock/mockgen`. Use table-driven tests for comprehensive coverage. Integration tests in `tests/` only when necessary. Target >80% coverage
Applied to files:
pkg/auth/types/mock_interfaces.gopkg/config/load_test.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to **/*.go : Mock generation: Use `go.uber.org/mock/mockgen` with `//go:generate` directives. Never manual mocks
Applied to files:
pkg/auth/types/mock_interfaces.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: XDG Base Directory Specification compliance implementation for atmos toolchain is complete: created toolchain/xdg_cache.go with GetXDGCacheDir() and GetXDGTempCacheDir() functions, updated toolchain/installer.go and cmd/toolchain_clean.go to use these XDG helpers, and changed all cache paths from hardcoded ~/.cache/tools-cache to XDG-compliant ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback).
Applied to files:
pkg/config/provisioned_identity_case.gopkg/config/load_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain XDG compliance implementation is complete with GetXDGCacheDir() and GetXDGTempCacheDir() functions in toolchain/xdg_cache.go, updated installer.go and toolchain_clean.go to use these helpers, and changed cache paths from ~/.cache/tools-cache to ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain when XDG_CACHE_HOME is not set).
Applied to files:
pkg/config/provisioned_identity_case.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: Final XDG Base Directory Specification implementation for atmos toolchain is complete and verified: toolchain/xdg_cache.go provides GetXDGCacheDir() and GetXDGTempCacheDir() functions, all hardcoded ~/.cache/tools-cache paths have been replaced with XDG-compliant paths using ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback), and tests have been updated to expect the new path structure.
Applied to files:
pkg/config/provisioned_identity_case.gopkg/config/load_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
pkg/config/provisioned_identity_case.gopkg/config/load_test.go
📚 Learning: 2024-12-13T15:33:34.159Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:17-31
Timestamp: 2024-12-13T15:33:34.159Z
Learning: In `pkg/config/cache.go`, when `XDG_CACHE_HOME` is not set, falling back to `.` (current directory) is acceptable and aligns with the requirement to primarily use `XDG_CACHE_HOME` for the cache directory.
Applied to files:
pkg/config/provisioned_identity_case.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/config/provisioned_identity_case.gopkg/config/load_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
pkg/config/load_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
pkg/config/load_test.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Test quality: Test behavior, not implementation. Never test stub functions - implement or remove. Avoid tautological tests - don't test hardcoded stubs return hardcoded values. Make code testable - use DI to avoid `os.Exit`, `CheckErrorPrintAndExit`, external systems. No coverage theater - validate real behavior. Remove always-skipped tests - fix or delete. Table-driven tests need real scenarios. Use `assert.ErrorIs(err, ErrSentinel)` for our/stdlib errors. String matching OK for third-party errors
Applied to files:
pkg/config/load_test.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
pkg/config/load_test.go
📚 Learning: 2025-03-12T21:38:42.699Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1139
File: pkg/config/go-homedir/homedir.go:183-196
Timestamp: 2025-03-12T21:38:42.699Z
Learning: The code in pkg/config/go-homedir is a direct fork of the mitchellh/go-homedir package and was intentionally imported as-is without modifications to maintain consistency with the original source. Security concerns or other improvements may be addressed in future PRs.
Applied to files:
pkg/config/load_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
pkg/config/load_test.go
📚 Learning: 2024-12-25T20:28:47.526Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/stack_processor_utils.go:380-380
Timestamp: 2024-12-25T20:28:47.526Z
Learning: Windows path handling often requires `filepath.Join` to ensure correct separators and comparisons. Insufficient tests can break cross-platform compatibility, so migrating from `path.Join` to `filepath.Join` needs thorough testing on Windows before merging.
Applied to files:
pkg/config/load_test.go
📚 Learning: 2025-12-13T00:52:18.278Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T00:52:18.278Z
Learning: Applies to **/*.go : Cross-platform compatibility: Linux/macOS/Windows compatible. Use SDKs over binaries. Use `filepath.Join()`, not hardcoded separators
Applied to files:
pkg/config/load_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
pkg/config/load_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.
Applied to files:
pkg/config/load_test.go
📚 Learning: 2025-11-10T23:23:39.771Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/registry/aqua/aqua_test.go:417-442
Timestamp: 2025-11-10T23:23:39.771Z
Learning: In Atmos toolchain AquaRegistry, tests should not hit real GitHub. Use the options pattern via WithGitHubBaseURL to inject an httptest server URL and make GetLatestVersion/GetAvailableVersions deterministic.
Applied to files:
pkg/config/load_test.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
pkg/config/load_test.go
📚 Learning: 2025-12-13T03:21:27.620Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:27.620Z
Learning: In Atmos, when calling cfg.InitCliConfig, you must first populate the schema.ConfigAndStacksInfo struct with global flag values using flags.ParseGlobalFlags(cmd, v) rather than passing an empty struct. The LoadConfig function (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) directly from the ConfigAndStacksInfo struct, NOT from Viper. Passing an empty struct causes config selection flags (--base-path, --config, --config-path, --profile) to be silently ignored. Correct pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
pkg/config/load_test.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
pkg/config/load_test.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
pkg/config/load_test.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
pkg/config/load_test.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
pkg/config/load_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests. This is implemented through custom testing framework extensions and is used consistently throughout the test suite.
Applied to files:
pkg/config/load_test.go
🧬 Code graph analysis (3)
pkg/auth/manager.go (1)
pkg/perf/perf.go (1)
Track(121-138)
pkg/config/provisioned_identity_case.go (4)
pkg/schema/schema.go (1)
AtmosConfiguration(53-96)pkg/xdg/xdg.go (1)
GetXDGCacheDir(33-35)pkg/logger/log.go (1)
Debug(24-26)pkg/auth/provisioning/writer.go (1)
ProvisionedFileName(26-26)
pkg/config/load_test.go (2)
pkg/schema/schema.go (1)
AtmosConfiguration(53-96)pkg/schema/schema_auth.go (2)
AuthConfig(4-12)Provider(21-36)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Summary
🔇 Additional comments (12)
pkg/config/provisioned_identity_case.go (1)
23-101: Provisioned identity case preservation and XDG usage look correct.The helper cleanly scans
atmos/auth/*/provisioned-identities.yaml, tolerates missing/unreadable files, and only backfillsIdentityCaseMapentries that user config hasn’t set, which matches the documented precedence and the tests inload_test.go. No changes needed.pkg/config/load_test.go (4)
6-7: Runtime import for OS-conditional tests is appropriate.Using
runtime.GOOSto skip the unreadable-file test on Windows is a straightforward way to keep the test portable.
1222-1291: User-vs-provisioned precedence flow is well specified.
TestUserConfigOverridesProvisionedIdentitiesaccurately captures the desired merge order (provisioned then reapply user config) and verifies both overridden and non-overridden identities, which should guard against regressions in precedence handling.
1293-1705: Provisioned identity case-preservation tests are thorough.The suite around
preserveProvisionedIdentityCaseandpreserveProviderIdentityCase(mixed case identities, user precedence, edge YAML structures, nil/empty maps, multiple providers, nil map initialization, unreadable files, and XDG cache dir failures) gives excellent coverage of the new behavior and matches the implementation inprovisioned_identity_case.go.
1793-1947: Helper tests for YAML extraction and provider-level case mapping are solid.The direct tests of
extractIdentitiesFromYAMLandpreserveProviderIdentityCasecover valid/malformed YAML, presence/absence ofauthandidentities, empty sections, and user-defined case precedence, which should make refactors to those helpers safe.pkg/auth/providers/aws/saml_test.go (1)
139-168: stubSamlMgr update correctly tracks the AuthManager interface.Adding
GetIdentityDisplayNamewith a pass-through implementation keeps the stub compiling against the expanded interface without changing test behavior.pkg/auth/list/formatter_table.go (1)
196-205: Identity table now respects original-case display names.Using
authManager.GetIdentityDisplayName(name)to populate the NAME column, with a nil-safe fallback toname, cleanly wires in the new case-preservation behavior without touching status or other columns.Also applies to: 247-250
pkg/auth/types/interfaces.go (1)
226-230: AuthManager interface extension for display names is reasonable.Adding
GetIdentityDisplayName(lowercaseKey string) stringcentralizes identity case-resolution behind a single method, which matches how the manager already ownsIdentityCaseMap. Implementations and mocks in this PR appear to have been updated accordingly.internal/exec/terraform_output_utils.go (1)
69-74: authContextWrapper’s GetIdentityDisplayName implementation is appropriate.Returning the key unchanged, with perf tracking, is a minimal and correct way to satisfy the extended AuthManager interface for this wrapper, which doesn’t own identity config or case maps.
Also applies to: 197-202
pkg/auth/hooks_test.go (1)
40-48: stubAuthManager update keeps hook tests aligned with AuthManager.The added
GetIdentityDisplayNamepassthrough matches the new interface contract without affecting existing hook test semantics.cmd/auth_console_test.go (1)
674-676: New GetIdentityDisplayName stubs look correct.Both mock implementations simply return the input name, which is a safe default and keeps these focused tests simple while satisfying the expanded AuthManager interface. No changes needed here.
Also applies to: 773-775
pkg/auth/types/mock_interfaces.go (1)
117-129: Generated gomock methods match the updated interfaces.The added
MockProvider.NameandMockAuthManager.GetIdentityDisplayNamemethods (and recorders) follow the same pattern as existing mocks and correctly mirror likely interface signatures. Regenerating viamockgenfor future interface changes will keep this in sync.Also applies to: 554-566
osterman
force-pushed
the
osterman/fix-identity-import
branch
from
42fbc93 to
e7ccd04
Compare
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
osterman
force-pushed
the
osterman/fix-identity-import
branch
from
23af48c to
6c4b2f5
Compare
Fix two related issues preventing provisioned identities from working: 1. Add injectProvisionedIdentitiesPostLoad() that runs after profiles are loaded, ensuring auth.providers (which may be defined in profiles) is available before checking for provisioned identity cache files. 2. Add processFileImportsIfPresent() to enable import: directives in profile files, which were previously silently ignored. Profile files are now processed for imports just like main config files. This allows auth.providers defined in profiles to properly auto-discover and load provisioned identities from the cache without manual import entries. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Re-apply user config and profiles after provisioned identities are injected, ensuring manually configured identities override auto-provisioned ones. This allows users to customize or override identities that were auto-provisioned from IAM Identity Center. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The Debug-level log messages for provisioned identity operations were appearing in test snapshots. These are internal implementation details that should be Trace-level to avoid cluttering normal debug output. Also fix rangeValCopy linter warning by accessing map values by key. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Respect AutoProvisionIdentities setting when loading cached identities Only load cached provisioned identities when the provider has auto_provision_identities: true enabled. This ensures cached identities are only used when the feature is currently enabled. - Use mergeConfig in reapplyUserConfigForPrecedence to process imports Changed from mergeConfigFile to mergeConfig so user config imports are also re-processed, ensuring user overrides in imported files take precedence over provisioned identities. - Fix import precedence test to reflect correct semantics Updated test to verify "importing file takes precedence over imported files" rather than the reverse. Moved import files to subdirectory to prevent double-loading via directory scan. - Update loadAtmosConfigsFromDirectory for correct precedence Process imports FIRST, then merge main file on top, aligning with mergeConfig() semantics. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Provisioned identity names were being displayed in lowercase (e.g., "core-artifacts/administratoraccess") instead of their original case (e.g., "core-artifacts/AdministratorAccess"). Root cause: The preserveIdentityCase function only read identity names from the main config file, not from provisioned identity cache files. When provisioned identities were loaded via Viper, their keys were automatically lowercased, and the original case was lost. Changes: - Add preserveProvisionedIdentityCase() function to read provisioned identity cache files and populate IdentityCaseMap - Respect user config precedence (user-defined case is not overwritten) - Add unit tests for case preservation - Add test cases for --identity flag case insensitivity 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The test for invalid YAML handling wasn't actually exercising the invalid YAML path because the provider didn't have auto_provision_identities enabled. Without this flag, the provider is skipped entirely and the invalid YAML is never parsed. Added auto_provision_identities: true to ensure the test actually exercises the code path it's designed to test. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… preservation Added table-driven tests for edge cases: - auto_provision_identities disabled - invalid YAML structure in cache file - missing auth section in cache file - missing identities section in cache file - non-existent cache file - empty identities section - nil AutoProvisionIdentities (treated as disabled) Also added tests for: - Multiple providers (enabled, disabled, no cache) - Nil IdentityCaseMap initialization 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive tests to achieve 100% coverage for provisioned_identity_case.go: - Extended TestPreserveProvisionedIdentityCase_EdgeCases with additional cases: - malformed_yaml_syntax: completely invalid YAML that fails parsing - auth_section_is_string: type assertion failure for auth section - identities_section_is_list: type assertion failure for identities - TestPreserveProvisionedIdentityCase_UnreadableFile: file permission errors - TestPreserveProvisionedIdentityCase_XDGCacheDirError: XDG directory errors - TestExtractIdentitiesFromYAML: direct function tests for all branches - TestPreserveProviderIdentityCase: direct function tests Coverage improved to 100% for all three functions: - preserveProvisionedIdentityCase: 100% - preserveProviderIdentityCase: 100% - extractIdentitiesFromYAML: 100% 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
File permissions work differently on Windows - setting 0o000 doesn't prevent reading files like it does on Unix systems. Skip the test on Windows since the behavior being tested is Unix-specific. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update the snapshot to include the new git root .atmos.d discovery trace messages that were added by the parent directory search feature. The snapshot now includes: - "Using test Git root override" trace messages - "Checking for .atmos.d in git root" trace messages - "Checking for .atmos.d in config directory" trace messages - path= suffixes on atmos.d loading failure messages 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Scan cache directory for all provider subdirectories instead of only processing providers with auto_provision_identities: true. This ensures identity case is preserved from cache files created by `auth login` regardless of current config flags. The IdentityCaseMap is only used for display purposes - this change does not affect how identities are loaded or merged into config. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The auth list command was showing provisioned identities in lowercase (e.g., "core-artifacts/terraformapplyaccess") instead of preserving the original case from AWS (e.g., "core-artifacts/TerraformApplyAccess"). This fix: - Adds GetIdentityDisplayName() method to AuthManager interface - Implements the method in manager.go to lookup original case from IdentityCaseMap - Updates formatter_tree.go and formatter_table.go to use display names - Adds test coverage for the display name functionality The IdentityCaseMap is populated during config loading from the provisioned identities cache file, which preserves the original case from AWS SSO. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update the "Zero-config AWS SSO identity management" milestone to include case preservation feature: identities now display with original case (e.g., "TerraformApplyAccess") while supporting case-insensitive resolution. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
osterman
force-pushed
the
osterman/fix-identity-import
branch
from
6c4b2f5 to
d960476
Compare
Updates three golden snapshot files to reflect log message changes: - New "Found atmos.yaml in current directory" trace message - Log level change from DEBUG to TRACE for "Found config ENV" - Additional "Preserved case-sensitive map keys" trace messages These changes are expected from the identity case preservation feature and rebase onto main. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
1 participant
what
injectProvisionedIdentitiesPostLoad()function that runs after profiles are loaded, ensuringauth.providers(which may be defined in profiles) is available before checking for provisioned identity cache filesprocessFileImportsIfPresent()helper to enableimport:directives in profile files (previously silently ignored)why
Provisioned identities were not auto-importing when
auth.providerswas defined in a profile because the provider check happened during early config loading, before profiles were merged. Additionally,import:directives inside profile files were completely ignored. These changes fix both issues, allowing proper auto-discovery of provisioned identities from cache files.references
Fixes provisioned identity auto-import for profiles with
auto_provision_identities: trueconfigured.Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.