Fix authentication in YAML template functions and add --identity flag to describe commands#1742
Merged
Fix authentication in YAML template functions and add --identity flag to describe commands#1742
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. |
|
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 |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
📝 WalkthroughWalkthroughThis PR adds an Changes
Sequence DiagramsequenceDiagram
participant User as CLI User
participant CLI as Describe Command
participant Flag as Flag Parser
participant Auth as Auth Layer
participant Mgr as AuthManager
participant Exec as Executor
participant YAML as YAML Processor
User->>CLI: run describe --identity my-profile
CLI->>Flag: Extract identity from flags
Flag-->>Auth: identity name
Auth->>Auth: Validate auth config exists
Auth->>Mgr: CreateAuthManagerFromIdentity()
Mgr-->>Auth: AuthManager (authenticated)
Auth-->>CLI: AuthManager or error
rect rgb(200, 220, 255)
Note over CLI,YAML: With AuthManager set
CLI->>Exec: ExecuteDescribeComponent(params with AuthManager)
Exec->>Mgr: GetStackInfo() for AuthContext
Mgr-->>Exec: ConfigAndStacksInfo with AuthContext
Exec->>Exec: Populate configAndStacksInfo.AuthContext
Exec->>YAML: Process templates/functions with credentials
YAML-->>Exec: Results with authenticated access
Exec-->>CLI: Component description
end
rect rgb(220, 220, 220)
Note over CLI,YAML: Without AuthManager (backward compat)
CLI->>Exec: ExecuteDescribeComponent(params, AuthManager: nil)
Exec->>YAML: Process templates/functions (no auth)
YAML-->>Exec: Results without credentials
Exec-->>CLI: Component description
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ 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 #1742 +/- ##
==========================================
+ Coverage 69.06% 69.11% +0.05%
==========================================
Files 388 388
Lines 35339 35554 +215
==========================================
+ Hits 24406 24573 +167
- Misses 8665 8705 +40
- Partials 2268 2276 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
osterman
reviewed
Nov 3, 2025
osterman
reviewed
Nov 3, 2025
tests/fixtures/scenarios/authmanager-propagation/components/terraform/test-component/main.tf
Outdated
Show resolved
Hide resolved
… functions Fixed critical nil pointer dereferences in terraform output YAML functions (!terraform.output, !terraform.state) when auth context is nil. The issue was caused by Go's interface nil gotcha where assigning a nil pointer to an interface variable creates a non-nil interface. ## Changes ### Core Fixes - internal/exec/terraform_output_utils.go: - Fixed authMgr declaration to use interface type with conditional assignment - Added auth package import - Added defer perf.Track() calls to all authContextWrapper methods - internal/exec/template_funcs_component.go: - Applied same nil interface fix for authMgr variable - Added auth package import ### Code Review Fixes - cmd/describe_component.go: - Simplified to use CreateAuthManagerFromIdentity helper - Removed duplicated auth manager creation logic - Removed unused imports (context, auth, credentials, validation) - internal/exec/describe_dependents.go: - Thread AuthManager through ExecuteDescribeStacks call (line 148) - Thread AuthManager through ExecuteDescribeComponent call (line 160) - internal/exec/describe_stacks_authmanager_propagation_test.go: - Changed .AnyTimes() to .Times(1) in mock expectations ### Test Coverage - tests/cli_describe_identity_test.go (NEW): - CLI integration tests for describe commands with --identity flag - Verifies no interactive selector shown with explicit identity - Tests backward compatibility without --identity flag - internal/exec/terraform_output_authcontext_wrapper_test.go (NEW): - Unit tests for authContextWrapper - Tests wrapper creation and GetStackInfo method - Verifies all stub methods panic as expected - Uses context.TODO() instead of nil per linter requirements ### Documentation - website/blog/2025-11-02-describe-commands-identity-flag.mdx: - Fixed MDX syntax error: `<100ms` → backticks - Fixed broken links to /cli/commands/auth/usage - Fixed template functions links to /functions/yaml - website/docs/cli/commands/describe/*.mdx (4 files): - Updated authentication links to correct URL ### Test Results - ✅ terraform_output_function - PASSING - ✅ terraform_output_function_(no_tty) - PASSING - ✅ All AuthManager tests - PASSING - ✅ All new CLI integration tests - PASSING - ✅ Code compiles successfully - ✅ Website builds successfully Fixes #1742 (CodeRabbit review comments) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ix-auth-terraform-state
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
internal/exec/terraform_output_authcontext_wrapper_test.go (1)
57-170: Consider refactoring to a table-driven test.This test validates that all stub methods panic, which is good defensive testing. However, the repetitive pattern across 17 sub-tests could be consolidated using a table-driven approach for better maintainability.
Here's how you could refactor this:
- t.Run("GetCachedCredentials panics", func(t *testing.T) { - assert.Panics(t, func() { - _, _ = wrapper.GetCachedCredentials(context.TODO(), "test") - }, "GetCachedCredentials should panic") - }) - - t.Run("Authenticate panics", func(t *testing.T) { - assert.Panics(t, func() { - _, _ = wrapper.Authenticate(context.TODO(), "test") - }, "Authenticate should panic") - }) - - // ... (rest of the repetitive tests) + tests := []struct { + name string + fn func() + }{ + {"GetCachedCredentials", func() { _, _ = wrapper.GetCachedCredentials(context.TODO(), "test") }}, + {"Authenticate", func() { _, _ = wrapper.Authenticate(context.TODO(), "test") }}, + {"Whoami", func() { _, _ = wrapper.Whoami(context.TODO(), "test") }}, + {"Validate", func() { _ = wrapper.Validate() }}, + {"GetDefaultIdentity", func() { _, _ = wrapper.GetDefaultIdentity(false) }}, + {"ListProviders", func() { _ = wrapper.ListProviders() }}, + {"Logout", func() { _ = wrapper.Logout(context.TODO(), "test") }}, + {"GetChain", func() { _ = wrapper.GetChain() }}, + {"ListIdentities", func() { _ = wrapper.ListIdentities() }}, + {"GetProviderForIdentity", func() { _ = wrapper.GetProviderForIdentity("test") }}, + {"GetFilesDisplayPath", func() { _ = wrapper.GetFilesDisplayPath("test") }}, + {"GetProviderKindForIdentity", func() { _, _ = wrapper.GetProviderKindForIdentity("test") }}, + {"GetIdentities", func() { _ = wrapper.GetIdentities() }}, + {"GetProviders", func() { _ = wrapper.GetProviders() }}, + {"LogoutProvider", func() { _ = wrapper.LogoutProvider(context.TODO(), "test") }}, + {"LogoutAll", func() { _ = wrapper.LogoutAll(context.TODO()) }}, + {"GetEnvironmentVariables", func() { _, _ = wrapper.GetEnvironmentVariables("test") }}, + {"PrepareShellEnvironment", func() { _, _ = wrapper.PrepareShellEnvironment(context.TODO(), "test", nil) }}, + } + + for _, tt := range tests { + t.Run(tt.name+" panics", func(t *testing.T) { + assert.Panics(t, tt.fn, tt.name+" should panic") + }) + }This approach makes it easier to add or remove methods and follows the table-driven test pattern recommended in the coding guidelines.
internal/exec/terraform_output_utils.go (1)
66-198: Consider returning errors instead of panicking in stub methods.The
authContextWrapperimplements theAuthManagerinterface with most methods panicking. While this works for the current code path (as confirmed by testing), panics are fragile—if future changes inadvertently call any stub method, the process crashes at runtime instead of returning a graceful error.Consider returning meaningful errors (e.g.,
errors.New("method not supported by authContextWrapper")) instead of panicking. This maintains the same contract while providing safer failure modes.Apply this pattern to stub methods:
func (a *authContextWrapper) GetCachedCredentials(ctx context.Context, identityName string) (*auth_types.WhoamiInfo, error) { defer perf.Track(nil, "exec.authContextWrapper.GetCachedCredentials")() - panic("authContextWrapper.GetCachedCredentials should not be called") + return nil, errors.New("authContextWrapper does not support GetCachedCredentials") }tests/cli_describe_identity_test.go (1)
107-143: LGTM!These tests properly verify backward compatibility—commands work without the identity flag. Error assertions are correct.
Optional: The atmosRunner initialization (lines 109-114) is duplicated from the first test function. Consider extracting to a package-level
TestMainor helper function if this pattern continues across more test 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 selected for processing (15)
cmd/describe_component.go(3 hunks)internal/exec/describe_dependents.go(5 hunks)internal/exec/describe_stacks_authmanager_propagation_test.go(1 hunks)internal/exec/template_funcs.go(1 hunks)internal/exec/template_funcs_component.go(4 hunks)internal/exec/template_funcs_component_test.go(3 hunks)internal/exec/terraform_output_authcontext_wrapper_test.go(1 hunks)internal/exec/terraform_output_utils.go(3 hunks)tests/cli_describe_identity_test.go(1 hunks)tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden(1 hunks)website/blog/2025-11-02-describe-commands-identity-flag.mdx(1 hunks)website/docs/cli/commands/describe/describe-affected.mdx(3 hunks)website/docs/cli/commands/describe/describe-component.mdx(3 hunks)website/docs/cli/commands/describe/describe-dependents.mdx(3 hunks)website/docs/cli/commands/describe/describe-stacks.mdx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- website/docs/cli/commands/describe/describe-affected.mdx
- internal/exec/template_funcs_component.go
- internal/exec/describe_stacks_authmanager_propagation_test.go
- website/docs/cli/commands/describe/describe-dependents.mdx
- website/docs/cli/commands/describe/describe-component.mdx
🧰 Additional context used
📓 Path-based instructions (5)
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
Files:
internal/exec/terraform_output_authcontext_wrapper_test.gotests/cli_describe_identity_test.gointernal/exec/template_funcs_component_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
Files:
internal/exec/terraform_output_authcontext_wrapper_test.gotests/cli_describe_identity_test.gocmd/describe_component.gointernal/exec/terraform_output_utils.gointernal/exec/template_funcs_component_test.gointernal/exec/describe_dependents.gointernal/exec/template_funcs.go
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use cases
Files:
website/docs/cli/commands/describe/describe-stacks.mdxwebsite/blog/2025-11-02-describe-commands-identity-flag.mdx
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands
Implement each CLI command in a separate file under cmd/
Use Viper for managing configuration, environment variables, and flags in the CLI
Keep separation of concerns between CLI interface (cmd) and business logic
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in Cobra command help
Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Follow single responsibility; separate command interface from business logic
Provide meaningful user feedback and include progress indicators for long-running operations
Provide clear error messages to users and troubleshooting hints where appropriate
Files:
cmd/describe_component.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
cmd/describe_component.gointernal/exec/terraform_output_utils.gointernal/exec/describe_dependents.gointernal/exec/template_funcs.go
🧠 Learnings (49)
📓 Common learnings
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: 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.
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`.
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-12-07T16:19:01.683Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: In `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necessary.
Applied to files:
internal/exec/terraform_output_authcontext_wrapper_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Applied to files:
internal/exec/terraform_output_authcontext_wrapper_test.gotests/cli_describe_identity_test.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:
tests/cli_describe_identity_test.gocmd/describe_component.gointernal/exec/terraform_output_utils.gowebsite/blog/2025-11-02-describe-commands-identity-flag.mdx
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
tests/cli_describe_identity_test.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:
tests/cli_describe_identity_test.gowebsite/docs/cli/commands/describe/describe-stacks.mdxinternal/exec/terraform_output_utils.gowebsite/blog/2025-11-02-describe-commands-identity-flag.mdx
📚 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:
tests/cli_describe_identity_test.gocmd/describe_component.gointernal/exec/terraform_output_utils.gointernal/exec/describe_dependents.gowebsite/blog/2025-11-02-describe-commands-identity-flag.mdx
📚 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:
tests/cli_describe_identity_test.gowebsite/docs/cli/commands/describe/describe-stacks.mdxcmd/describe_component.gointernal/exec/terraform_output_utils.gointernal/exec/describe_dependents.gointernal/exec/template_funcs.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:
tests/cli_describe_identity_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Use table-driven tests for multiple scenarios
Applied to files:
tests/cli_describe_identity_test.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:
tests/cli_describe_identity_test.gowebsite/docs/cli/commands/describe/describe-stacks.mdxwebsite/blog/2025-11-02-describe-commands-identity-flag.mdx
📚 Learning: 2025-01-09T22:27:25.538Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/validate_stacks.go:20-23
Timestamp: 2025-01-09T22:27:25.538Z
Learning: The validate commands in Atmos can have different help handling implementations. Specifically, validate_component.go and validate_stacks.go are designed to handle help requests differently, with validate_stacks.go including positional argument checks while validate_component.go does not.
Applied to files:
tests/cli_describe_identity_test.gowebsite/docs/cli/commands/describe/describe-stacks.mdxcmd/describe_component.go
📚 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:
website/docs/cli/commands/describe/describe-stacks.mdx
📚 Learning: 2025-01-19T15:49:15.593Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Applied to files:
website/docs/cli/commands/describe/describe-stacks.mdx
📚 Learning: 2025-01-19T22:30:27.600Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-01-19T22:30:27.600Z
Learning: The Atmos YAML function `!env` is used to retrieve environment variables and assign them to sections in stack manifests. It supports both simple types (string, number, boolean) and complex types (JSON-encoded lists, maps, objects).
Applied to files:
website/docs/cli/commands/describe/describe-stacks.mdx
📚 Learning: 2025-01-19T22:30:27.600Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-01-19T22:30:27.600Z
Learning: The Atmos YAML function `!include` allows downloading local or remote files from different sources and assigning their contents to sections in stack manifests. It supports various protocols (file, http, git, s3, etc.) and can filter content using YQ expressions.
Applied to files:
website/docs/cli/commands/describe/describe-stacks.mdx
📚 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:
website/docs/cli/commands/describe/describe-stacks.mdxtests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden
📚 Learning: 2024-12-03T05:18:49.169Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2024-12-03T05:18:49.169Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
Applied to files:
website/docs/cli/commands/describe/describe-stacks.mdxinternal/exec/terraform_output_utils.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/describe_component.gointernal/exec/terraform_output_utils.gointernal/exec/describe_dependents.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_component.gointernal/exec/terraform_output_utils.gointernal/exec/template_funcs_component_test.gointernal/exec/describe_dependents.gointernal/exec/template_funcs.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_component.gointernal/exec/terraform_output_utils.gointernal/exec/template_funcs_component_test.gointernal/exec/describe_dependents.gointernal/exec/template_funcs.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:
cmd/describe_component.gointernal/exec/template_funcs_component_test.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_component.gointernal/exec/terraform_output_utils.gointernal/exec/template_funcs_component_test.gointernal/exec/describe_dependents.gointernal/exec/template_funcs.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands
Applied to files:
cmd/describe_component.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Provide comprehensive help text for all commands and flags
Applied to files:
cmd/describe_component.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Provide clear error messages to users and troubleshooting hints where appropriate
Applied to files:
cmd/describe_component.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Include examples in Cobra command help
Applied to files:
cmd/describe_component.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Viper for managing configuration, environment variables, and flags in the CLI
Applied to files:
cmd/describe_component.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_component.gointernal/exec/terraform_output_utils.gointernal/exec/describe_dependents.go
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden
📚 Learning: 2025-02-14T23:12:38.030Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1061
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:8-8
Timestamp: 2025-02-14T23:12:38.030Z
Learning: Test snapshots in the Atmos project, particularly for dry run scenarios, may be updated during the development process, and temporary inconsistencies in their content should not be flagged as issues.
Applied to files:
tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden
📚 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/terraform_output_utils.gointernal/exec/template_funcs_component_test.gointernal/exec/template_funcs.go
📚 Learning: 2025-10-03T18:02:08.535Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: internal/exec/terraform.go:269-272
Timestamp: 2025-10-03T18:02:08.535Z
Learning: In internal/exec/terraform.go, when auth.TerraformPreHook fails, the error is logged but execution continues. This is a deliberate design choice to allow Terraform commands to proceed even if authentication setup fails, rather than failing fast.
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2024-12-03T03:52:02.524Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:144-145
Timestamp: 2024-12-03T03:52:02.524Z
Learning: Avoid adding context timeouts to Terraform commands in `execTerraformOutput` because their execution time can vary from seconds to hours, and making it configurable would require redesigning the command interface.
Applied to files:
internal/exec/terraform_output_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:
internal/exec/terraform_output_utils.gointernal/exec/template_funcs_component_test.gointernal/exec/describe_dependents.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:
internal/exec/terraform_output_utils.gointernal/exec/describe_dependents.go
📚 Learning: 2024-10-28T01:51:30.811Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2025-02-06T13:38:07.216Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 984
File: internal/exec/copy_glob.go:0-0
Timestamp: 2025-02-06T13:38:07.216Z
Learning: The `u.LogTrace` function in the `cloudposse/atmos` repository accepts `atmosConfig` as its first parameter, followed by the message string.
Applied to files:
internal/exec/terraform_output_utils.gointernal/exec/describe_dependents.go
📚 Learning: 2024-12-03T05:29:07.718Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:145-146
Timestamp: 2024-12-03T05:29:07.718Z
Learning: In the Atmos project, a 5-minute timeout in the `execTerraformOutput` function is acceptable for retrieving `terraform output` for a component in a stack.
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2024-11-12T03:16:02.910Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
internal/exec/template_funcs_component_test.gointernal/exec/template_funcs.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/template_funcs_component_test.gointernal/exec/template_funcs.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:
internal/exec/template_funcs_component_test.gointernal/exec/describe_dependents.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/template_funcs_component_test.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/template_funcs_component_test.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:
internal/exec/describe_dependents.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:
internal/exec/describe_dependents.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:
internal/exec/describe_dependents.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:
internal/exec/describe_dependents.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:
website/blog/2025-11-02-describe-commands-identity-flag.mdx
🧬 Code graph analysis (5)
internal/exec/terraform_output_authcontext_wrapper_test.go (1)
pkg/schema/schema.go (2)
AuthContext(498-506)AWSAuthContext(510-525)
tests/cli_describe_identity_test.go (2)
tests/testhelpers/atmos_runner.go (1)
NewAtmosRunner(31-35)pkg/logger/log.go (1)
Info(34-36)
cmd/describe_component.go (4)
pkg/config/config.go (1)
InitCliConfig(25-62)pkg/schema/schema.go (1)
ConfigAndStacksInfo(527-610)errors/errors.go (1)
ErrFailedToInitConfig(285-285)cmd/identity_flag.go (2)
GetIdentityFromFlags(38-61)CreateAuthManagerFromIdentity(115-156)
internal/exec/terraform_output_utils.go (3)
pkg/schema/schema.go (3)
ConfigAndStacksInfo(527-610)Context(409-424)AuthContext(498-506)internal/exec/describe_component.go (2)
ExecuteDescribeComponent(212-228)ExecuteDescribeComponentParams(202-209)pkg/describe/describe_component.go (1)
ExecuteDescribeComponent(9-26)
internal/exec/describe_dependents.go (2)
internal/exec/describe_component.go (2)
ExecuteDescribeComponent(212-228)ExecuteDescribeComponentParams(202-209)pkg/describe/describe_component.go (1)
ExecuteDescribeComponent(9-26)
🪛 LanguageTool
website/blog/2025-11-02-describe-commands-identity-flag.mdx
[grammar] ~233-~233: Please add a punctuation mark at the end of paragraph.
Context: ... describe operations with authenticated access ### CI/CD Considerations **For CI/CD ...
(PUNCTUATION_PARAGRAPH_END)
[style] ~273-~273: Using many exclamation marks might seem excessive (in this case: 9 exclamation marks for a text that’s 5718 characters long)
Context: ...entials from context This ensures that !terraform.state and !terraform.output functions can access r...
(EN_EXCESSIVE_EXCLAMATION)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build (windows)
- GitHub Check: Build (linux)
- GitHub Check: Review Dependency Licenses
- GitHub Check: autofix
- GitHub Check: website-deploy-preview
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (16)
tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden (1)
10-10: Skip commenting on trailing whitespace.This is a single trailing space difference in a test snapshot, likely an artifact of regenerating the snapshot during authentication flow testing. No functional impact.
internal/exec/terraform_output_authcontext_wrapper_test.go (3)
1-11: Imports and package declaration look good.Standard test setup with appropriate dependencies.
13-34: Test coverage for wrapper creation is solid.Both nil and valid cases are tested appropriately.
36-55: GetStackInfo test validates proper context propagation.Field-level checks ensure the authContext is correctly accessible.
website/docs/cli/commands/describe/describe-stacks.mdx (3)
27-29: Clear guidance on authentication requirements.The info box effectively communicates when
--identityis needed and how to disable function processing. This helps users understand the default behavior and available options.
57-61: Examples cover both authentication modes.Good inclusion of both interactive and explicit identity usage patterns. The examples demonstrate real-world usage scenarios effectively.
100-101: Comprehensive flag documentation.The
--identityflag documentation clearly explains when it's required, provides both usage modes, and links to detailed auth docs. Well done.website/blog/2025-11-02-describe-commands-identity-flag.mdx (1)
1-306: Comprehensive feature announcement.This blog post effectively communicates the new feature with clear problem statement, solution explanation, practical examples, and migration guidance. The technical details section provides helpful context on implementation and performance characteristics. The backward compatibility emphasis and error handling explanations are particularly valuable for users.
internal/exec/describe_dependents.go (2)
31-31: AuthManager properly integrated into the flow.The AuthManager field additions to both structs and the forwarding at line 92 correctly thread authentication through the describe dependents pipeline. Clean implementation.
Also applies to: 43-43, 92-92
148-161: Past review concern fully resolved.Both call sites now correctly pass
args.AuthManagerinstead ofnil. The ExecuteDescribeStacks call at line 148 and the ExecuteDescribeComponent parameter struct at line 160 properly propagate the AuthManager, ensuring YAML template functions have access to authenticated credentials.cmd/describe_component.go (1)
74-100: Auth flow properly centralized.The code now uses
CreateAuthManagerFromIdentityto handle identity→AuthManager setup, ensuring consistent validation across describe commands. The AuthManager is correctly threaded through toExecuteDescribeComponentvia the params struct, enabling authenticated credential access for YAML template functions like!terraform.stateand!terraform.output.internal/exec/template_funcs_component_test.go (1)
59-81: Test signatures correctly updated.The test calls now include the additional
configAndStacksInfoparameter (passed asnil), matching the updatedcomponentFuncsignature. This aligns with the broader auth context propagation refactor while keeping tests focused on their specific scenarios.internal/exec/template_funcs.go (1)
46-46: Context propagation established.The
configAndStacksInfois now threaded through tocomponentFunc, enabling downstream component resolution to access authentication context when template functions are evaluated.internal/exec/terraform_output_utils.go (1)
550-564: Auth context propagation fixed.The code now creates an
authContextWrapperfromauthContextwhen available and passes it toExecuteDescribeComponent, resolving the critical bug where YAML template functions couldn't access authenticated credentials. This directly addresses the past review concern about passingnilAuthManager.tests/cli_describe_identity_test.go (2)
16-22: LGTM!The atmosRunner initialization is properly handled with graceful skip on build failure and clear logging.
92-104: LGTM!This test properly verifies the command succeeds without the identity flag and correctly asserts the expected behavior.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/cli_describe_identity_test.go (2)
15-22: Consider extracting duplicate atmosRunner initialization.Both test functions have identical atmosRunner initialization logic. Extract to a helper function or use TestMain to reduce duplication:
func ensureAtmosRunner(t *testing.T) { if atmosRunner == nil { atmosRunner = testhelpers.NewAtmosRunner(coverDir) if err := atmosRunner.Build(); err != nil { t.Skipf("Failed to initialize Atmos: %v", err) } logger.Info("Atmos runner initialized", "coverageEnabled", coverDir != "") } }Then call
ensureAtmosRunner(t)at the start of each test function.Also applies to: 85-90
92-118: Consider table-driven tests for consistency.These two tests follow the same pattern and could be combined using a table-driven approach, matching the style used in the first test function:
func TestDescribeCommandsWithoutAuthWork(t *testing.T) { ensureAtmosRunner(t) tests := []struct { name string args []string }{ { name: "describe stacks without auth should work", args: []string{"describe", "stacks"}, }, { name: "list components without auth should work", args: []string{"list", "components"}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Chdir("fixtures/scenarios/basic") cmd := atmosRunner.Command(tt.args...) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr err := cmd.Run() assert.NoError(t, err, tt.name) }) } }
📜 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)
tests/cli_describe_identity_test.go(1 hunks)tests/fixtures/scenarios/authmanager-propagation/stacks/test.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/fixtures/scenarios/authmanager-propagation/stacks/test.yaml
🧰 Additional context used
📓 Path-based instructions (2)
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
Files:
tests/cli_describe_identity_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
Files:
tests/cli_describe_identity_test.go
🧠 Learnings (15)
📓 Common learnings
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: 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: 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`.
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: 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.
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.
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.
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.
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, ...).
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
tests/cli_describe_identity_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:
tests/cli_describe_identity_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Use table-driven tests for multiple scenarios
Applied to files:
tests/cli_describe_identity_test.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:
tests/cli_describe_identity_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Applied to files:
tests/cli_describe_identity_test.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:
tests/cli_describe_identity_test.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:
tests/cli_describe_identity_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:
tests/cli_describe_identity_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:
tests/cli_describe_identity_test.go
📚 Learning: 2025-02-19T05:50:35.853Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden:0-0
Timestamp: 2025-02-19T05:50:35.853Z
Learning: Backtick formatting should only be applied to flag descriptions in Go source files, not in golden test files (test snapshots) as they are meant to capture the raw command output.
Applied to files:
tests/cli_describe_identity_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Include integration tests for command flows and end-to-end CLI where possible
Applied to files:
tests/cli_describe_identity_test.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:
tests/cli_describe_identity_test.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:
tests/cli_describe_identity_test.go
📚 Learning: 2025-01-09T22:27:25.538Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/validate_stacks.go:20-23
Timestamp: 2025-01-09T22:27:25.538Z
Learning: The validate commands in Atmos can have different help handling implementations. Specifically, validate_component.go and validate_stacks.go are designed to handle help requests differently, with validate_stacks.go including positional argument checks while validate_component.go does not.
Applied to files:
tests/cli_describe_identity_test.go
🧬 Code graph analysis (1)
tests/cli_describe_identity_test.go (2)
tests/testhelpers/atmos_runner.go (1)
NewAtmosRunner(31-35)pkg/logger/log.go (2)
Info(34-36)Error(54-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build (linux)
- GitHub Check: Build (windows)
- GitHub Check: Build (macos)
- GitHub Check: Analyze (go)
- GitHub Check: website-deploy-preview
- GitHub Check: autofix
- GitHub Check: Review Dependency Licenses
- GitHub Check: Summary
🔇 Additional comments (1)
tests/cli_describe_identity_test.go (1)
24-66: Solid test coverage for identity flag behavior.The table-driven tests properly verify that:
- Commands fail gracefully with non-existent identity
- No interactive selector appears when explicit identity is provided
- Error assertions are present (addressing the previous review)
The test coverage aligns well with the PR objectives for identity flag support.
osterman
approved these changes
Nov 3, 2025
|
These changes were released in v1.197.0. |
11 tasks
osterman
pushed a commit
that referenced
this pull request
Nov 13, 2025
These unit tests were written before authentication was added to describe commands and need to explicitly opt out of authentication to work correctly. ## Problem Tests TestDescribeAffected, TestDescribeDependents, and TestDescribeStacksRunnable were failing with "authentication not configured in atmos.yaml" error. These tests were written in June 2025 by Sam Tholiya before authentication existed. When authentication was added to describe commands in PR #1742 (Nov 3, 2025), these existing unit tests were not updated and began failing. ## Root Cause Some tests bind viper to ATMOS_IDENTITY environment variable, and viper retains this state across tests. When tests that set ATMOS_IDENTITY run first, subsequent tests inherit this value through viper, causing authentication validation to run even though the tests didn't configure authentication. ## Solution Explicitly disable authentication in each test by: - Setting ATMOS_IDENTITY=false via t.Setenv() to set environment variable - Setting viper.Set("identity", "false") to ensure viper recognizes the disabled state - This causes the identity value to be normalized to "__DISABLED__", which skips authentication validation Also added empty auth: section to basic fixture to document that authentication is not configured. ## Changes - cmd/describe_affected_test.go: Add authentication disablement - cmd/describe_dependents_test.go: Add authentication disablement - cmd/describe_stacks_test.go: Add authentication disablement - tests/fixtures/scenarios/basic/atmos.yaml: Add empty auth section with comment These are unit tests that mock the execution layer, so they don't need real authentication - they just need to opt out of it explicitly now that authentication is part of the command flow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Nov 20, 2025
This was referenced Nov 29, 2025
This was referenced Dec 15, 2025
This was referenced Dec 22, 2025
This was referenced Jan 17, 2026
This was referenced Feb 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
!terraform.stateand!terraform.outputYAML template functions failed to access authenticated credentials when using the--identityflag--identityflag support to allatmos describecommands (describe stacks,describe component,describe affected,describe dependents)AuthContextfromAuthManagerthrough the entire component description pipeline, enabling template functions to access authenticated credentials for remote state backends (S3, Azure Blob, GCS)--identityis used without auth configured inatmos.yamlExecuteDescribeComponentto use parameter struct pattern for cleaner APIwhy
Problem 1: Template Functions Authentication Failure
Users reported that
!terraform.stateand!terraform.outputtemplate functions failed with "context deadline exceeded" errors when using the--identityflag:The workaround of running
atmos terraform outputworked fine with the same identity, indicating the credentials were being authenticated but not propagated to template function processing.Root Cause: The authentication context was not being threaded from
AuthManagerto the YAML function processors. WhenExecuteDescribeComponentwas called, it didn't receive theAuthManager, soconfigAndStacksInfo.AuthContextremainednil, causing template functions to fail when accessing remote state.Problem 2: Missing Identity Support for Describe Commands
The
atmos describefamily of commands (describe stacks,describe component,describe affected,describe dependents) can execute YAML template functions that require authentication, but had no way to authenticate at runtime. Users were forced to:atmos auth login --identity <identity>before describe commandsProblem 3: Confusing Error Messages
When users tried to use
--identitywithout configuring theauthsection inatmos.yaml, they received confusing authentication errors instead of clear guidance.Solution:
Thread AuthContext Through Pipeline:
stackInfowithAuthContextbefore creatingAuthManagerAuthManagerthroughExecuteDescribeComponentand related functionsAuthContextfromAuthManagerand populateconfigAndStacksInfostackInfo.AuthContextAdd --identity Flag to Describe Commands:
--identityas aPersistentFlagto parentdescribecommand (auto-inherits to all subcommands)--identity my-identity--identity(no value)-i my-identityAuth Configuration Validation:
--identityflag is provided--identity(backward compatible)Business Impact:
references
Technical Documentation:
docs/prd/terraform-template-functions-auth-context.md- Details the authentication propagation fixdocs/prd/describe-commands-identity-flag.md- Documents the --identity flag feature for describe commandsdocs/test-review-auth-identity.md- Comprehensive test coverage analysisBlog Post:
website/blog/2025-11-02-describe-commands-identity-flag.mdx- User-facing announcement of the new --identity flag featureKey Changes:
Authentication Context Propagation (
terraform-template-functions-auth-context.md):cmd/cmd_utils.goto createstackInfobeforeAuthManagerExecuteDescribeComponentto acceptAuthManagervia parameter structAuthContextpropagation inExecuteDescribeComponentWithContextAuthManagerDescribe Commands Identity Flag (
describe-commands-identity-flag.md):--identityflag tocmd/describe.goasPersistentFlagCreateAuthManagerFromIdentityfor centralized auth logicTesting:
ExecuteDescribeComponentwith AuthManager--identityDocumentation:
--identityflagSummary by CodeRabbit
Release Notes
New Features
--identityflag to describe commands for authenticating YAML template functions that require access to remote resourcesDocumentation