Fix Atmos Auth for atmos describe and atmos list commands#1827
Conversation
|
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. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned Files
|
📝 WalkthroughWalkthroughAdds stack-level default identity discovery and merging into Atmos auth flow, threads an AuthManager through list/describe execution paths, introduces AtmosConfig-aware auth creation helpers, adds a stack auth loader, updates list CLI with an --identity flag, adds tests and fixtures, and bumps github.com/aws/smithy-go to v1.24.0. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant ListCmd as List Command
participant AuthLayer as Auth helper
participant StackLoader as Stack Auth Loader
participant StacksProc as Stacks Processor
User->>ListCmd: run `atmos list` [--identity ID]
ListCmd->>AuthLayer: getIdentityFromCommand(cmd) / ATMOS_IDENTITY
alt identity provided
ListCmd->>AuthLayer: CreateAuthManagerFromIdentityWithAtmosConfig(id, auth, atmosConfig)
else identity empty
AuthLayer->>StackLoader: LoadStackAuthDefaults(atmosConfig)
StackLoader-->>AuthLayer: stack defaults map
AuthLayer->>AuthLayer: merge stack defaults into atmosConfig.Auth
AuthLayer->>AuthLayer: CreateAuthManagerFromIdentityWithAtmosConfig("", mergedAuth, atmosConfig)
end
AuthLayer-->>ListCmd: authManager
ListCmd->>StacksProc: ExecuteDescribeStacks(..., authManager)
StacksProc-->>ListCmd: results
ListCmd-->>User: output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1827 +/- ##
==========================================
+ Coverage 72.07% 72.16% +0.09%
==========================================
Files 474 475 +1
Lines 45521 45713 +192
==========================================
+ Hits 32807 32988 +181
- Misses 10095 10104 +9
- Partials 2619 2621 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Add tests for checkAndMergeDefaultIdentity in workflow_utils.go - Add tests for scanAndMergeStackAuthDefaults in manager_helpers.go - Add tests for edge cases in stack_auth_scanner.go (read errors, invalid patterns) - Improve coverage for hasAnyDefault, applyStackDefaults, and other helper functions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai full review please |
|
@aknysh: I'll perform a comprehensive review of this PR. This looks like a substantial change addressing stack-level authentication identity resolution across multiple commands. I'll analyze the implementation of both the Component Auth Merge and Stack Scanning approaches, along with the related documentation and test fixtures. 🧠 Learnings used✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
docs/fixes/stack-level-default-auth-identity.md (1)
88-88: Minor: Add punctuation at end of paragraph.Per static analysis hint, add a period after "defaults".
- The `AuthManager` sees them when checking for defaults + The `AuthManager` sees them when checking for defaults.pkg/list/list_instances.go (2)
275-302: AuthManager plumbing through processInstances helpers looks good; minor doc cleanup.
- Passing
authManagerthroughprocessInstancesWithDepsintostacksProcessor.ExecuteDescribeStacksand threading it viaprocessInstancesis consistent and keeps the stacks processing testable via theStacksProcessorinterface.- The comment on Line 275 says “collects, filters, and sorts instances”, but the function now just collects and sorts; consider updating the comment to remove “filters” or clarify that filtering happens later in the upload path.
- If
authManageris allowed to benil(e.g., in tests), a brief comment or godoc note on the parameter could help future callers understand the expectation; otherwise, it’s fine to rely on a non‑nil contract.
304-342: ExecuteListInstancesCmd now depends on AuthManager; consider adding perf tracking.
- Wiring
authManagerintoprocessInstanceshere matches the new stack‑aware auth flow forlist instancesand keeps the auth concerns outside of this package’s core logic. That part looks solid.- Since
ExecuteListInstancesCmdis exported, it would be good to addperf.Trackper project convention. For example:-func ExecuteListInstancesCmd(info *schema.ConfigAndStacksInfo, cmd *cobra.Command, args []string, authManager auth.AuthManager) error { - // Inline initializeConfig. +func ExecuteListInstancesCmd(info *schema.ConfigAndStacksInfo, cmd *cobra.Command, args []string, authManager auth.AuthManager) error { + defer perf.Track(nil, "pkg/list.ExecuteListInstancesCmd")() + + // Inline initializeConfig.(You’ll need to ensure
perfis imported the same way it is in other instrumented files.)
📜 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 ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (28)
NOTICE(2 hunks)cmd/describe_affected.go(1 hunks)cmd/describe_component.go(2 hunks)cmd/describe_dependents.go(1 hunks)cmd/describe_stacks.go(1 hunks)cmd/identity_flag.go(1 hunks)cmd/list/instances.go(2 hunks)docs/fixes/stack-level-default-auth-identity.md(1 hunks)go.mod(1 hunks)internal/exec/mock_stacks_processor.go(2 hunks)internal/exec/stacks_processor.go(4 hunks)internal/exec/stacks_processor_test.go(1 hunks)internal/exec/terraform.go(1 hunks)internal/exec/terraform_nested_auth_helper.go(1 hunks)internal/exec/workflow_utils.go(2 hunks)internal/exec/workflow_utils_test.go(2 hunks)pkg/auth/manager_helpers.go(4 hunks)pkg/auth/manager_helpers_test.go(1 hunks)pkg/config/stack_auth_scanner.go(1 hunks)pkg/config/stack_auth_scanner_test.go(1 hunks)pkg/list/list_instances.go(4 hunks)pkg/list/list_instances_coverage_test.go(4 hunks)pkg/list/list_instances_process_test.go(6 hunks)tests/fixtures/scenarios/stack-auth-defaults/README.md(1 hunks)tests/fixtures/scenarios/stack-auth-defaults/atmos.yaml(1 hunks)tests/fixtures/scenarios/stack-auth-defaults/components/terraform/test-component/main.tf(1 hunks)tests/fixtures/scenarios/stack-auth-defaults/stacks/_defaults.yaml(1 hunks)tests/fixtures/scenarios/stack-auth-defaults/stacks/test.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
{go.mod,go.sum}
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Files:
go.mod
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use functional options pattern to avoid functions with many parameters. Define Option functions that modify a Config struct and pass variadic options to New functions.
Use context.Context for cancellation signals, deadlines/timeouts, and request-scoped values (trace IDs). Context should be first parameter in functions that accept it. DO NOT use context for configuration, dependencies, or avoiding proper function parameters.
All comments must end with periods. This is enforced by the godot linter.
NEVER delete existing comments without a very strong reason. Preserve helpful comments, update them to match code changes, refactor for clarity, and add context when modifying code. Only remove factually incorrect, duplicated, or outdated comments.
Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
All errors MUST be wrapped using static errors defined in errors/errors.go. Use errors.Join for combining multiple errors, fmt.Errorf with %w for adding string context, error builder for complex errors, and errors.Is() for error checking. NEVER use dynamic errors directly.
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation. Never create manual mocks.
Use viper.BindEnv with ATMOS_ prefix for environment variables.
Use colors from pkg/ui/theme/colors.go for theme consistency.
Ensure Linux/macOS/Windows compatibility. Use SDKs over binaries. Use filepath.Join(), not hardcoded path separators.
Small focused files (<600 lines). One cmd/impl per file. Co-locate tests. Never use //revive:disable:file-length-limit.
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider us...
Files:
cmd/identity_flag.gocmd/describe_affected.gointernal/exec/mock_stacks_processor.gointernal/exec/stacks_processor_test.gopkg/auth/manager_helpers.gointernal/exec/stacks_processor.gointernal/exec/workflow_utils_test.gopkg/list/list_instances_coverage_test.gopkg/config/stack_auth_scanner.gocmd/describe_stacks.gopkg/config/stack_auth_scanner_test.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_component.gointernal/exec/terraform.gopkg/list/list_instances.gocmd/list/instances.gopkg/auth/manager_helpers_test.gointernal/exec/workflow_utils.gocmd/describe_dependents.gopkg/list/list_instances_process_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cmd/**/*.go: Use unified flag parsing infrastructure in pkg/flags/. Commands MUST use flags.NewStandardParser() for command-specific flags. NEVER call viper.BindEnv() or viper.BindPFlag() directly outside pkg/flags/ (Forbidigo enforces this).
Telemetry is auto-enabled via RootCmd.ExecuteC(). Non-standard paths use telemetry.CaptureCmd(). Never capture user data.
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/identity_flag.gocmd/describe_affected.gocmd/describe_stacks.gocmd/describe_component.gocmd/list/instances.gocmd/describe_dependents.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: 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. Target >80% code coverage.
Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
**/*_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
Files:
internal/exec/stacks_processor_test.gointernal/exec/workflow_utils_test.gopkg/list/list_instances_coverage_test.gopkg/config/stack_auth_scanner_test.gopkg/auth/manager_helpers_test.gopkg/list/list_instances_process_test.go
🧠 Learnings (59)
📓 Common learnings
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.
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1533
File: pkg/config/load.go:585-637
Timestamp: 2025-09-27T20:50:20.564Z
Learning: In the cloudposse/atmos repository, command merging prioritizes precedence over display ordering. Help commands are displayed lexicographically regardless of internal array order, so the mergeCommandArrays function focuses on ensuring the correct precedence chain (top-level file wins) rather than maintaining specific display order.
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.
📚 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.mod,go.sum} : Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Applied to files:
go.modNOTICE
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: Go version 1.23.0 was deliberately introduced by the maintainer (aknysh) in January 2025. While this might be a pre-release or development version of Go, it has been approved for use in this project.
Applied to files:
go.modNOTICE
📚 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:
go.modinternal/exec/mock_stacks_processor.gopkg/auth/manager_helpers.gointernal/exec/stacks_processor.gocmd/describe_component.gointernal/exec/terraform.gopkg/list/list_instances.gocmd/list/instances.goNOTICEcmd/describe_dependents.go
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: The project uses Go version 1.23.0 which has been confirmed by the maintainer to be working in production for months. Do not flag this as an invalid Go version.
Applied to files:
go.mod
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/components/terraform/test-component/main.tftests/fixtures/scenarios/stack-auth-defaults/README.mdtests/fixtures/scenarios/stack-auth-defaults/stacks/_defaults.yamltests/fixtures/scenarios/stack-auth-defaults/atmos.yamltests/fixtures/scenarios/stack-auth-defaults/stacks/test.yaml
📚 Learning: 2025-09-07T18:07:00.549Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
Applied to files:
cmd/identity_flag.godocs/fixes/stack-level-default-auth-identity.mdcmd/describe_affected.gointernal/exec/mock_stacks_processor.gotests/fixtures/scenarios/stack-auth-defaults/README.mdtests/fixtures/scenarios/stack-auth-defaults/stacks/_defaults.yamlpkg/auth/manager_helpers.gopkg/config/stack_auth_scanner.gocmd/describe_stacks.gocmd/describe_component.gointernal/exec/terraform.gointernal/exec/workflow_utils.gocmd/describe_dependents.go
📚 Learning: 2025-09-25T01:02:48.697Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/manager.go:304-312
Timestamp: 2025-09-25T01:02:48.697Z
Learning: The auth manager in pkg/auth/manager.go should remain cloud-agnostic and not contain AWS-specific logic or references to specific cloud providers. Keep the manager generic and extensible.
Applied to files:
cmd/identity_flag.gocmd/describe_affected.gointernal/exec/mock_stacks_processor.gopkg/auth/manager_helpers.gocmd/describe_stacks.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_component.gointernal/exec/terraform.gopkg/list/list_instances.gocmd/list/instances.gopkg/auth/manager_helpers_test.gocmd/describe_dependents.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
cmd/identity_flag.gocmd/describe_affected.gocmd/describe_stacks.gointernal/exec/terraform_nested_auth_helper.gointernal/exec/terraform.go
📚 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:
cmd/identity_flag.godocs/fixes/stack-level-default-auth-identity.mdinternal/exec/terraform_nested_auth_helper.gointernal/exec/terraform.go
📚 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:
cmd/describe_affected.gointernal/exec/mock_stacks_processor.gointernal/exec/stacks_processor.gopkg/list/list_instances_coverage_test.gocmd/describe_stacks.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_component.gointernal/exec/terraform.gocmd/list/instances.gocmd/describe_dependents.gopkg/list/list_instances_process_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:
cmd/describe_affected.gopkg/auth/manager_helpers.gopkg/list/list_instances_coverage_test.gocmd/describe_stacks.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_component.gointernal/exec/terraform.gopkg/list/list_instances.gocmd/list/instances.gopkg/auth/manager_helpers_test.gointernal/exec/workflow_utils.gocmd/describe_dependents.go
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
cmd/describe_affected.gointernal/exec/terraform.go
📚 Learning: 2025-08-16T23:33:07.477Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:651-652
Timestamp: 2025-08-16T23:33:07.477Z
Learning: In the cloudposse/atmos Go codebase, ExecuteDescribeDependents expects a pointer to AtmosConfiguration (*schema.AtmosConfiguration), so when calling it with a value returned by cfg.InitCliConfig (which returns schema.AtmosConfiguration), the address-of operator (&) is necessary: ExecuteDescribeDependents(&atmosConfig, ...).
Applied to files:
cmd/describe_affected.gointernal/exec/mock_stacks_processor.gointernal/exec/stacks_processor.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_component.gocmd/list/instances.gocmd/describe_dependents.gopkg/list/list_instances_process_test.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:
cmd/describe_affected.gointernal/exec/mock_stacks_processor.gotests/fixtures/scenarios/stack-auth-defaults/README.mdpkg/auth/manager_helpers.gopkg/config/stack_auth_scanner.gocmd/describe_stacks.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_component.gointernal/exec/terraform.gopkg/list/list_instances.gocmd/list/instances.gopkg/auth/manager_helpers_test.gointernal/exec/workflow_utils.gocmd/describe_dependents.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
cmd/describe_affected.gointernal/exec/mock_stacks_processor.gointernal/exec/stacks_processor_test.gointernal/exec/stacks_processor.gopkg/config/stack_auth_scanner.gocmd/describe_stacks.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_dependents.gopkg/list/list_instances_process_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:
cmd/describe_affected.gocmd/describe_stacks.gocmd/describe_component.gopkg/list/list_instances.gocmd/list/instances.gocmd/describe_dependents.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Use interface-driven design with dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen using //go:generate directives.
Applied to files:
internal/exec/mock_stacks_processor.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:
internal/exec/mock_stacks_processor.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:
internal/exec/mock_stacks_processor.go
📚 Learning: 2025-01-25T03:44:52.619Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md:14-23
Timestamp: 2025-01-25T03:44:52.619Z
Learning: Test fixtures under `tests/fixtures/` should not be modified unless the test case itself needs to change, as they are deliberately set up to represent specific scenarios for testing purposes.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/README.mdtests/fixtures/scenarios/stack-auth-defaults/stacks/test.yaml
📚 Learning: 2025-03-25T12:24:36.177Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:263-264
Timestamp: 2025-03-25T12:24:36.177Z
Learning: Tests for the default Bitbucket username fallback to "x-token-auth" will be added during a future refactoring phase rather than in this PR.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/README.mdtests/fixtures/scenarios/stack-auth-defaults/stacks/_defaults.yamltests/fixtures/scenarios/stack-auth-defaults/atmos.yaml
📚 Learning: 2025-02-18T15:20:49.080Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1061
File: tests/fixtures/scenarios/vendor-pulls-ssh/vendor.yaml:20-22
Timestamp: 2025-02-18T15:20:49.080Z
Learning: Hardcoded credentials are acceptable in test fixtures when they are specifically testing credential handling, masking, or injection behavior. For example, in `tests/fixtures/scenarios/vendor-pulls-ssh/vendor.yaml`, credentials like "myuser:supersecret" are used to test that direct credentials in URLs are not overwritten by token injection.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/README.mdtests/fixtures/scenarios/stack-auth-defaults/atmos.yamltests/fixtures/scenarios/stack-auth-defaults/stacks/test.yaml
📚 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:
tests/fixtures/scenarios/stack-auth-defaults/README.mdpkg/auth/manager_helpers_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:
tests/fixtures/scenarios/stack-auth-defaults/README.mdpkg/auth/manager_helpers_test.go
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
internal/exec/stacks_processor_test.gopkg/list/list_instances_coverage_test.gocmd/describe_component.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:
internal/exec/stacks_processor_test.gointernal/exec/stacks_processor.gopkg/config/stack_auth_scanner.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-11-09T19:06:58.470Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1752
File: pkg/profile/list/formatter_table.go:27-29
Timestamp: 2025-11-09T19:06:58.470Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` is enforced on all functions via linting, including high-frequency utility functions, formatters, and renderers. This is a repository-wide policy to maintain consistency and avoid making case-by-case judgment calls about which functions should have profiling.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-11-30T04:16:24.023Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:34-48
Timestamp: 2025-11-30T04:16:24.023Z
Learning: In the cloudposse/atmos repository, the `defer perf.Track()` guideline applies to functions that perform meaningful work (I/O, computation, external calls), but explicitly excludes trivial accessors/mutators (e.g., simple getters, setters with single integer increments, string joins, or map appends) where the tracking overhead would exceed the actual method cost and provide no actionable performance data. Hot-path methods called in tight loops should especially avoid perf.Track() if they perform only trivial operations.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Applied to files:
pkg/auth/manager_helpers.gocmd/describe_component.gopkg/list/list_instances.goNOTICE
📚 Learning: 2025-11-30T04:16:01.879Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:50-59
Timestamp: 2025-11-30T04:16:01.879Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` should NOT be added to trivial O(1) getter methods that only return field references or check map lengths (e.g., `GetDeferredValues()`, `HasDeferredValues()`). The guideline to add perf tracking to "all public functions" applies to functions that do meaningful work (I/O, computation, external calls), not to trivial accessors where the tracking overhead would exceed the operation time and pollute performance reports.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-10-13T18:13:54.020Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1622
File: pkg/perf/perf.go:140-184
Timestamp: 2025-10-13T18:13:54.020Z
Learning: In pkg/perf/perf.go, the `trackWithSimpleStack` function intentionally skips ownership checks at call stack depth > 1 to avoid expensive `getGoroutineID()` calls on every nested function. This is a performance optimization for the common single-goroutine execution case (most Atmos commands), accepting the rare edge case of potential metric corruption if multi-goroutine execution occurs at depth > 1. The ~19× performance improvement justifies this trade-off.
Applied to files:
pkg/auth/manager_helpers.gointernal/exec/stacks_processor.gopkg/config/stack_auth_scanner.gopkg/list/list_instances_process_test.go
📚 Learning: 2025-05-30T03:21:37.197Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1274
File: go.mod:63-63
Timestamp: 2025-05-30T03:21:37.197Z
Learning: The redis dependency (github.com/redis/go-redis/v9) in the atmos project is only used in tests, not in production code.
Applied to files:
pkg/auth/manager_helpers.gopkg/list/list_instances.goNOTICE
📚 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:
internal/exec/stacks_processor.gopkg/list/list_instances_coverage_test.gointernal/exec/terraform_nested_auth_helper.gopkg/list/list_instances_process_test.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:
internal/exec/stacks_processor.gotests/fixtures/scenarios/stack-auth-defaults/atmos.yamlNOTICE
📚 Learning: 2024-10-20T00:57:53.500Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:0-0
Timestamp: 2024-10-20T00:57:53.500Z
Learning: In `internal/exec/validate_stacks.go`, when downloading the Atmos JSON Schema file to the temp directory, the temporary file is overwritten each time, so explicit removal is not necessary.
Applied to files:
internal/exec/stacks_processor.go
📚 Learning: 2025-04-11T22:06:46.999Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
internal/exec/stacks_processor.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:
internal/exec/workflow_utils_test.gopkg/config/stack_auth_scanner_test.gopkg/auth/manager_helpers_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:
internal/exec/workflow_utils_test.gopkg/config/stack_auth_scanner_test.gopkg/auth/manager_helpers_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:
internal/exec/workflow_utils_test.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
internal/exec/workflow_utils_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*_test.go : Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
Applied to files:
internal/exec/workflow_utils_test.gopkg/config/stack_auth_scanner_test.gopkg/auth/manager_helpers_test.go
📚 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/fixtures/scenarios/stack-auth-defaults/atmos.yaml
📚 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/fixtures/scenarios/stack-auth-defaults/atmos.yaml
📚 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:
tests/fixtures/scenarios/stack-auth-defaults/atmos.yaml
📚 Learning: 2025-01-25T03:51:57.689Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/atmos.yaml
📚 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/stack_auth_scanner_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*_test.go : 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. Target >80% code coverage.
Applied to files:
pkg/config/stack_auth_scanner_test.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
cmd/describe_component.gopkg/list/list_instances.go
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
cmd/describe_component.gopkg/list/list_instances.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
pkg/list/list_instances.go
📚 Learning: 2024-12-05T22:33:40.955Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Applied to files:
pkg/list/list_instances.go
📚 Learning: 2025-11-10T03:03:31.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-11-10T03:03:31.505Z
Learning: In the Atmos codebase, commands using the `StandardParser` flag pattern (from pkg/flags) do NOT need explicit `viper.BindPFlag()` calls in their code. The StandardParser encapsulates flag binding internally: flags are registered via `parser.RegisterFlags(cmd)` in init(), and bound via `parser.BindFlagsToViper(cmd, v)` in RunE, which internally calls viper.BindPFlag for each flag. This pattern is used throughout Atmos (e.g., cmd/toolchain/get.go, cmd/toolchain/info.go, cmd/toolchain/install.go, cmd/toolchain/path.go). Do not flag missing viper.BindPFlag calls when StandardParser is used.
Applied to files:
cmd/list/instances.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 Viper for managing configuration, environment variables, and flags in CLI commands
Applied to files:
cmd/list/instances.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to cmd/**/*.go : Use unified flag parsing infrastructure in pkg/flags/. Commands MUST use flags.NewStandardParser() for command-specific flags. NEVER call viper.BindEnv() or viper.BindPFlag() directly outside pkg/flags/ (Forbidigo enforces this).
Applied to files:
cmd/list/instances.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Use viper.BindEnv with ATMOS_ prefix for environment variables.
Applied to files:
cmd/list/instances.go
📚 Learning: 2025-11-01T20:24:29.557Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1714
File: NOTICE:0-0
Timestamp: 2025-11-01T20:24:29.557Z
Learning: In the cloudposse/atmos repository, the NOTICE file is programmatically generated and should not be manually edited. Issues with dependency license URLs in NOTICE will be resolved when upstream package metadata is corrected.
Applied to files:
NOTICE
📚 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:
NOTICE
🧬 Code graph analysis (16)
cmd/describe_affected.go (1)
cmd/identity_flag.go (2)
GetIdentityFromFlags(35-59)CreateAuthManagerFromIdentityWithAtmosConfig(155-161)
internal/exec/mock_stacks_processor.go (1)
internal/exec/describe_stacks.go (1)
ExecuteDescribeStacks(116-932)
internal/exec/workflow_utils_test.go (2)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/schema/schema_auth.go (1)
AuthConfig(4-12)
pkg/list/list_instances_coverage_test.go (1)
pkg/list/list_instances.go (1)
ExecuteListInstancesCmd(305-342)
pkg/config/stack_auth_scanner.go (2)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/utils/glob_utils.go (1)
GetGlobMatches(36-65)
cmd/describe_stacks.go (1)
cmd/identity_flag.go (2)
GetIdentityFromFlags(35-59)CreateAuthManagerFromIdentityWithAtmosConfig(155-161)
pkg/config/stack_auth_scanner_test.go (2)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/config/stack_auth_scanner.go (2)
ScanStackAuthDefaults(41-75)MergeStackAuthDefaults(157-172)
internal/exec/terraform_nested_auth_helper.go (2)
pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManagerWithAtmosConfig(218-269)pkg/config/const.go (1)
IdentityFlagSelectValue(136-136)
cmd/describe_component.go (6)
cmd/identity_flag.go (2)
GetIdentityFromFlags(35-59)CreateAuthManagerFromIdentity(136-141)pkg/auth/config_helpers.go (2)
CopyGlobalAuthConfig(16-57)MergeComponentAuthFromConfig(138-160)internal/exec/describe_component.go (2)
ExecuteDescribeComponent(212-228)ExecuteDescribeComponentParams(202-209)pkg/describe/describe_component.go (1)
ExecuteDescribeComponent(9-26)errors/errors.go (1)
ErrInvalidComponent(203-203)pkg/config/const.go (1)
AuthSectionName(84-84)
internal/exec/terraform.go (3)
pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManagerWithAtmosConfig(218-269)pkg/config/const.go (1)
IdentityFlagSelectValue(136-136)cmd/auth.go (1)
IdentityFlagSelectValue(14-14)
pkg/list/list_instances.go (2)
pkg/schema/schema.go (1)
ConfigAndStacksInfo(621-715)internal/exec/stacks_processor.go (1)
DefaultStacksProcessor(29-29)
cmd/list/instances.go (4)
pkg/config/config.go (1)
InitCliConfig(28-67)pkg/schema/schema.go (1)
ConfigAndStacksInfo(621-715)pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManagerWithAtmosConfig(218-269)pkg/list/list_instances.go (1)
ExecuteListInstancesCmd(305-342)
pkg/auth/manager_helpers_test.go (4)
pkg/schema/schema_auth.go (1)
AuthConfig(4-12)pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManagerWithAtmosConfig(218-269)pkg/schema/schema.go (1)
AtmosConfiguration(53-94)errors/errors.go (1)
ErrAuthNotConfigured(489-489)
internal/exec/workflow_utils.go (2)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/config/stack_auth_scanner.go (2)
ScanStackAuthDefaults(41-75)MergeStackAuthDefaults(157-172)
cmd/describe_dependents.go (1)
cmd/identity_flag.go (2)
GetIdentityFromFlags(35-59)CreateAuthManagerFromIdentityWithAtmosConfig(155-161)
pkg/list/list_instances_process_test.go (1)
internal/exec/describe_stacks.go (1)
ExecuteDescribeStacks(116-932)
🪛 GitHub Actions: Dependency Review
NOTICE
[error] 1-1: NOTICE file is out of date. Run './scripts/generate-notice.sh' locally and commit the changes.
🪛 LanguageTool
docs/fixes/stack-level-default-auth-identity.md
[grammar] ~88-~88: Please add a punctuation mark at the end of paragraph.
Context: ...uthManager` sees them when checking for defaults ### Why Stack Config Didn't Work For ...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~166-~166: To join two clauses or introduce examples, consider using an em dash.
Context: ...omConfig-internal/exec/terraform.go` - Original implementation of this pattern ...
(DASH_RULE)
[typographical] ~196-~196: To join two clauses or introduce examples, consider using an em dash.
Context: ...terraform *` (all terraform subcommands) - Has specific component+stack ### CLI Co...
(DASH_RULE)
tests/fixtures/scenarios/stack-auth-defaults/README.md
[uncategorized] ~17-~17: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...s to find auth identity defaults BEFORE full stack processing. This allows stack-level def...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[typographical] ~43-~43: To join two clauses or introduce examples, consider using an em dash.
Context: ...s/fixes/stack-level-default-identity.md- Full issue documentation -pkg/config/s...
(DASH_RULE)
🪛 markdownlint-cli2 (0.18.1)
tests/fixtures/scenarios/stack-auth-defaults/README.md
22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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
The identity flag check (cmd.Flags().Changed("identity")) was dead code
because --identity is not registered in instancesParser. The code works
correctly via stack-level default resolution when identityName is empty.
Addresses CodeRabbit review feedback on PR #1827.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
internal/exec/workflow_utils_test.go (1)
3-12: Nice behavioral coverage for default‑identity resolution and stack scanning.The new tests around
checkAndMergeDefaultIdentitycover the key paths (no identities, non‑default, atmos‑level default, multiple identities, stack‑scanned defaults, and scan errors falling back to atmos.yaml). The temp stack setup usingfilepath.Join/os.WriteFileis straightforward and portable, and import grouping still follows the project’s conventions. This is in good shape.Also applies to: 143-275
tests/fixtures/scenarios/stack-auth-defaults/README.md (1)
22-32: Add language specifier to fenced code block.The directory structure block should have a language identifier for consistency with markdown linting rules.
-``` +```text stack-auth-defaults/ ├── atmos.yaml # Auth config with identity (NO default)pkg/auth/manager_helpers_test.go (1)
999-1028: Usefilepath.Joinfor cross-platform compatibility.String concatenation for paths may cause issues on Windows. Per coding guidelines, use
filepath.Join()instead of hardcoded path separators.+import ( + "path/filepath" + ... +) + func TestScanAndMergeStackAuthDefaults_WithStackFiles(t *testing.T) { // Create a temporary directory with stack files. tmpDir := t.TempDir() // Create a stack file with default identity. stackContent := `auth: identities: stack-identity: default: true ` - err := os.WriteFile(tmpDir+"/stack.yaml", []byte(stackContent), 0o644) + err := os.WriteFile(filepath.Join(tmpDir, "stack.yaml"), []byte(stackContent), 0o644) require.NoError(t, err) authConfig := &schema.AuthConfig{ Identities: map[string]schema.Identity{ "stack-identity": {Kind: "aws/assume-role", Default: false}, }, } atmosConfig := &schema.AtmosConfiguration{ BasePath: tmpDir, - IncludeStackAbsolutePaths: []string{tmpDir + "/*.yaml"}, + IncludeStackAbsolutePaths: []string{filepath.Join(tmpDir, "*.yaml")}, }internal/exec/mock_stacks_processor.go (1)
12-18: Import grouping could be adjusted.Per coding guidelines, imports should be grouped: stdlib, 3rd-party, then Atmos packages. Currently
reflect(stdlib) is followed by Atmos packages, thengomock(3rd-party).However, this is generated code via MockGen, so regenerating will restore this order.
import ( reflect "reflect" + gomock "go.uber.org/mock/gomock" + auth "github.com/cloudposse/atmos/pkg/auth" schema "github.com/cloudposse/atmos/pkg/schema" - gomock "go.uber.org/mock/gomock" )pkg/list/list_instances.go (1)
304-342: Entry point properly accepts and propagates authManager.The command executor correctly threads the auth manager to enable stack-level defaults. The caller (
cmd/list/instances.go) handles creating the AuthManager viaCreateAndAuthenticateManagerWithAtmosConfig.Note: Consider adding
defer perf.Track(nil, "list.ExecuteListInstancesCmd")()at the function start per coding guidelines, though this may be tracked at the cmd layer.
📜 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 ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (28)
NOTICE(2 hunks)cmd/describe_affected.go(1 hunks)cmd/describe_component.go(2 hunks)cmd/describe_dependents.go(1 hunks)cmd/describe_stacks.go(1 hunks)cmd/identity_flag.go(1 hunks)cmd/list/instances.go(2 hunks)docs/fixes/stack-level-default-auth-identity.md(1 hunks)go.mod(1 hunks)internal/exec/mock_stacks_processor.go(2 hunks)internal/exec/stacks_processor.go(4 hunks)internal/exec/stacks_processor_test.go(1 hunks)internal/exec/terraform.go(1 hunks)internal/exec/terraform_nested_auth_helper.go(1 hunks)internal/exec/workflow_utils.go(2 hunks)internal/exec/workflow_utils_test.go(2 hunks)pkg/auth/manager_helpers.go(4 hunks)pkg/auth/manager_helpers_test.go(1 hunks)pkg/config/stack_auth_scanner.go(1 hunks)pkg/config/stack_auth_scanner_test.go(1 hunks)pkg/list/list_instances.go(4 hunks)pkg/list/list_instances_coverage_test.go(4 hunks)pkg/list/list_instances_process_test.go(6 hunks)tests/fixtures/scenarios/stack-auth-defaults/README.md(1 hunks)tests/fixtures/scenarios/stack-auth-defaults/atmos.yaml(1 hunks)tests/fixtures/scenarios/stack-auth-defaults/components/terraform/test-component/main.tf(1 hunks)tests/fixtures/scenarios/stack-auth-defaults/stacks/_defaults.yaml(1 hunks)tests/fixtures/scenarios/stack-auth-defaults/stacks/test.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use functional options pattern to avoid functions with many parameters. Define Option functions that modify a Config struct and pass variadic options to New functions.
Use context.Context for cancellation signals, deadlines/timeouts, and request-scoped values (trace IDs). Context should be first parameter in functions that accept it. DO NOT use context for configuration, dependencies, or avoiding proper function parameters.
All comments must end with periods. This is enforced by the godot linter.
NEVER delete existing comments without a very strong reason. Preserve helpful comments, update them to match code changes, refactor for clarity, and add context when modifying code. Only remove factually incorrect, duplicated, or outdated comments.
Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
All errors MUST be wrapped using static errors defined in errors/errors.go. Use errors.Join for combining multiple errors, fmt.Errorf with %w for adding string context, error builder for complex errors, and errors.Is() for error checking. NEVER use dynamic errors directly.
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation. Never create manual mocks.
Use viper.BindEnv with ATMOS_ prefix for environment variables.
Use colors from pkg/ui/theme/colors.go for theme consistency.
Ensure Linux/macOS/Windows compatibility. Use SDKs over binaries. Use filepath.Join(), not hardcoded path separators.
Small focused files (<600 lines). One cmd/impl per file. Co-locate tests. Never use //revive:disable:file-length-limit.
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider us...
Files:
internal/exec/stacks_processor_test.gointernal/exec/stacks_processor.gocmd/list/instances.gopkg/config/stack_auth_scanner.gointernal/exec/workflow_utils.gopkg/list/list_instances_coverage_test.gopkg/auth/manager_helpers.gointernal/exec/mock_stacks_processor.gopkg/list/list_instances_process_test.gocmd/identity_flag.gocmd/describe_affected.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_stacks.gointernal/exec/terraform.gointernal/exec/workflow_utils_test.gopkg/config/stack_auth_scanner_test.gocmd/describe_component.gopkg/list/list_instances.gopkg/auth/manager_helpers_test.gocmd/describe_dependents.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: 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. Target >80% code coverage.
Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
**/*_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
Files:
internal/exec/stacks_processor_test.gopkg/list/list_instances_coverage_test.gopkg/list/list_instances_process_test.gointernal/exec/workflow_utils_test.gopkg/config/stack_auth_scanner_test.gopkg/auth/manager_helpers_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cmd/**/*.go: Use unified flag parsing infrastructure in pkg/flags/. Commands MUST use flags.NewStandardParser() for command-specific flags. NEVER call viper.BindEnv() or viper.BindPFlag() directly outside pkg/flags/ (Forbidigo enforces this).
Telemetry is auto-enabled via RootCmd.ExecuteC(). Non-standard paths use telemetry.CaptureCmd(). Never capture user data.
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/list/instances.gocmd/identity_flag.gocmd/describe_affected.gocmd/describe_stacks.gocmd/describe_component.gocmd/describe_dependents.go
{go.mod,go.sum}
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Files:
go.mod
🧠 Learnings (58)
📓 Common learnings
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.
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
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.
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:263-264
Timestamp: 2025-03-25T12:24:36.177Z
Learning: Tests for the default Bitbucket username fallback to "x-token-auth" will be added during a future refactoring phase rather than in this PR.
📚 Learning: 2025-02-18T15:20:49.080Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1061
File: tests/fixtures/scenarios/vendor-pulls-ssh/vendor.yaml:20-22
Timestamp: 2025-02-18T15:20:49.080Z
Learning: Hardcoded credentials are acceptable in test fixtures when they are specifically testing credential handling, masking, or injection behavior. For example, in `tests/fixtures/scenarios/vendor-pulls-ssh/vendor.yaml`, credentials like "myuser:supersecret" are used to test that direct credentials in URLs are not overwritten by token injection.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/README.mdtests/fixtures/scenarios/stack-auth-defaults/atmos.yaml
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/README.mdtests/fixtures/scenarios/stack-auth-defaults/stacks/test.yamltests/fixtures/scenarios/stack-auth-defaults/components/terraform/test-component/main.tftests/fixtures/scenarios/stack-auth-defaults/stacks/_defaults.yamltests/fixtures/scenarios/stack-auth-defaults/atmos.yaml
📚 Learning: 2025-09-07T18:07:00.549Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/README.mddocs/fixes/stack-level-default-auth-identity.mdpkg/config/stack_auth_scanner.gointernal/exec/workflow_utils.gopkg/auth/manager_helpers.gointernal/exec/mock_stacks_processor.gocmd/identity_flag.gocmd/describe_affected.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_stacks.gointernal/exec/terraform.gocmd/describe_component.gotests/fixtures/scenarios/stack-auth-defaults/stacks/_defaults.yamlcmd/describe_dependents.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:
tests/fixtures/scenarios/stack-auth-defaults/README.mdinternal/exec/workflow_utils_test.gopkg/auth/manager_helpers_test.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:
tests/fixtures/scenarios/stack-auth-defaults/README.mdcmd/list/instances.gopkg/config/stack_auth_scanner.gointernal/exec/workflow_utils.gopkg/auth/manager_helpers.gointernal/exec/mock_stacks_processor.gocmd/describe_affected.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_stacks.gointernal/exec/terraform.gocmd/describe_component.gopkg/list/list_instances.gopkg/auth/manager_helpers_test.gocmd/describe_dependents.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:
tests/fixtures/scenarios/stack-auth-defaults/README.mdpkg/auth/manager_helpers_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.mod,go.sum} : Manage dependencies with Go modules and keep dependencies up to date while minimizing external dependencies
Applied to files:
NOTICEgo.mod
📚 Learning: 2025-11-01T20:24:29.557Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1714
File: NOTICE:0-0
Timestamp: 2025-11-01T20:24:29.557Z
Learning: In the cloudposse/atmos repository, the NOTICE file is programmatically generated and should not be manually edited. Issues with dependency license URLs in NOTICE will be resolved when upstream package metadata is corrected.
Applied to files:
NOTICE
📚 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:
NOTICE
📚 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:
NOTICEcmd/list/instances.gopkg/auth/manager_helpers.gointernal/exec/mock_stacks_processor.gocmd/describe_affected.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_stacks.gocmd/describe_component.gopkg/list/list_instances.gocmd/describe_dependents.gogo.mod
📚 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:
NOTICE
📚 Learning: 2025-05-30T03:21:37.197Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1274
File: go.mod:63-63
Timestamp: 2025-05-30T03:21:37.197Z
Learning: The redis dependency (github.com/redis/go-redis/v9) in the atmos project is only used in tests, not in production code.
Applied to files:
NOTICEpkg/auth/manager_helpers.gopkg/list/list_instances.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Applied to files:
NOTICEpkg/auth/manager_helpers.gocmd/describe_component.gopkg/list/list_instances.go
📚 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:
docs/fixes/stack-level-default-auth-identity.mdcmd/identity_flag.gointernal/exec/terraform_nested_auth_helper.gointernal/exec/terraform.gocmd/describe_dependents.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
internal/exec/stacks_processor_test.gointernal/exec/stacks_processor.gopkg/config/stack_auth_scanner.gointernal/exec/mock_stacks_processor.gopkg/list/list_instances_process_test.gocmd/describe_stacks.gointernal/exec/terraform.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:
internal/exec/stacks_processor_test.gointernal/exec/stacks_processor.gopkg/config/stack_auth_scanner_test.go
📚 Learning: 2025-09-25T01:02:48.697Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/manager.go:304-312
Timestamp: 2025-09-25T01:02:48.697Z
Learning: The auth manager in pkg/auth/manager.go should remain cloud-agnostic and not contain AWS-specific logic or references to specific cloud providers. Keep the manager generic and extensible.
Applied to files:
internal/exec/stacks_processor.gocmd/list/instances.gopkg/auth/manager_helpers.gointernal/exec/mock_stacks_processor.gocmd/identity_flag.gocmd/describe_affected.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_stacks.gointernal/exec/terraform.gocmd/describe_component.gopkg/list/list_instances.gopkg/auth/manager_helpers_test.gocmd/describe_dependents.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:
internal/exec/stacks_processor.gopkg/list/list_instances_coverage_test.gopkg/auth/manager_helpers.gopkg/list/list_instances_process_test.gocmd/describe_stacks.gointernal/exec/terraform.go
📚 Learning: 2025-10-13T18:13:54.020Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1622
File: pkg/perf/perf.go:140-184
Timestamp: 2025-10-13T18:13:54.020Z
Learning: In pkg/perf/perf.go, the `trackWithSimpleStack` function intentionally skips ownership checks at call stack depth > 1 to avoid expensive `getGoroutineID()` calls on every nested function. This is a performance optimization for the common single-goroutine execution case (most Atmos commands), accepting the rare edge case of potential metric corruption if multi-goroutine execution occurs at depth > 1. The ~19× performance improvement justifies this trade-off.
Applied to files:
internal/exec/stacks_processor.gopkg/config/stack_auth_scanner.gopkg/auth/manager_helpers.gopkg/list/list_instances_process_test.gocmd/describe_stacks.go
📚 Learning: 2024-10-20T00:57:53.500Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:0-0
Timestamp: 2024-10-20T00:57:53.500Z
Learning: In `internal/exec/validate_stacks.go`, when downloading the Atmos JSON Schema file to the temp directory, the temporary file is overwritten each time, so explicit removal is not necessary.
Applied to files:
internal/exec/stacks_processor.go
📚 Learning: 2025-04-11T22:06:46.999Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
internal/exec/stacks_processor.go
📚 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:
internal/exec/stacks_processor.gocmd/list/instances.gopkg/list/list_instances_coverage_test.gointernal/exec/mock_stacks_processor.gopkg/list/list_instances_process_test.gocmd/describe_affected.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_stacks.gointernal/exec/terraform.gocmd/describe_component.gocmd/describe_dependents.go
📚 Learning: 2025-08-16T23:33:07.477Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:651-652
Timestamp: 2025-08-16T23:33:07.477Z
Learning: In the cloudposse/atmos Go codebase, ExecuteDescribeDependents expects a pointer to AtmosConfiguration (*schema.AtmosConfiguration), so when calling it with a value returned by cfg.InitCliConfig (which returns schema.AtmosConfiguration), the address-of operator (&) is necessary: ExecuteDescribeDependents(&atmosConfig, ...).
Applied to files:
internal/exec/stacks_processor.gocmd/list/instances.gointernal/exec/mock_stacks_processor.gopkg/list/list_instances_process_test.gocmd/describe_affected.gocmd/describe_component.gocmd/describe_dependents.go
📚 Learning: 2025-11-10T03:03:31.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-11-10T03:03:31.505Z
Learning: In the Atmos codebase, commands using the `StandardParser` flag pattern (from pkg/flags) do NOT need explicit `viper.BindPFlag()` calls in their code. The StandardParser encapsulates flag binding internally: flags are registered via `parser.RegisterFlags(cmd)` in init(), and bound via `parser.BindFlagsToViper(cmd, v)` in RunE, which internally calls viper.BindPFlag for each flag. This pattern is used throughout Atmos (e.g., cmd/toolchain/get.go, cmd/toolchain/info.go, cmd/toolchain/install.go, cmd/toolchain/path.go). Do not flag missing viper.BindPFlag calls when StandardParser is used.
Applied to files:
cmd/list/instances.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 Viper for managing configuration, environment variables, and flags in CLI commands
Applied to files:
cmd/list/instances.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:
cmd/list/instances.gocmd/describe_affected.gocmd/describe_stacks.gocmd/describe_component.gopkg/list/list_instances.gocmd/describe_dependents.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to cmd/**/*.go : Use unified flag parsing infrastructure in pkg/flags/. Commands MUST use flags.NewStandardParser() for command-specific flags. NEVER call viper.BindEnv() or viper.BindPFlag() directly outside pkg/flags/ (Forbidigo enforces this).
Applied to files:
cmd/list/instances.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Use viper.BindEnv with ATMOS_ prefix for environment variables.
Applied to files:
cmd/list/instances.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:
cmd/list/instances.gopkg/config/stack_auth_scanner.gointernal/exec/workflow_utils.gopkg/list/list_instances_coverage_test.gopkg/auth/manager_helpers.gocmd/identity_flag.gocmd/describe_affected.gointernal/exec/terraform_nested_auth_helper.gocmd/describe_stacks.gointernal/exec/terraform.gocmd/describe_component.gopkg/list/list_instances.gocmd/describe_dependents.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
internal/exec/workflow_utils.gointernal/exec/workflow_utils_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/list/list_instances_coverage_test.gointernal/exec/workflow_utils_test.gopkg/config/stack_auth_scanner_test.gopkg/auth/manager_helpers_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/auth/manager_helpers.gocmd/describe_dependents.go
📚 Learning: 2025-11-09T19:06:58.470Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1752
File: pkg/profile/list/formatter_table.go:27-29
Timestamp: 2025-11-09T19:06:58.470Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` is enforced on all functions via linting, including high-frequency utility functions, formatters, and renderers. This is a repository-wide policy to maintain consistency and avoid making case-by-case judgment calls about which functions should have profiling.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-11-30T04:16:24.023Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:34-48
Timestamp: 2025-11-30T04:16:24.023Z
Learning: In the cloudposse/atmos repository, the `defer perf.Track()` guideline applies to functions that perform meaningful work (I/O, computation, external calls), but explicitly excludes trivial accessors/mutators (e.g., simple getters, setters with single integer increments, string joins, or map appends) where the tracking overhead would exceed the actual method cost and provide no actionable performance data. Hot-path methods called in tight loops should especially avoid perf.Track() if they perform only trivial operations.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-11-30T04:16:01.879Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:50-59
Timestamp: 2025-11-30T04:16:01.879Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` should NOT be added to trivial O(1) getter methods that only return field references or check map lengths (e.g., `GetDeferredValues()`, `HasDeferredValues()`). The guideline to add perf tracking to "all public functions" applies to functions that do meaningful work (I/O, computation, external calls), not to trivial accessors where the tracking overhead would exceed the operation time and pollute performance reports.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Use interface-driven design with dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen using //go:generate directives.
Applied to files:
internal/exec/mock_stacks_processor.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:
internal/exec/mock_stacks_processor.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:
internal/exec/mock_stacks_processor.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
cmd/identity_flag.gocmd/describe_affected.gointernal/exec/terraform.go
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
cmd/describe_affected.gointernal/exec/terraform_nested_auth_helper.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
internal/exec/terraform_nested_auth_helper.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*_test.go : Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
Applied to files:
internal/exec/workflow_utils_test.gopkg/config/stack_auth_scanner_test.gopkg/auth/manager_helpers_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/stack_auth_scanner_test.gopkg/auth/manager_helpers_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/stack_auth_scanner_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*_test.go : 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. Target >80% code coverage.
Applied to files:
pkg/config/stack_auth_scanner_test.go
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
cmd/describe_component.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
cmd/describe_component.gopkg/list/list_instances.go
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
cmd/describe_component.gopkg/list/list_instances.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
pkg/list/list_instances.go
📚 Learning: 2024-12-05T22:33:40.955Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Applied to files:
pkg/list/list_instances.go
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: Go version 1.23.0 was deliberately introduced by the maintainer (aknysh) in January 2025. While this might be a pre-release or development version of Go, it has been approved for use in this project.
Applied to files:
go.mod
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: The project uses Go version 1.23.0 which has been confirmed by the maintainer to be working in production for months. Do not flag this as an invalid Go version.
Applied to files:
go.mod
📚 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/fixtures/scenarios/stack-auth-defaults/atmos.yaml
📚 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/fixtures/scenarios/stack-auth-defaults/atmos.yaml
📚 Learning: 2025-03-25T12:24:36.177Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:263-264
Timestamp: 2025-03-25T12:24:36.177Z
Learning: Tests for the default Bitbucket username fallback to "x-token-auth" will be added during a future refactoring phase rather than in this PR.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/atmos.yaml
📚 Learning: 2025-01-25T03:44:52.619Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md:14-23
Timestamp: 2025-01-25T03:44:52.619Z
Learning: Test fixtures under `tests/fixtures/` should not be modified unless the test case itself needs to change, as they are deliberately set up to represent specific scenarios for testing purposes.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/atmos.yaml
🧬 Code graph analysis (12)
internal/exec/workflow_utils.go (2)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/config/stack_auth_scanner.go (2)
ScanStackAuthDefaults(41-75)MergeStackAuthDefaults(157-172)
pkg/list/list_instances_coverage_test.go (1)
pkg/list/list_instances.go (1)
ExecuteListInstancesCmd(305-342)
internal/exec/mock_stacks_processor.go (2)
internal/exec/describe_stacks.go (1)
ExecuteDescribeStacks(116-932)pkg/schema/schema.go (1)
AtmosConfiguration(53-94)
pkg/list/list_instances_process_test.go (1)
internal/exec/describe_stacks.go (1)
ExecuteDescribeStacks(116-932)
cmd/describe_affected.go (1)
cmd/identity_flag.go (2)
GetIdentityFromFlags(35-59)CreateAuthManagerFromIdentityWithAtmosConfig(155-161)
internal/exec/terraform_nested_auth_helper.go (2)
pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManagerWithAtmosConfig(218-269)pkg/config/const.go (1)
IdentityFlagSelectValue(136-136)
cmd/describe_stacks.go (1)
cmd/identity_flag.go (2)
GetIdentityFromFlags(35-59)CreateAuthManagerFromIdentityWithAtmosConfig(155-161)
internal/exec/workflow_utils_test.go (2)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/schema/schema_auth.go (1)
AuthConfig(4-12)
pkg/config/stack_auth_scanner_test.go (2)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/schema/schema_auth.go (1)
AuthConfig(4-12)
pkg/list/list_instances.go (2)
pkg/schema/schema.go (1)
ConfigAndStacksInfo(621-715)internal/exec/stacks_processor.go (1)
DefaultStacksProcessor(29-29)
pkg/auth/manager_helpers_test.go (4)
pkg/schema/schema_auth.go (3)
AuthConfig(4-12)IdentityVia(62-65)Principal(69-72)pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManagerWithAtmosConfig(218-269)pkg/schema/schema.go (1)
AtmosConfiguration(53-94)errors/errors.go (1)
ErrAuthNotConfigured(489-489)
cmd/describe_dependents.go (1)
cmd/identity_flag.go (2)
GetIdentityFromFlags(35-59)CreateAuthManagerFromIdentityWithAtmosConfig(155-161)
🪛 GitHub Actions: Dependency Review
NOTICE
[error] 1-1: NOTICE file is out of date. Run './scripts/generate-notice.sh' locally and commit the changes.
🪛 LanguageTool
tests/fixtures/scenarios/stack-auth-defaults/README.md
[uncategorized] ~17-~17: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...s to find auth identity defaults BEFORE full stack processing. This allows stack-level def...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[typographical] ~43-~43: To join two clauses or introduce examples, consider using an em dash.
Context: ...s/fixes/stack-level-default-identity.md- Full issue documentation -pkg/config/s...
(DASH_RULE)
docs/fixes/stack-level-default-auth-identity.md
[grammar] ~88-~88: Please add a punctuation mark at the end of paragraph.
Context: ...uthManager` sees them when checking for defaults ### Why Stack Config Didn't Work For ...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~166-~166: To join two clauses or introduce examples, consider using an em dash.
Context: ...omConfig-internal/exec/terraform.go` - Original implementation of this pattern ...
(DASH_RULE)
[typographical] ~196-~196: To join two clauses or introduce examples, consider using an em dash.
Context: ...terraform *` (all terraform subcommands) - Has specific component+stack ### CLI Co...
(DASH_RULE)
🪛 markdownlint-cli2 (0.18.1)
tests/fixtures/scenarios/stack-auth-defaults/README.md
22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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
…commands Add the --identity flag as a persistent flag on the list command so users can explicitly specify an identity for authentication. Also bind to ATMOS_IDENTITY and IDENTITY environment variables. This follows the same pattern as the describe commands. The flag/env var is inherited by all list subcommands (instances, components, stacks, etc.). When not specified, stack-level default identity scanning is used. Example usage: atmos list instances --identity=my-identity atmos list instances -i my-identity ATMOS_IDENTITY=my-identity atmos list instances 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Document the --identity flag and ATMOS_IDENTITY environment variable support for all atmos list subcommands: - list stacks - list components - list values - list vars - list metadata - list settings - list workflows - list themes Also add Authentication section to main list usage page explaining: - Using --identity flag - Using ATMOS_IDENTITY env var - Stack-level default identity scanning 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Updated the following list commands to support --identity flag and ATMOS_IDENTITY environment variable: - list stacks - list components - list values/vars - list metadata - list settings The --identity flag is registered as a persistent flag on the parent listCmd, so it's available to all subcommands. Commands that don't use ExecuteDescribeStacks (workflows, vendor, themes) don't need auth support since they read from config/registry directly. Created helper functions in utils.go: - getIdentityFromCommand: Gets identity from flag or env var - createAuthManagerForList: Creates AuthManager for list commands - setDefaultCSVDelimiter: Sets CSV delimiter default - getComponentFilter: Extracts component filter from args - initConfigAndAuth: Initializes CLI config and auth manager - validateComponentFilter: Validates component exists - handleNoValuesError: Handles NoValuesFoundError with logging Refactored metadata.go and settings.go to reduce cyclomatic complexity by extracting common logic into reusable helper functions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
cmd/describe_component.go (1)
75-119: Component-auth merge wiring fordescribe componentlooks solid, with one small lint nit.
- Good call loading
atmosConfigup front, then:
- Copying global auth with
auth.CopyGlobalAuthConfig(&atmosConfig.Auth).- Using
e.ExecuteDescribeComponentwithProcessTemplates=falseandProcessYamlFunctions=falseplusAuthManager: nilto get a raw component config without triggering auth-dependent YAML functions.- Short-circuiting on
ErrInvalidComponentso we don’t prompt for an identity on a non-existent component.- Merging component-level (including stack-level)
authinto global viaauth.MergeComponentAuthFromConfig(&atmosConfig.Auth, componentConfig, &atmosConfig, cfg.AuthSectionName)and then creating theAuthManagerfrom the merged config.- That matches the documented “Component Auth Merge” approach and should correctly surface stack-level default identities for this command.
Minor nit for linters: the trailing inline comment on the
AuthManagerfieldAuthManager: nil, // No auth manager yet - we're determining which identity to use.should end with a period to satisfy the “all comments end with a period” rule.
website/docs/cli/commands/list/list-workflows.mdx (1)
95-133: Remove duplicate Examples section.Lines 115-133 appear to be a duplicate of lines 95-114. The "Examples" section with "Custom Columns for Workflows" is repeated twice with identical content.
Apply this diff to remove the duplicate:
-## Examples - -### Custom Columns for Workflows - -This configuration customizes the output of `atmos list workflows`: - -```yaml -# In atmos.yaml -workflows: - list: - columns: - - name: Workflow - value: "{{ .workflow_name }}" - - name: Definition File - value: "{{ .workflow_file }}" - - name: Description - value: "{{ .workflow_description }}" -``` -Running `atmos list workflows` will display these columns.cmd/list/instances.go (1)
3-14: Reorganize imports into three groups separated by blank lines.The import organization doesn't follow the coding guidelines. Imports should be in three groups (Go stdlib, 3rd-party excluding Atmos, Atmos packages) separated by blank lines.
Apply this diff to reorganize imports:
import ( "github.com/spf13/cobra" "github.com/spf13/viper" + e "github.com/cloudposse/atmos/internal/exec" "github.com/cloudposse/atmos/pkg/auth" cfg "github.com/cloudposse/atmos/pkg/config" "github.com/cloudposse/atmos/pkg/flags" "github.com/cloudposse/atmos/pkg/flags/global" "github.com/cloudposse/atmos/pkg/list" "github.com/cloudposse/atmos/pkg/schema" )As per coding guidelines, organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically.
♻️ Duplicate comments (2)
tests/fixtures/scenarios/stack-auth-defaults/README.md (1)
43-43: Broken link reference already flagged.The past review comment already identified this broken link. The file should be
docs/fixes/stack-level-default-auth-identity.mdnotstack-level-default-identity.md.internal/exec/workflow_utils_test.go (1)
3-13: Import organization follows guidelines.Three groups properly separated: stdlib, 3rd-party (testify), Atmos packages with correct aliases.
🧹 Nitpick comments (5)
docs/fixes/stack-level-default-auth-identity.md (1)
1-210: Nice, this doc crisply explains the stack-level auth defaults behavior.The issue description, root cause split by command type, two-approach solution, precedence rules, and “key files changed” sections all line up with the implementation pattern in the codebase.
Only tiny nit: in the “Why Profile Config Works” / “Why Stack Config Didn’t Work” area, one bullet like “The
AuthManagersees them when checking for defaults” could end with a period for consistency with the rest of the bullets, but that’s purely cosmetic.website/docs/cli/commands/list/themes.mdx (1)
86-87: Themes identity flag docs are fine; consider marking it optional like others.The note that
--identityis inherited and usually unnecessary here is helpful, and theATMOS_IDENTITYreference is consistent with other docs. For symmetry with the rest of the list docs, you might add<em>(optional)</em>to the flag label, but that’s purely a consistency polish.cmd/list/components.go (1)
90-94: Consider consistent error wrapping.The authManager error is returned directly. Other places in this file wrap errors with context (lines 87-88, 98-99). For consistency, you might wrap this one too.
authManager, err := createAuthManagerForList(cmd, &atmosConfig) if err != nil { - return nil, err + return nil, fmt.Errorf("error creating auth manager: %v", err) }cmd/list/utils.go (1)
127-211: Unify identity normalization between list helpers and GetIdentityFromFlags.
getIdentityFromCommandreturns the raw--identity/viper.GetString("identity")value, whileGetIdentityFromFlagsnormalizes"false","0","no","off"(any case) into the disabled sentinel before creating the auth manager. That means for list commands:
--identity=off/ATMOS_IDENTITY=offwill behave as expected.- But
--identity=false,--identity=no, orATMOS_IDENTITY=false/no/0will be treated as literal identity names, not as “auth disabled”, unlike other commands that go throughGetIdentityFromFlags.To keep identity behavior consistent across commands, consider reusing the same normalization logic here (e.g., factoring it into a small shared helper that both
GetIdentityFromFlagsandgetIdentityFromCommandcan call, or duplicating the small switch locally withcfg.IdentityFlagDisabledValue).cmd/list/instances.go (1)
93-100: Consider using existing identity resolution helper if available.The manual flag-then-env-var resolution pattern here may duplicate logic elsewhere in the codebase. If a helper function exists for identity resolution, using it would provide a single code path and reduce duplication.
Based on osterman's comment.
|
@coderabbitai full review please |
|
@aknysh: I'll perform a comprehensive review of this PR. This addresses stack-level authentication identity resolution across 🧠 Learnings used✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/fixtures/scenarios/stack-auth-defaults/README.md (1)
43-45: Link reference appears correct now.The file path
docs/fixes/stack-level-default-auth-identity.mdmatches the actual documentation file in this PR. Previous review flagged this was addressed.
🧹 Nitpick comments (2)
docs/fixes/stack-level-default-auth-identity.md (1)
88-88: Minor: Missing period at end of paragraph.Static analysis flagged this, but it's a trivial documentation nit.
-- The `AuthManager` sees them when checking for defaults +- The `AuthManager` sees them when checking for defaults.internal/exec/workflow_utils_test.go (1)
381-411: Consider removing tautological constant test.The
TestWorkflowErrTitletest (lines 408-411) only verifies that a constant equals its literal value. Per coding guidelines, avoid tautological tests. TheTestKnownWorkflowErrorsSlicetest is more valuable as it confirms expected errors are present.-// TestWorkflowErrTitle tests the error title constant. -func TestWorkflowErrTitle(t *testing.T) { - assert.Equal(t, "Workflow Error", WorkflowErrTitle) -}
📜 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 (18)
cmd/describe_affected.go(1 hunks)cmd/describe_dependents.go(1 hunks)cmd/describe_stacks.go(1 hunks)cmd/identity_flag.go(1 hunks)cmd/list/instances.go(2 hunks)cmd/list/list.go(1 hunks)cmd/list/utils.go(2 hunks)docs/fixes/stack-level-default-auth-identity.md(1 hunks)internal/exec/terraform.go(1 hunks)internal/exec/terraform_nested_auth_helper.go(1 hunks)internal/exec/workflow_utils.go(2 hunks)internal/exec/workflow_utils_test.go(3 hunks)pkg/auth/manager_helpers.go(4 hunks)pkg/auth/manager_helpers_test.go(1 hunks)pkg/config/stack_auth_loader.go(1 hunks)pkg/config/stack_auth_loader_test.go(1 hunks)tests/fixtures/scenarios/stack-auth-defaults/README.md(1 hunks)website/docs/cli/commands/list/usage.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- cmd/describe_affected.go
- cmd/list/list.go
- cmd/describe_dependents.go
- website/docs/cli/commands/list/usage.mdx
- cmd/describe_stacks.go
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use functional options pattern to avoid functions with many parameters. Define Option functions that modify a Config struct and pass variadic options to New functions.
Use context.Context for cancellation signals, deadlines/timeouts, and request-scoped values (trace IDs). Context should be first parameter in functions that accept it. DO NOT use context for configuration, dependencies, or avoiding proper function parameters.
All comments must end with periods. This is enforced by the godot linter.
NEVER delete existing comments without a very strong reason. Preserve helpful comments, update them to match code changes, refactor for clarity, and add context when modifying code. Only remove factually incorrect, duplicated, or outdated comments.
Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
All errors MUST be wrapped using static errors defined in errors/errors.go. Use errors.Join for combining multiple errors, fmt.Errorf with %w for adding string context, error builder for complex errors, and errors.Is() for error checking. NEVER use dynamic errors directly.
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation. Never create manual mocks.
Use viper.BindEnv with ATMOS_ prefix for environment variables.
Use colors from pkg/ui/theme/colors.go for theme consistency.
Ensure Linux/macOS/Windows compatibility. Use SDKs over binaries. Use filepath.Join(), not hardcoded path separators.
Small focused files (<600 lines). One cmd/impl per file. Co-locate tests. Never use //revive:disable:file-length-limit.
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider us...
Files:
cmd/identity_flag.gopkg/auth/manager_helpers_test.gointernal/exec/terraform.gopkg/auth/manager_helpers.gointernal/exec/workflow_utils.gopkg/config/stack_auth_loader.gocmd/list/utils.gopkg/config/stack_auth_loader_test.gocmd/list/instances.gointernal/exec/terraform_nested_auth_helper.gointernal/exec/workflow_utils_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cmd/**/*.go: Use unified flag parsing infrastructure in pkg/flags/. Commands MUST use flags.NewStandardParser() for command-specific flags. NEVER call viper.BindEnv() or viper.BindPFlag() directly outside pkg/flags/ (Forbidigo enforces this).
Telemetry is auto-enabled via RootCmd.ExecuteC(). Non-standard paths use telemetry.CaptureCmd(). Never capture user data.
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/identity_flag.gocmd/list/utils.gocmd/list/instances.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: 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. Target >80% code coverage.
Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
**/*_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
Files:
pkg/auth/manager_helpers_test.gopkg/config/stack_auth_loader_test.gointernal/exec/workflow_utils_test.go
🧠 Learnings (58)
📓 Common learnings
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
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.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
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.
📚 Learning: 2025-09-07T18:07:00.549Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
Applied to files:
cmd/identity_flag.gointernal/exec/terraform.gopkg/auth/manager_helpers.godocs/fixes/stack-level-default-auth-identity.mdinternal/exec/workflow_utils.gopkg/config/stack_auth_loader.gocmd/list/utils.gocmd/list/instances.gointernal/exec/terraform_nested_auth_helper.gotests/fixtures/scenarios/stack-auth-defaults/README.md
📚 Learning: 2025-09-25T01:02:48.697Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/manager.go:304-312
Timestamp: 2025-09-25T01:02:48.697Z
Learning: The auth manager in pkg/auth/manager.go should remain cloud-agnostic and not contain AWS-specific logic or references to specific cloud providers. Keep the manager generic and extensible.
Applied to files:
cmd/identity_flag.gopkg/auth/manager_helpers_test.gointernal/exec/terraform.gopkg/auth/manager_helpers.gointernal/exec/terraform_nested_auth_helper.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:
cmd/identity_flag.gointernal/exec/terraform.gopkg/auth/manager_helpers.gointernal/exec/workflow_utils.gocmd/list/utils.gocmd/list/instances.gointernal/exec/terraform_nested_auth_helper.gointernal/exec/workflow_utils_test.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:
cmd/identity_flag.gopkg/auth/manager_helpers_test.gointernal/exec/terraform.gopkg/auth/manager_helpers.gointernal/exec/workflow_utils.gopkg/config/stack_auth_loader.gocmd/list/utils.gocmd/list/instances.gointernal/exec/terraform_nested_auth_helper.gotests/fixtures/scenarios/stack-auth-defaults/README.md
📚 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:
cmd/identity_flag.gointernal/exec/terraform.gopkg/auth/manager_helpers.gocmd/list/utils.gocmd/list/instances.gointernal/exec/terraform_nested_auth_helper.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
cmd/identity_flag.go
📚 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:
cmd/identity_flag.gopkg/auth/manager_helpers.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:
cmd/identity_flag.gocmd/list/utils.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/auth/manager_helpers_test.gopkg/config/stack_auth_loader_test.gointernal/exec/workflow_utils_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/auth/manager_helpers_test.gotests/fixtures/scenarios/stack-auth-defaults/README.md
📚 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/auth/manager_helpers_test.gotests/fixtures/scenarios/stack-auth-defaults/README.md
📚 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/auth/manager_helpers_test.gopkg/config/stack_auth_loader_test.gointernal/exec/workflow_utils_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/auth/manager_helpers_test.gopkg/config/stack_auth_loader_test.go
📚 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:
internal/exec/terraform.gocmd/list/instances.gointernal/exec/terraform_nested_auth_helper.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:
internal/exec/terraform.gocmd/list/utils.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
internal/exec/terraform.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:
internal/exec/terraform.gopkg/auth/manager_helpers.gocmd/list/utils.gocmd/list/instances.gointernal/exec/terraform_nested_auth_helper.gointernal/exec/workflow_utils_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
Applied to files:
pkg/auth/manager_helpers.gointernal/exec/workflow_utils_test.go
📚 Learning: 2025-10-13T18:13:54.020Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1622
File: pkg/perf/perf.go:140-184
Timestamp: 2025-10-13T18:13:54.020Z
Learning: In pkg/perf/perf.go, the `trackWithSimpleStack` function intentionally skips ownership checks at call stack depth > 1 to avoid expensive `getGoroutineID()` calls on every nested function. This is a performance optimization for the common single-goroutine execution case (most Atmos commands), accepting the rare edge case of potential metric corruption if multi-goroutine execution occurs at depth > 1. The ~19× performance improvement justifies this trade-off.
Applied to files:
pkg/auth/manager_helpers.gointernal/exec/workflow_utils.go
📚 Learning: 2025-11-09T19:06:58.470Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1752
File: pkg/profile/list/formatter_table.go:27-29
Timestamp: 2025-11-09T19:06:58.470Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` is enforced on all functions via linting, including high-frequency utility functions, formatters, and renderers. This is a repository-wide policy to maintain consistency and avoid making case-by-case judgment calls about which functions should have profiling.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-09-27T20:50:20.564Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1533
File: pkg/config/load.go:585-637
Timestamp: 2025-09-27T20:50:20.564Z
Learning: In the cloudposse/atmos repository, command merging prioritizes precedence over display ordering. Help commands are displayed lexicographically regardless of internal array order, so the mergeCommandArrays function focuses on ensuring the correct precedence chain (top-level file wins) rather than maintaining specific display order.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-11-30T04:16:24.155Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:34-48
Timestamp: 2025-11-30T04:16:24.155Z
Learning: In the cloudposse/atmos repository, the `defer perf.Track()` guideline applies to functions that perform meaningful work (I/O, computation, external calls), but explicitly excludes trivial accessors/mutators (e.g., simple getters, setters with single integer increments, string joins, or map appends) where the tracking overhead would exceed the actual method cost and provide no actionable performance data. Hot-path methods called in tight loops should especially avoid perf.Track() if they perform only trivial operations.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Applied to files:
pkg/auth/manager_helpers.gocmd/list/utils.gointernal/exec/workflow_utils_test.go
📚 Learning: 2025-11-30T04:16:01.899Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:50-59
Timestamp: 2025-11-30T04:16:01.899Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` should NOT be added to trivial O(1) getter methods that only return field references or check map lengths (e.g., `GetDeferredValues()`, `HasDeferredValues()`). The guideline to add perf tracking to "all public functions" applies to functions that do meaningful work (I/O, computation, external calls), not to trivial accessors where the tracking overhead would exceed the operation time and pollute performance reports.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-05-30T03:21:37.197Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1274
File: go.mod:63-63
Timestamp: 2025-05-30T03:21:37.197Z
Learning: The redis dependency (github.com/redis/go-redis/v9) in the atmos project is only used in tests, not in production code.
Applied to files:
pkg/auth/manager_helpers.go
📚 Learning: 2025-04-11T22:06:46.999Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
internal/exec/workflow_utils.gointernal/exec/workflow_utils_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:
internal/exec/workflow_utils.gointernal/exec/workflow_utils_test.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/stack_auth_loader.go
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
cmd/list/utils.gointernal/exec/terraform_nested_auth_helper.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:
cmd/list/utils.gocmd/list/instances.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
cmd/list/utils.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 cmd/**/*.go : Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Applied to files:
cmd/list/utils.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 Viper for managing configuration, environment variables, and flags in CLI commands
Applied to files:
cmd/list/utils.gocmd/list/instances.go
📚 Learning: 2025-01-18T15:15:41.645Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform.go:37-46
Timestamp: 2025-01-18T15:15:41.645Z
Learning: In the atmos CLI, error handling is intentionally structured to use LogErrorAndExit for consistent error display, avoiding Cobra's default error handling to prevent duplicate error messages.
Applied to files:
cmd/list/utils.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
cmd/list/utils.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 cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file under `cmd/` directory
Applied to files:
cmd/list/utils.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:
cmd/list/utils.go
📚 Learning: 2025-11-10T03:03:31.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-11-10T03:03:31.505Z
Learning: In the Atmos codebase, commands using the `StandardParser` flag pattern (from pkg/flags) do NOT need explicit `viper.BindPFlag()` calls in their code. The StandardParser encapsulates flag binding internally: flags are registered via `parser.RegisterFlags(cmd)` in init(), and bound via `parser.BindFlagsToViper(cmd, v)` in RunE, which internally calls viper.BindPFlag for each flag. This pattern is used throughout Atmos (e.g., cmd/toolchain/get.go, cmd/toolchain/info.go, cmd/toolchain/install.go, cmd/toolchain/path.go). Do not flag missing viper.BindPFlag calls when StandardParser is used.
Applied to files:
cmd/list/utils.gocmd/list/instances.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to cmd/**/*.go : Use unified flag parsing infrastructure in pkg/flags/. Commands MUST use flags.NewStandardParser() for command-specific flags. NEVER call viper.BindEnv() or viper.BindPFlag() directly outside pkg/flags/ (Forbidigo enforces this).
Applied to files:
cmd/list/instances.go
📚 Learning: 2025-08-16T23:33:07.477Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:651-652
Timestamp: 2025-08-16T23:33:07.477Z
Learning: In the cloudposse/atmos Go codebase, ExecuteDescribeDependents expects a pointer to AtmosConfiguration (*schema.AtmosConfiguration), so when calling it with a value returned by cfg.InitCliConfig (which returns schema.AtmosConfiguration), the address-of operator (&) is necessary: ExecuteDescribeDependents(&atmosConfig, ...).
Applied to files:
cmd/list/instances.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Use viper.BindEnv with ATMOS_ prefix for environment variables.
Applied to files:
cmd/list/instances.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
internal/exec/terraform_nested_auth_helper.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
internal/exec/workflow_utils_test.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning the error instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions.
Applied to files:
internal/exec/workflow_utils_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*_test.go : Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
Applied to files:
internal/exec/workflow_utils_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:
internal/exec/workflow_utils_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:
internal/exec/workflow_utils_test.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:
internal/exec/workflow_utils_test.go
📚 Learning: 2024-10-31T07:09:31.983Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:181-182
Timestamp: 2024-10-31T07:09:31.983Z
Learning: In `internal/exec/vendor_utils.go`, the variables `mergedSources` and `mergedImports` are declared and used later in the code. Do not suggest removing them as unused variables.
Applied to files:
internal/exec/workflow_utils_test.go
📚 Learning: 2024-11-02T15:35:09.958Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Applied to files:
internal/exec/workflow_utils_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Small focused files (<600 lines). One cmd/impl per file. Co-locate tests. Never use //revive:disable:file-length-limit.
Applied to files:
internal/exec/workflow_utils_test.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: The user confirmed that the errors package has an error string wrapping format, contradicting the previous learning about ErrWrappingFormat being invalid. The current usage of fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) appears to be the correct pattern.
Applied to files:
internal/exec/workflow_utils_test.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: ErrWrappingFormat is correctly defined as "%w: %w" in the errors package and is used throughout the codebase to wrap two error types together. The usage fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) is the correct pattern when both arguments are error types.
Applied to files:
internal/exec/workflow_utils_test.go
📚 Learning: 2025-01-25T03:44:52.619Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md:14-23
Timestamp: 2025-01-25T03:44:52.619Z
Learning: Test fixtures under `tests/fixtures/` should not be modified unless the test case itself needs to change, as they are deliberately set up to represent specific scenarios for testing purposes.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/README.md
📚 Learning: 2025-03-25T12:24:36.177Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:263-264
Timestamp: 2025-03-25T12:24:36.177Z
Learning: Tests for the default Bitbucket username fallback to "x-token-auth" will be added during a future refactoring phase rather than in this PR.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/README.md
📚 Learning: 2025-02-18T15:20:49.080Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1061
File: tests/fixtures/scenarios/vendor-pulls-ssh/vendor.yaml:20-22
Timestamp: 2025-02-18T15:20:49.080Z
Learning: Hardcoded credentials are acceptable in test fixtures when they are specifically testing credential handling, masking, or injection behavior. For example, in `tests/fixtures/scenarios/vendor-pulls-ssh/vendor.yaml`, credentials like "myuser:supersecret" are used to test that direct credentials in URLs are not overwritten by token injection.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/README.md
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
tests/fixtures/scenarios/stack-auth-defaults/README.md
🧬 Code graph analysis (9)
cmd/identity_flag.go (3)
pkg/auth/manager_helpers.go (2)
CreateAndAuthenticateManager(184-192)CreateAndAuthenticateManagerWithAtmosConfig(218-269)pkg/config/const.go (1)
IdentityFlagSelectValue(136-136)cmd/auth.go (1)
IdentityFlagSelectValue(14-14)
internal/exec/terraform.go (5)
pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManagerWithAtmosConfig(218-269)pkg/auth/types/interfaces.go (1)
Identity(81-125)pkg/schema/schema_auth.go (1)
Identity(49-59)pkg/config/const.go (1)
IdentityFlagSelectValue(136-136)cmd/auth.go (1)
IdentityFlagSelectValue(14-14)
pkg/auth/manager_helpers.go (3)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/perf/perf.go (1)
Track(121-138)pkg/config/stack_auth_loader.go (2)
LoadStackAuthDefaults(41-75)MergeStackAuthDefaults(157-172)
internal/exec/workflow_utils.go (2)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/config/stack_auth_loader.go (2)
LoadStackAuthDefaults(41-75)MergeStackAuthDefaults(157-172)
pkg/config/stack_auth_loader.go (2)
pkg/schema/schema.go (1)
AtmosConfiguration(53-94)pkg/utils/glob_utils.go (1)
GetGlobMatches(36-65)
pkg/config/stack_auth_loader_test.go (1)
pkg/config/stack_auth_loader.go (2)
LoadStackAuthDefaults(41-75)MergeStackAuthDefaults(157-172)
cmd/list/instances.go (4)
pkg/config/config.go (1)
InitCliConfig(28-67)pkg/schema/schema.go (1)
ConfigAndStacksInfo(621-715)pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManagerWithAtmosConfig(218-269)pkg/list/list_instances.go (1)
ExecuteListInstancesCmd(305-342)
internal/exec/terraform_nested_auth_helper.go (2)
pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManagerWithAtmosConfig(218-269)pkg/config/const.go (1)
IdentityFlagSelectValue(136-136)
internal/exec/workflow_utils_test.go (2)
errors/errors.go (1)
ExitCodeError(574-576)internal/exec/workflow_utils.go (9)
ErrWorkflowNoSteps(31-31)ErrInvalidWorkflowStepType(32-32)ErrInvalidFromStep(33-33)ErrWorkflowStepFailed(34-34)ErrWorkflowNoWorkflow(35-35)ErrWorkflowFileNotFound(36-36)ErrInvalidWorkflowManifest(37-37)KnownWorkflowErrors(39-47)WorkflowErrTitle(30-30)
🪛 LanguageTool
docs/fixes/stack-level-default-auth-identity.md
[grammar] ~88-~88: Please add a punctuation mark at the end of paragraph.
Context: ...uthManager` sees them when checking for defaults ### Why Stack Config Didn't Work For ...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~166-~166: To join two clauses or introduce examples, consider using an em dash.
Context: ...omConfig-internal/exec/terraform.go` - Original implementation of this pattern ...
(DASH_RULE)
[typographical] ~196-~196: To join two clauses or introduce examples, consider using an em dash.
Context: ...terraform *` (all terraform subcommands) - Has specific component+stack ### CLI Co...
(DASH_RULE)
tests/fixtures/scenarios/stack-auth-defaults/README.md
[uncategorized] ~17-~17: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...s to find auth identity defaults BEFORE full stack processing. This allows stack-level def...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[typographical] ~43-~43: To join two clauses or introduce examples, consider using an em dash.
Context: ...es/stack-level-default-auth-identity.md- Full issue documentation -pkg/config/s...
(DASH_RULE)
⏰ 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 (30)
pkg/config/stack_auth_loader_test.go (1)
1-471: Solid test coverage for the new stack auth loader.The test suite covers a wide range of scenarios including empty paths, multiple files, exclusion patterns, invalid YAML, templates, merging behavior, and precedence rules. Good use of
t.TempDir()for file isolation and proper assertion patterns.A few observations:
- Tests effectively validate the non-fatal error handling for invalid YAML files
- Precedence logic (stack overrides atmos.yaml) is well-tested
- Edge cases like directories named with
.yamlextensions are coveredpkg/config/stack_auth_loader.go (4)
1-13: Imports are properly organized.Three groups separated by blank lines: stdlib, third-party, and Atmos packages with correct aliases (
log,u).
41-75: Clean implementation of LoadStackAuthDefaults.Good defensive handling with early return for empty include paths and non-fatal error handling for individual file failures. The perf tracking and debug logging provide observability.
119-152: The nolint directive is well-justified.The
//nolint:nilerron line 141 is appropriate here. YAML parse errors are expected for files with Go templates, and returning empty defaults without error is the correct behavior for this lightweight pre-scan approach.
154-172: Clear precedence logic in MergeStackAuthDefaults.The approach of clearing existing defaults before applying stack defaults correctly implements the "stack takes precedence" model. Well-documented with inline comments.
internal/exec/terraform.go (1)
106-107: Good integration with the new Atmos-config-enabled auth path.The switch to
CreateAndAuthenticateManagerWithAtmosConfigwith the added comment clarifies the intent. Passing&atmosConfigcorrectly provides the pointer needed for stack-level default loading.pkg/auth/manager_helpers_test.go (3)
878-913: Good test for nil AtmosConfig handling.This verifies backward compatibility - when atmosConfig is nil, the function behaves like the original
CreateAndAuthenticateManager. Correct expected behavior: no default means nil manager returned.
999-1028: Nice integration test with real stack files.Creating temporary stack files and verifying the merge behavior provides confidence that the end-to-end flow works correctly. The assertion on line 1027 validates that stack defaults properly override the initial
Default: false.
1091-1123: Helpful test for resolveIdentityName with auth configured.This validates the auto-detection path when a default identity exists. Good coverage of the resolution logic.
internal/exec/terraform_nested_auth_helper.go (1)
173-179: Consistent update for nested component auth resolution.The change to
CreateAndAuthenticateManagerWithAtmosConfigwith the addedatmosConfigparameter enables stack-level default identity loading for nested component references (e.g.,!terraform.state). This aligns with the broader auth flow changes.docs/fixes/stack-level-default-auth-identity.md (1)
1-210: Excellent documentation of the fix.This document clearly explains:
- The symptom and root cause (chicken-and-egg problem)
- Why profile config works but stack config didn't
- Both solution approaches with code examples
- Precedence order for identity resolution
- Which commands use which approach
The level of detail will help future maintainers understand the design decisions.
internal/exec/workflow_utils.go (2)
68-102: Clean helper for checking and merging default identities.The function correctly:
- Short-circuits when no identities exist
- Loads stack defaults with proper error fallback
- Merges stack defaults (which take precedence)
- Returns whether a default exists after merging
The fallback to atmos.yaml defaults on load error ensures workflows remain functional even if stack scanning fails.
164-169: Good integration point for default identity detection.This enables workflows to automatically use a configured default identity without requiring an explicit
--identityflag. The check happens after explicit identity checks, maintaining proper precedence.internal/exec/workflow_utils_test.go (5)
3-13: Import organization looks good now.The imports are properly organized into three groups with single blank lines separating them. The past review comment about the extra blank line appears to be addressed.
77-86: Good addition of ExitCodeError test cases.These test cases properly verify that
ExitCodeErroris recognized as a known workflow error, both directly and when wrapped. This aligns with the PR's authentication flow improvements.
153-224: Solid table-driven tests for checkAndMergeDefaultIdentity.The test cases cover the essential scenarios: no identities, identities without default, identity with default, and multiple identities. Clean structure following Go testing conventions.
226-266: Comprehensive stack loading integration test.Good use of
t.TempDir()for test isolation. The test properly verifies that stack-level defaults are loaded and merged into the AtmosConfiguration, confirming the identity's default flag is updated.
268-308: Error handling paths well covered.Tests for invalid glob patterns verify graceful degradation - returning true when atmos.yaml has a default despite load errors, and false when no default exists. This matches expected behavior.
cmd/identity_flag.go (2)
133-141: Good clarification in the doc comment.The note about not loading stack configs helps developers choose the right function variant. Clear guidance.
143-161: Clean implementation with accurate documentation.The doc comment correctly describes the precedence: stack defaults override atmos.yaml defaults. The function properly delegates to the auth package implementation, maintaining separation of concerns. This addresses the past review comment about documentation inconsistency.
pkg/auth/manager_helpers.go (4)
181-192: Clean backward-compatible refactor.Delegating to the full implementation with nil atmosConfig maintains backward compatibility while enabling stack auth loading when atmosConfig is provided. Good approach.
194-217: Documentation now accurately reflects implementation.The doc comment correctly states "Stack defaults take precedence over atmos.yaml defaults" (line 200), which matches the
MergeStackAuthDefaultsbehavior. The past review concern about priority order inconsistency is addressed.
218-269: Well-structured implementation with proper instrumentation.The function includes
perf.Trackas required by coding guidelines. The logic flow is clear: disable check → stack defaults loading → identity resolution → manager creation → authentication.
271-294: Robust helper with graceful error handling.The
loadAndMergeStackAuthDefaultsfunction properly:
- Includes perf tracking
- Logs errors without failing (non-fatal approach)
- Short-circuits when no defaults found
- Follows Atmos inheritance model (stack > atmos.yaml)
cmd/list/utils.go (4)
3-21: Imports properly organized.Good separation: stdlib, 3rd-party, atmos packages. Aliases follow conventions (cfg, f, listutils, etc.).
141-161: Good delegation to shared auth infrastructure.Clean wrapper that obtains identity from the command and delegates to the Atmos-config-aware auth manager creation. This properly enables stack-level default identity loading for list commands.
163-176: Simple, focused helper functions.
setDefaultCSVDelimiterandgetComponentFilterare clear single-purpose helpers that reduce duplication across list commands.
178-211: Well-structured initialization and error handling helpers.
initConfigAndAuthcentralizes config+auth setup,validateComponentFilterprovides clear validation, andhandleNoValuesErroruses proper error type checking witherrors.As(). Good patterns.cmd/list/instances.go (2)
8-14: Imports properly organized.Auth, config, and schema imports added correctly for the new authentication flow.
87-107: Authentication flow properly integrated.The command correctly:
- Loads atmos config
- Gets identity via shared helper (
getIdentityFromCommand) that checks the--identityflag (registered as a persistent flag on the parentlistcommand) and falls back to theATMOS_IDENTITYenvironment variable- Creates auth manager with stack-level default identity loading
- Passes auth manager to the execution function
The
--identityflag is properly registered on the parentlistcommand (cmd/list/list.go:31) as a persistent flag, making it available to all subcommands includinginstances. The helper function correctly handles flag precedence (explicit flag > environment variable).
|
@aknysh: I'll perform a comprehensive review of this PR now. This addresses stack-level authentication identity resolution for 🧠 Learnings used✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
NOTICE (1)
186-186: Regenerate NOTICE file.Pipeline is failing because this file is out of date. Run
./scripts/generate-notice.shlocally and commit the result. Based on learnings, this file is programmatically generated and shouldn't be manually edited.Also applies to: 571-571
internal/exec/workflow_utils_test.go (1)
9-12: Extra blank line in imports.Per a prior review, there's an extra blank line between the third-party import and Atmos packages. Should be a single blank line.
🧹 Nitpick comments (6)
cmd/list/stacks.go (1)
83-103: Auth integration looks good, but consider adding perf tracking.The authentication flow integration is clean. However, per coding guidelines, public functions should include performance tracking.
+import "github.com/cloudposse/atmos/pkg/perf" + func listStacksWithOptions(cmd *cobra.Command, opts *StacksOptions) ([]string, error) { configAndStacksInfo := schema.ConfigAndStacksInfo{} atmosConfig, err := config.InitCliConfig(configAndStacksInfo, true) if err != nil { return nil, fmt.Errorf("error initializing CLI config: %v", err) } + defer perf.Track(&atmosConfig, "list.listStacksWithOptions")() // Create AuthManager for authentication support.pkg/auth/manager_helpers.go (1)
226-226: Minor: Log message references wrong function name.The log message says "CreateAndAuthenticateManager called" but this is
CreateAndAuthenticateManagerWithAtmosConfig.- log.Debug("CreateAndAuthenticateManager called", "identityName", identityName, "hasAuthConfig", authConfig != nil) + log.Debug("CreateAndAuthenticateManagerWithAtmosConfig called", "identityName", identityName, "hasAuthConfig", authConfig != nil, "hasAtmosConfig", atmosConfig != nil)cmd/list/instances.go (2)
87-107: Consider usinginitConfigAndAuthhelper to reduce duplication.This block duplicates the pattern already encapsulated in
initConfigAndAuth(utils.go lines 178-192). That helper consolidates config loading and auth manager creation into a single call.Per the past review comment, there's a preference for a single code path. Using the helper would align with that guidance.
- // Load atmos configuration to get auth config. - atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, false) - if err != nil { - return err - } - - // Get identity from --identity flag or ATMOS_IDENTITY env var using shared helper. - identityName := getIdentityFromCommand(cmd) - - // Create AuthManager with stack-level default identity loading. - authManager, err := auth.CreateAndAuthenticateManagerWithAtmosConfig( - identityName, - &atmosConfig.Auth, - cfg.IdentityFlagSelectValue, - &atmosConfig, - ) - if err != nil { - return err - } + // Load atmos configuration and create auth manager. + _, authManager, err := initConfigAndAuth(cmd) + if err != nil { + return err + }Note: If
atmosConfigis needed later, keep the full variable binding. Otherwise, the helper simplifies this nicely.
3-14: Imports can be trimmed if usinginitConfigAndAuth.If you adopt the helper, the
auth,cfg, andschemaimports become unnecessary sinceinitConfigAndAuthhandles those dependencies internally.pkg/config/stack_auth_loader.go (1)
77-117: Potential duplicate files from overlapping glob patterns.If
includePathscontains overlapping patterns (e.g.,stacks/*.yamlandstacks/prod/*.yaml), the same file could appear multiple times inallFiles. WhileloadFileForAuthDefaultsis idempotent, processing duplicates wastes cycles.Consider deduplicating:
func getAllStackFiles(includePaths, excludePaths []string) []string { - var allFiles []string + seen := make(map[string]bool) + var allFiles []string for _, pattern := range includePaths { matches, err := u.GetGlobMatches(pattern) if err != nil { continue // Non-fatal: skip invalid patterns. } - allFiles = append(allFiles, matches...) + for _, match := range matches { + if !seen[match] { + seen[match] = true + allFiles = append(allFiles, match) + } + } }cmd/list/utils_test.go (1)
102-172: Guard against viper global state leaking between tests.Nice coverage of identity precedence (none → viper → flag → flag over viper). To avoid leaking viper state into other tests, add a top‑level cleanup:
func TestGetIdentityFromCommand(t *testing.T) { + t.Cleanup(viper.Reset) + testCases := []struct {You can still keep the per‑case
viper.Reset()calls for isolation.As per coding guidelines, keeping global config state isolated in tests improves reliability.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
NOTICE (1)
186-186: Regenerate NOTICE file to resolve the pipeline failure.The pipeline indicates the NOTICE file is out of date. Run
./scripts/generate-notice.shlocally and commit the updated file.Based on learnings, the NOTICE file is programmatically generated and should not be manually edited.
Also applies to: 571-571
cmd/list/utils_test.go (1)
3-14: Reorganize imports per coding guidelines.Imports must be organized into three groups separated by blank lines: Go stdlib, 3rd-party packages, and Atmos packages.
As per coding guidelines, maintain the three-group import structure with proper spacing.
cmd/list/utils.go (1)
127-139: Consider consolidating with existing GetIdentityFromFlags function.This logic duplicates
cmd/identity_flag.go:GetIdentityFromFlags(). While the current implementation works, consolidating to use the existing function would maintain a single code path for identity retrieval.cmd/list/instances.go (1)
78-107: Use the sameConfigAndStacksInfofor auth as for listing (and consider reusing the list auth helper).Right now
executeListInstancesCmddoes:// Load atmos configuration to get auth config. atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, false)while
list.ExecuteListInstancesCmdlater does:err := list.ExecuteListInstancesCmd(&configAndStacksInfo, cmd, args, authManager)and inside
ExecuteListInstancesCmdthe config is re‑initialized with:atmosConfig, err := cfg.InitCliConfig(*info, true)So the authManager is built from an Atmos config that ignores the CLI-derived
configAndStacksInfo(base path, config overrides, etc.), while the actual list execution uses a config built fromconfigAndStacksInfo. That can lead to subtle mismatches where auth is resolved against one config and stacks are loaded from another.You can keep the current flow but fix the mismatch with a minimal change:
- // Load atmos configuration to get auth config. - atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, false) + // Load Atmos configuration for auth using the same CLI-derived settings. + atmosConfig, err := cfg.InitCliConfig(configAndStacksInfo, false)This ensures identity resolution and stack listing share the same Atmos configuration.
Separately, you already have
createAuthManagerForListused incmd/list/components.goto encapsulate identity resolution and auth creation for list commands. To keep behavior consistent and avoid drift, consider replacing the inlinegetIdentityFromCommand+auth.CreateAndAuthenticateManagerWithAtmosConfigblock here with a call to that helper as well, mirroring the components command. This would centralize list-auth behavior in one place. (This echoes earlier maintainer feedback about avoiding reimplementation of shared logic.)
🧹 Nitpick comments (10)
tests/fixtures/scenarios/stack-auth-defaults/README.md (1)
22-32: Add language specifier to fenced code block.The markdown code fence is missing a language specification, which violates markdownlint rules. Since this displays a directory tree structure, use
textor leave it blank but explicit.-``` +```text stack-auth-defaults/ ├── atmos.yaml # Auth config with identity (NO default) ├── stacks/website/docs/cli/commands/list/list-metadata.mdx (1)
44-45:--identityflag docs look good; consider mentioning--identity=offsemantics.The new flag description is clear and matches the intended auth behavior for metadata listing. If you want full parity with the broader auth docs, you might optionally add a short note that
--identity=offdisables auth for this command, or link to the new stack-level-default-auth docs.website/docs/cli/commands/list/list-components.mdx (1)
66-67: Consistent and clear--identitydocumentation.This mirrors the metadata docs and clearly explains when identity is needed and how
ATMOS_IDENTITYties in. Optionally, you could also mention--identity=offhere for symmetry with the global auth behavior.cmd/describe_component.go (1)
75-119: Component auth merge flow is sound and matches terraform behavior.The sequence of:
- initializing
atmosConfigjust to access global auth,- reading the component config via
ExecuteDescribeComponentwith templates/functions disabled,- merging component-level auth into a copied global config, and
- creating the auth manager from the merged config
is coherent and avoids YAML-function/auth circular dependency. The early return on
ErrInvalidComponentalso prevents spurious identity prompts.If you find yourself repeating this pattern elsewhere (terraform, describe component, etc.), consider a small helper like
BuildMergedAuthConfigForComponent(atmosConfig, component, stack)to centralize the merge logic.internal/exec/workflow_utils.go (1)
68-102: Default-identity helper is correct, but its effect on workflows may be limited.
checkAndMergeDefaultIdentitycleanly loads and merges stack-level defaults and respects the “stack overrides atmos.yaml” rule. InExecuteWorkflow, though, its only effect is to flipneedsAuthand trigger auth manager creation when there is a default identity but no explicit step/CLI identity. BecausestepIdentityremains empty in that case, no authentication or env preparation happens for any step.If the intent is for workflows with no explicit identity to still run under the stack-default identity, you may want to also infer a
stepIdentity(e.g., when there is exactly one default) or otherwise consume the default when building step env. If this extra behavior is not desired, consider adding a brief comment noting that defaults here only control manager initialization so future readers don’t assume steps automatically inherit them.Also applies to: 161-169
cmd/list/instances_test.go (1)
253-272: Consider asserting upload flag existence.The test currently allows the upload flag to be missing (lines 267-271). If the flag is expected to exist, a stricter assertion would catch regressions.
- if uploadFlag != nil { - assert.Equal(t, "false", uploadFlag.DefValue, "upload flag default should be false") - } - // Note: If the flag is not found, that's not necessarily an error - it may be registered - // lazily or through a different mechanism. The important test is that the parser exists. + assert.NotNil(t, uploadFlag, "upload flag should be registered") + assert.Equal(t, "false", uploadFlag.DefValue, "upload flag default should be false")pkg/auth/manager_helpers.go (1)
182-194: Stack-aware auth helper looks correct; tiny optional log polish.The split between
CreateAndAuthenticateManager(backward-compatible wrapper) andCreateAndAuthenticateManagerWithAtmosConfiglooks clean, and theloadAndMergeStackAuthDefaultshelper correctly loads stack-level defaults and applies them with stack > atmos.yaml precedence. Error handling for stack-default loading is non-fatal and well‑scoped.One tiny nit: the debug line inside
CreateAndAuthenticateManagerWithAtmosConfigstill logs"CreateAndAuthenticateManager called", which could be confusing when tracing behavior. If you touch this again, consider updating the log string to match the function name.Also applies to: 218-269, 271-294
pkg/auth/manager_helpers_test.go (1)
1000-1021: Usefilepath.Joinfor temp stack file paths and globs.Minor portability nit: building paths as
tmpDir+"/stack.yaml"andtmpDir + "/*.yaml"can be brittle on Windows. Usingfilepath.Join(tmpDir, "stack.yaml")andfilepath.Join(tmpDir, "*.yaml")will be more robust across platforms while keeping the test semantics the same.pkg/config/stack_auth_loader.go (1)
41-75: Stack auth loader and merge semantics look solid; consider a nil guard foratmosConfig.The loader/merger design is coherent: globbing include/exclude paths, doing minimal YAML parsing into
stackAuthSection, treating glob/YAML issues as non-fatal, and then letting stack defaults clear and override atmos.yaml defaults without creating new identities all align with the documented precedence and “lightweight scan” intent.One small hardening you might consider: add an early
if atmosConfig == nil { return defaults, nil }inLoadStackAuthDefaultsbefore dereferencingIncludeStackAbsolutePaths. Current call sites appear to always pass a non-nil config, but this would prevent future nil-pointer surprises at essentially zero cost.Also applies to: 77-117, 119-152, 155-172, 175-215
docs/fixes/stack-level-default-auth-identity.md (1)
83-96: Doc content matches implementation; only minor punctuation nits.The explanation of the two approaches, precedence order, and which commands use which path lines up with the code. If you care about polishing language-tool warnings, you could add a trailing period where sentences currently flow directly into the next heading and consider em dashes instead of hyphens in a couple of “X - Y” phrases, but that’s purely cosmetic.
Also applies to: 152-157, 164-167, 194-197
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/list/utils.go (1)
219-236: Component validation and NoValues handling look good (tiny optional tweak).
validateComponentFilterreturning a typedComponentDefinitionNotFoundErrorandhandleNoValuesErrorusingerrors.Asto special‑caseNoValuesFoundErroralign well with the new list error types and keep callers simple. If you ever need richer logging, you could optionally pass the entire*NoValuesFoundErrortologFuncinstead of justcomponentFilter, but it’s not necessary for this PR.
📜 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 (2)
cmd/list/utils.go(2 hunks)cmd/list/utils_test.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/list/utils_test.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use functional options pattern to avoid functions with many parameters. Define Option functions that modify a Config struct and pass variadic options to New functions.
Use context.Context for cancellation signals, deadlines/timeouts, and request-scoped values (trace IDs). Context should be first parameter in functions that accept it. DO NOT use context for configuration, dependencies, or avoiding proper function parameters.
All comments must end with periods. This is enforced by the godot linter.
NEVER delete existing comments without a very strong reason. Preserve helpful comments, update them to match code changes, refactor for clarity, and add context when modifying code. Only remove factually incorrect, duplicated, or outdated comments.
Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
All errors MUST be wrapped using static errors defined in errors/errors.go. Use errors.Join for combining multiple errors, fmt.Errorf with %w for adding string context, error builder for complex errors, and errors.Is() for error checking. NEVER use dynamic errors directly.
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation. Never create manual mocks.
Use viper.BindEnv with ATMOS_ prefix for environment variables.
Use colors from pkg/ui/theme/colors.go for theme consistency.
Ensure Linux/macOS/Windows compatibility. Use SDKs over binaries. Use filepath.Join(), not hardcoded path separators.
Small focused files (<600 lines). One cmd/impl per file. Co-locate tests. Never use //revive:disable:file-length-limit.
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider us...
Files:
cmd/list/utils.go
cmd/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cmd/**/*.go: Use unified flag parsing infrastructure in pkg/flags/. Commands MUST use flags.NewStandardParser() for command-specific flags. NEVER call viper.BindEnv() or viper.BindPFlag() directly outside pkg/flags/ (Forbidigo enforces this).
Telemetry is auto-enabled via RootCmd.ExecuteC(). Non-standard paths use telemetry.CaptureCmd(). Never capture user data.
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/list/utils.go
🧠 Learnings (21)
📓 Common learnings
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
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.
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
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.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
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.
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/manager.go:304-312
Timestamp: 2025-09-25T01:02:48.697Z
Learning: The auth manager in pkg/auth/manager.go should remain cloud-agnostic and not contain AWS-specific logic or references to specific cloud providers. Keep the manager generic and extensible.
📚 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:
cmd/list/utils.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:
cmd/list/utils.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:
cmd/list/utils.go
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
cmd/list/utils.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
cmd/list/utils.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:
cmd/list/utils.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 cmd/**/*.go : Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Applied to files:
cmd/list/utils.go
📚 Learning: 2025-09-07T18:07:00.549Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
Applied to files:
cmd/list/utils.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to **/*.go : Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Applied to files:
cmd/list/utils.go
📚 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:
cmd/list/utils.go
📚 Learning: 2025-09-05T14:57:37.360Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 1448
File: cmd/ansible.go:26-28
Timestamp: 2025-09-05T14:57:37.360Z
Learning: The Atmos codebase uses a consistent pattern for commands that delegate to external tools: `PersistentFlags().Bool("", false, doubleDashHint)` where doubleDashHint provides help text about using double dashes to separate Atmos options from native command arguments. This pattern is used across terraform, packer, helmfile, atlantis, aws, and ansible commands.
Applied to files:
cmd/list/utils.go
📚 Learning: 2025-01-09T22:37:01.004Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.
Applied to files:
cmd/list/utils.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:
cmd/list/utils.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:
cmd/list/utils.go
📚 Learning: 2025-01-18T15:15:41.645Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform.go:37-46
Timestamp: 2025-01-18T15:15:41.645Z
Learning: In the atmos CLI, error handling is intentionally structured to use LogErrorAndExit for consistent error display, avoiding Cobra's default error handling to prevent duplicate error messages.
Applied to files:
cmd/list/utils.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
cmd/list/utils.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 Viper for managing configuration, environment variables, and flags in CLI commands
Applied to files:
cmd/list/utils.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 cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file under `cmd/` directory
Applied to files:
cmd/list/utils.go
📚 Learning: 2024-12-05T22:33:40.955Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_components.go:53-54
Timestamp: 2024-12-05T22:33:40.955Z
Learning: In the Atmos CLI Go codebase, using `u.LogErrorAndExit` within completion functions is acceptable because it logs the error and exits the command execution.
Applied to files:
cmd/list/utils.go
📚 Learning: 2025-11-10T03:03:31.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-11-10T03:03:31.505Z
Learning: In the Atmos codebase, commands using the `StandardParser` flag pattern (from pkg/flags) do NOT need explicit `viper.BindPFlag()` calls in their code. The StandardParser encapsulates flag binding internally: flags are registered via `parser.RegisterFlags(cmd)` in init(), and bound via `parser.BindFlagsToViper(cmd, v)` in RunE, which internally calls viper.BindPFlag for each flag. This pattern is used throughout Atmos (e.g., cmd/toolchain/get.go, cmd/toolchain/info.go, cmd/toolchain/install.go, cmd/toolchain/path.go). Do not flag missing viper.BindPFlag calls when StandardParser is used.
Applied to files:
cmd/list/utils.go
🧬 Code graph analysis (1)
cmd/list/utils.go (6)
pkg/config/const.go (1)
IdentityFlagDisabledValue(137-137)pkg/schema/schema.go (2)
AtmosConfiguration(53-94)ConfigAndStacksInfo(621-715)pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManagerWithAtmosConfig(218-269)pkg/list/format/delimited.go (2)
DefaultTSVDelimiter(14-14)DefaultCSVDelimiter(13-13)pkg/config/config.go (1)
InitCliConfig(28-67)pkg/list/errors/types.go (3)
InitConfigError(95-97)ComponentDefinitionNotFoundError(183-185)NoValuesFoundError(9-12)
⏰ 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 (4)
cmd/list/utils.go (4)
3-22: Imports grouping and aliases look consistent.Stdlib, third‑party, and Atmos imports are correctly grouped and alphabetized, and existing aliases (
cfg,u, etc.) are preserved. No changes needed here.
128-164: Identity resolution + normalization are coherent and minimal.The
getIdentityFromCommand+normalizeIdentityValuecombo cleanly handles flag‑over‑env precedence and maps common “falsey” values tocfg.IdentityFlagDisabledValue, which lines up with the auth manager’sshouldDisableAuthbehavior. The inline note about the import cycle versuscmd.GetIdentityFromFlagsis a reasonable justification for this small, local duplication.
166-217: Auth manager creation and config init flow are wired correctly.
createAuthManagerForListis a thin, readable wrapper aroundauth.CreateAndAuthenticateManagerWithAtmosConfig, andinitConfigAndAuthcleanly separates config init errors viaInitConfigErrorwhile still returning a possibly‑nilAuthManagerwhen auth isn’t configured or is disabled. This matches the stated design for list/describe flows and keeps the call sites straightforward.
188-201: CSV delimiter and component filter helpers are straightforward.
setDefaultCSVDelimitercorrectly flips the delimiter when the format is CSV but the delimiter is still the TSV default, andgetComponentFilter’s “first arg or empty string” behavior matches how the list commands use positional component filters.
Brings in recent changes from main including: - Documentation restructure with progressive learning path (#1588) - Updated screengrabs for v1.200.0 (#1832) - Atmos YAML functions documentation updates (#1826) - Auth fixes for describe and list commands (#1827) Resolved file location conflicts: - Moved scaffold-templates.mdx to website/docs/ - Moved template-functions.mdx to website/docs/ (directories were reorganized in main) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
what
atmos describeandatmos listcommandsatmos describe componentcommand--identityflag andATMOS_IDENTITYenv var support to allatmos listcommands that useExecuteDescribeStacksdescribe component,terraform *)describe stacks,describe affected,describe dependents,list *)why
Issue 1: Stack-level default identity was not recognized
When users configured
default: truefor an identity in stack config files (e.g.,stacks/orgs/acme/_defaults.yaml), it was ignored. Users were prompted to select an identity even though one was configured as default.Root cause:
Solution: Two approaches depending on the command type:
Component Auth Merge (for
describe component,terraform *):Stack Loading (for
describe stacks,describe affected,describe dependents,list *):auth.identities.*.default: truewithout processing templatesIssue 2:
atmos listcommands did not use Atmos AuthThe list commands were not creating an AuthManager, meaning they couldn't authenticate even when identities were configured.
Solution:
--identityflag as a persistent flag on the parentlistcommandATMOS_IDENTITYenvironment variable support via Viper bindingcmd/list/utils.go:getIdentityFromCommand: Gets identity from flag or env varcreateAuthManagerForList: Creates AuthManager for list commandsExecuteDescribeStacksto support authentication:list stackslist componentslist values/list varslist metadatalist settingslist instancesCommands that don't use
ExecuteDescribeStacks(workflows, vendor, themes) don't need auth support since they read from config/registry directly.Priority Order (lowest to highest)
atmos.yamldefaults--identity) / environment variable (ATMOS_IDENTITY)Usage
references
docs/fixes/stack-level-default-auth-identity.mdpkg/config/stack_auth_loader.gopkg/auth/manager_helpers.gocmd/list/utils.gotests/fixtures/scenarios/stack-auth-defaults/Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.