-
-
Notifications
You must be signed in to change notification settings - Fork 140
fix: Propagate auth context through nested !terraform.state functions
#1786
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 ReviewThe following issues were found:
License Issuesgo.mod
Scanned Files
|
|
@coderabbitai full review please |
|
@aknysh: I'll perform a full review of all the changes in this PR. ✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughImplements AuthManager propagation through nested Terraform YAML function evaluations, adds per-component auth resolution helpers, threads Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Atmos as Atmos (terraform.go)
participant AuthMgr as AuthManager
participant Info as ConfigAndStacksInfo
participant YAML as YAML Func (!terraform.state / !terraform.output)
participant Getter as StateGetter / OutputGetter
participant Desc as ExecuteDescribeComponent
User->>Atmos: run command
Atmos->>AuthMgr: create & authenticate
AuthMgr-->>Info: store in info.AuthManager
Atmos->>Desc: execute top-level component (info includes AuthManager)
Desc->>YAML: evaluate !terraform.state / !terraform.output
YAML->>Getter: GetState/GetOutput(..., authContext, authManager)
Getter->>AuthMgr: resolve nested component auth (resolveAuthManagerForNestedComponent)
alt component defines override
Getter->>AuthMgr: create component-specific AuthManager
note right of AuthMgr: merged component+global identities
else inherit
Getter->>AuthMgr: use parent AuthManager
end
Getter->>Desc: execute nested component with resolved AuthManager
Desc-->>YAML: return nested value
YAML-->>Atmos: value substituted
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/exec/describe_component_nested_authmanager_test.go (1)
14-89: Excellent coverage of the core 3-level nesting scenario.The test structure is solid with clear documentation. However, this test and the next two (lines 91-182) could be refactored to use the
setupNestedAuthTesthelper introduced at line 297 to reduce duplication and improve consistency with scenarios 2-5.Consider refactoring to use the helper:
func TestNestedAuthManagerPropagation(t *testing.T) { - ctrl := gomock.NewController(t) + ctrl, mockAuthManager := setupNestedAuthTest(t, "test-nested-identity", "us-east-1") defer ctrl.Finish() - - // Setup: Create AuthContext with AWS credentials. - expectedAuthContext := &schema.AuthContext{ - AWS: &schema.AWSAuthContext{ - Profile: "test-nested-identity", - Region: "us-east-1", - CredentialsFile: "/tmp/test-nested-creds", - ConfigFile: "/tmp/test-nested-config", - }, - } - - // Create stackInfo with AuthContext (what AuthManager.Authenticate() populates). - authStackInfo := &schema.ConfigAndStacksInfo{ - AuthContext: expectedAuthContext, - Stack: "test", - } - - // Create mock AuthManager that returns our authStackInfo. - mockAuthManager := types.NewMockAuthManager(ctrl) - mockAuthManager.EXPECT(). - GetStackInfo(). - Return(authStackInfo). - AnyTimes() - - // Use the nested propagation fixture. - workDir := "../../tests/fixtures/scenarios/authmanager-nested-propagation" - t.Chdir(workDir)Note: The helper doesn't set
CredentialsFileandConfigFile, but those fields don't appear to be verified in assertions, so the refactor should be safe.
📜 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 (30)
docs/fixes/nested-terraform-state-auth-context-propagation.md(1 hunks)docs/terraform-output-yaml-func-authentication-flow.md(2 hunks)docs/terraform-state-yaml-func-authentication-flow.md(2 hunks)errors/errors.go(1 hunks)examples/quick-start-advanced/Dockerfile(1 hunks)go.mod(5 hunks)internal/exec/describe_component_auth_override_test.go(1 hunks)internal/exec/describe_component_authmanager_propagation_test.go(1 hunks)internal/exec/describe_component_nested_authmanager_test.go(1 hunks)internal/exec/mock_terraform_output_getter.go(1 hunks)internal/exec/mock_terraform_state_getter.go(1 hunks)internal/exec/terraform.go(1 hunks)internal/exec/terraform_nested_auth_helper.go(1 hunks)internal/exec/terraform_output_getter.go(2 hunks)internal/exec/terraform_output_utils.go(5 hunks)internal/exec/terraform_state_getter.go(2 hunks)internal/exec/terraform_state_utils.go(4 hunks)internal/exec/yaml_func_terraform_output.go(1 hunks)internal/exec/yaml_func_terraform_output_authcontext_test.go(4 hunks)internal/exec/yaml_func_terraform_state.go(1 hunks)internal/exec/yaml_func_terraform_state_authcontext_test.go(3 hunks)pkg/hooks/store_cmd.go(2 hunks)pkg/hooks/store_cmd_nil_handling_test.go(9 hunks)pkg/hooks/store_cmd_test.go(1 hunks)pkg/schema/schema.go(1 hunks)tests/fixtures/scenarios/authmanager-nested-propagation/README.md(1 hunks)tests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml(1 hunks)tests/fixtures/scenarios/authmanager-nested-propagation/stacks/test.yaml(1 hunks)tests/fixtures/scenarios/authmanager-propagation/stacks/test.yaml(1 hunks)website/docs/integrations/atlantis.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
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/integrations/atlantis.mdx
**/*.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:
errors/errors.gointernal/exec/terraform_output_utils.gointernal/exec/terraform.gointernal/exec/describe_component_auth_override_test.gointernal/exec/yaml_func_terraform_output_authcontext_test.gointernal/exec/mock_terraform_output_getter.gointernal/exec/terraform_nested_auth_helper.gopkg/hooks/store_cmd_nil_handling_test.gointernal/exec/terraform_state_utils.gointernal/exec/mock_terraform_state_getter.gointernal/exec/yaml_func_terraform_output.gointernal/exec/yaml_func_terraform_state_authcontext_test.gointernal/exec/yaml_func_terraform_state.gointernal/exec/terraform_output_getter.gointernal/exec/describe_component_authmanager_propagation_test.gointernal/exec/terraform_state_getter.gointernal/exec/describe_component_nested_authmanager_test.gopkg/schema/schema.gopkg/hooks/store_cmd_test.gopkg/hooks/store_cmd.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
errors/errors.gointernal/exec/terraform_output_utils.gointernal/exec/terraform.gointernal/exec/mock_terraform_output_getter.gointernal/exec/terraform_nested_auth_helper.gointernal/exec/terraform_state_utils.gointernal/exec/mock_terraform_state_getter.gointernal/exec/yaml_func_terraform_output.gointernal/exec/yaml_func_terraform_state.gointernal/exec/terraform_output_getter.gointernal/exec/terraform_state_getter.gopkg/schema/schema.gopkg/hooks/store_cmd.go
**/*_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/describe_component_auth_override_test.gointernal/exec/yaml_func_terraform_output_authcontext_test.gopkg/hooks/store_cmd_nil_handling_test.gointernal/exec/yaml_func_terraform_state_authcontext_test.gointernal/exec/describe_component_authmanager_propagation_test.gointernal/exec/describe_component_nested_authmanager_test.gopkg/hooks/store_cmd_test.go
go.{mod,sum}
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
go.{mod,sum}: Manage dependencies with Go modules
Keep dependencies up to date
Files:
go.mod
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/hooks/store_cmd_nil_handling_test.gopkg/schema/schema.gopkg/hooks/store_cmd_test.gopkg/hooks/store_cmd.go
🧠 Learnings (61)
📓 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: 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.
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
📚 Learning: 2025-09-10T17:34:52.568Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/providers/github/oidc.go:96-100
Timestamp: 2025-09-10T17:34:52.568Z
Learning: The ATMOS_ environment variable binding guideline applies to Atmos configuration variables, not external service-required environment variables like GitHub Actions OIDC variables (GITHUB_ACTIONS, ACTIONS_ID_TOKEN_*) which must use their standard names.
Applied to files:
website/docs/integrations/atlantis.mdxtests/fixtures/scenarios/authmanager-nested-propagation/atmos.yamlexamples/quick-start-advanced/Dockerfile
📚 Learning: 2024-11-23T00:13:22.004Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 801
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-23T00:13:22.004Z
Learning: When updating the `ATMOS_VERSION` in Dockerfiles, the team prefers to pin to the next future version when the PR merges, even if the version is not yet released.
Applied to files:
website/docs/integrations/atlantis.mdxexamples/quick-start-advanced/Dockerfile
📚 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:
website/docs/integrations/atlantis.mdxtests/fixtures/scenarios/authmanager-nested-propagation/atmos.yamlexamples/quick-start-advanced/Dockerfile
📚 Learning: 2024-11-12T03:15:15.627Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-12T03:15:15.627Z
Learning: It is acceptable to set `ARG ATMOS_VERSION` to a future version like `1.105.0` in `examples/quick-start-advanced/Dockerfile` if that will be the next release.
Applied to files:
website/docs/integrations/atlantis.mdxexamples/quick-start-advanced/Dockerfile
📚 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:
website/docs/integrations/atlantis.mdxtests/fixtures/scenarios/authmanager-nested-propagation/atmos.yamlexamples/quick-start-advanced/Dockerfile
📚 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/integrations/atlantis.mdxexamples/quick-start-advanced/Dockerfile
📚 Learning: 2024-12-12T15:15:46.457Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos-cli-imports/atmos.yaml:7-7
Timestamp: 2024-12-12T15:15:46.457Z
Learning: In example configuration files, such as `examples/demo-atmos-cli-imports/atmos.yaml`, it's acceptable to use `refs/heads/main` in remote URLs.
Applied to files:
website/docs/integrations/atlantis.mdx
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
website/docs/integrations/atlantis.mdxexamples/quick-start-advanced/Dockerfile
📚 Learning: 2025-01-25T15:21:40.413Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos-cli-imports/atmos.yaml:8-8
Timestamp: 2025-01-25T15:21:40.413Z
Learning: In Atmos, when a directory is specified for configuration loading (e.g., in the `import` section of atmos.yaml), all files within that directory should be treated as Atmos configurations. Do not suggest restricting file extensions in directory-based glob patterns.
Applied to files:
tests/fixtures/scenarios/authmanager-nested-propagation/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/authmanager-nested-propagation/atmos.yamlexamples/quick-start-advanced/Dockerfile
📚 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/authmanager-nested-propagation/atmos.yamlexamples/quick-start-advanced/Dockerfile
📚 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:
tests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml
📚 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/authmanager-nested-propagation/atmos.yamldocs/terraform-output-yaml-func-authentication-flow.mdpkg/hooks/store_cmd_nil_handling_test.gointernal/exec/terraform_state_utils.gointernal/exec/yaml_func_terraform_state.godocs/terraform-state-yaml-func-authentication-flow.mdpkg/hooks/store_cmd_test.gopkg/hooks/store_cmd.go
📚 Learning: 2025-09-24T20:45:40.401Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: tests/fixtures/scenarios/atmos-auth/stacks/deploy/nonprod.yaml:3-4
Timestamp: 2025-09-24T20:45:40.401Z
Learning: In Atmos stack files, the correct syntax for importing other stack files is `import:` (singular), not `imports:` (plural). All stack files in the Atmos codebase consistently use `import:` followed by a list of paths to import.
Applied to files:
tests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml
📚 Learning: 2025-02-24T22:46:39.744Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1085
File: pkg/config/load.go:219-221
Timestamp: 2025-02-24T22:46:39.744Z
Learning: In the Atmos configuration system, imports from atmos.d are optional. When import errors occur, they should be logged at debug level and the process should continue, rather than failing completely.
Applied to files:
tests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml
📚 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/terraform_output_utils.gointernal/exec/terraform.gointernal/exec/yaml_func_terraform_output_authcontext_test.gointernal/exec/terraform_nested_auth_helper.gopkg/hooks/store_cmd_nil_handling_test.gointernal/exec/terraform_state_utils.gointernal/exec/mock_terraform_state_getter.gointernal/exec/yaml_func_terraform_output.gointernal/exec/yaml_func_terraform_state_authcontext_test.gointernal/exec/yaml_func_terraform_state.gointernal/exec/terraform_output_getter.gointernal/exec/terraform_state_getter.gopkg/schema/schema.gopkg/hooks/store_cmd.gotests/fixtures/scenarios/authmanager-nested-propagation/README.md
📚 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/describe_component_auth_override_test.gointernal/exec/yaml_func_terraform_output_authcontext_test.gointernal/exec/mock_terraform_output_getter.gopkg/hooks/store_cmd_nil_handling_test.gointernal/exec/terraform_state_utils.gointernal/exec/mock_terraform_state_getter.gointernal/exec/yaml_func_terraform_state_authcontext_test.gointernal/exec/terraform_output_getter.gointernal/exec/describe_component_authmanager_propagation_test.gointernal/exec/terraform_state_getter.gopkg/schema/schema.gopkg/hooks/store_cmd_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:
internal/exec/terraform_output_utils.gointernal/exec/terraform_state_utils.go
📚 Learning: 2025-04-26T15:54:10.506Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1195
File: internal/exec/terraform_clean.go:99-99
Timestamp: 2025-04-26T15:54:10.506Z
Learning: The error variable `ErrRelPath` is defined in `internal/exec/terraform_clean_util.go` and is used across files in the `exec` package, including in `terraform_clean.go`. This is part of an approach to standardize error handling in the codebase.
Applied to files:
internal/exec/terraform_output_utils.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.gointernal/exec/terraform.gointernal/exec/terraform_nested_auth_helper.gointernal/exec/terraform_state_utils.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:
internal/exec/terraform_output_utils.gopkg/hooks/store_cmd_nil_handling_test.gointernal/exec/terraform_state_utils.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/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.gogo.modinternal/exec/terraform_state_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:
internal/exec/terraform_output_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:
internal/exec/terraform_output_utils.gointernal/exec/terraform_state_utils.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/terraform_output_utils.gointernal/exec/terraform_state_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:
internal/exec/terraform.gointernal/exec/terraform_output_getter.gointernal/exec/terraform_state_getter.gopkg/hooks/store_cmd.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.gointernal/exec/terraform_output_getter.gointernal/exec/terraform_state_getter.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:
tests/fixtures/scenarios/authmanager-nested-propagation/stacks/test.yamldocs/terraform-output-yaml-func-authentication-flow.mddocs/terraform-state-yaml-func-authentication-flow.mdtests/fixtures/scenarios/authmanager-nested-propagation/README.md
📚 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:
internal/exec/describe_component_auth_override_test.gointernal/exec/describe_component_authmanager_propagation_test.gointernal/exec/describe_component_nested_authmanager_test.gopkg/hooks/store_cmd_test.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
internal/exec/describe_component_auth_override_test.gopkg/hooks/store_cmd_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:
internal/exec/describe_component_auth_override_test.gopkg/hooks/store_cmd_test.goexamples/quick-start-advanced/Dockerfile
📚 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/yaml_func_terraform_output_authcontext_test.gointernal/exec/yaml_func_terraform_state_authcontext_test.gopkg/hooks/store_cmd_test.go
📚 Learning: 2024-12-03T04:01:16.446Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:0-0
Timestamp: 2024-12-03T04:01:16.446Z
Learning: In the `terraform.output.mdx` documentation file (`website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx`), the cache invalidation and cache scope behavior for the `!terraform.output` function are already described.
Applied to files:
docs/terraform-output-yaml-func-authentication-flow.mddocs/terraform-state-yaml-func-authentication-flow.md
📚 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:
docs/terraform-output-yaml-func-authentication-flow.mdinternal/exec/yaml_func_terraform_output.godocs/terraform-state-yaml-func-authentication-flow.md
📚 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:
docs/terraform-output-yaml-func-authentication-flow.mdinternal/exec/yaml_func_terraform_output.gointernal/exec/terraform_output_getter.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 go.{mod,sum} : Manage dependencies with Go modules
Applied to files:
go.mod
📚 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 go.{mod,sum} : Keep dependencies up to date
Applied to files:
go.mod
📚 Learning: 2024-11-18T13:59:10.824Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
Applied to files:
go.mod
📚 Learning: 2025-10-11T04:03:41.651Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1611
File: go.mod:3-3
Timestamp: 2025-10-11T04:03:41.651Z
Learning: The Go module "go" directive supports three-part version numbers (e.g., "go 1.24.8") since Go 1.21+. The format "go 1.N.P" is valid and recommended for specifying patch versions. Earlier Go versions only accepted major.minor format, but modern tooling fully supports the patch component.
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: 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: 2025-11-10T23:23:39.771Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/registry/aqua/aqua_test.go:417-442
Timestamp: 2025-11-10T23:23:39.771Z
Learning: In Atmos toolchain AquaRegistry, tests should not hit real GitHub. Use the options pattern via WithGitHubBaseURL to inject an httptest server URL and make GetLatestVersion/GetAvailableVersions deterministic.
Applied to files:
pkg/hooks/store_cmd_nil_handling_test.gopkg/hooks/store_cmd_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:
internal/exec/terraform_state_utils.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:
internal/exec/terraform_state_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:
internal/exec/terraform_state_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_state_utils.go
📚 Learning: 2024-12-17T07:08:41.288Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 863
File: internal/exec/yaml_func_terraform_output.go:34-38
Timestamp: 2024-12-17T07:08:41.288Z
Learning: In the `processTagTerraformOutput` function within `internal/exec/yaml_func_terraform_output.go`, parameters are separated by spaces and do not contain spaces. Therefore, using `strings.Fields()` for parsing is acceptable, and there's no need to handle parameters with spaces.
Applied to files:
internal/exec/terraform_output_getter.go
📚 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:
docs/terraform-state-yaml-func-authentication-flow.md
📚 Learning: 2025-04-25T20:54:19.701Z
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
Applied to files:
docs/terraform-state-yaml-func-authentication-flow.md
📚 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:
internal/exec/describe_component_authmanager_propagation_test.gopkg/hooks/store_cmd_test.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:
internal/exec/terraform_state_getter.gopkg/hooks/store_cmd.go
📚 Learning: 2024-12-17T07:10:26.295Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 863
File: internal/exec/stack_utils.go:201-223
Timestamp: 2024-12-17T07:10:26.295Z
Learning: Unit tests for remote state backend functionality are located in the `examples/tests` directory.
Applied to files:
tests/fixtures/scenarios/authmanager-propagation/stacks/test.yaml
📚 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: Consider using testify/mock for creating mock implementations
Applied to files:
pkg/hooks/store_cmd_test.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:
examples/quick-start-advanced/Dockerfile
📚 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:
examples/quick-start-advanced/Dockerfile
📚 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:
examples/quick-start-advanced/Dockerfile
📚 Learning: 2024-10-25T20:26:56.449Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 729
File: .goreleaser.yml:26-26
Timestamp: 2024-10-25T20:26:56.449Z
Learning: There is no inconsistency in the version path in `build.sh`; it already uses the correct path `github.com/cloudposse/atmos/pkg/version.Version`.
Applied to files:
examples/quick-start-advanced/Dockerfile
📚 Learning: 2025-09-30T19:03:50.738Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 1560
File: pkg/utils/string_utils.go:43-64
Timestamp: 2025-09-30T19:03:50.738Z
Learning: In the Atmos codebase, YAML tags like !terraform.output rely on positional arguments, so the SplitStringByDelimiter function in pkg/utils/string_utils.go must preserve empty strings (even after trimming quotes) to maintain the correct number of positional arguments. Filtering out empty values after trimming would collapse the array and break these function calls.
Applied to files:
pkg/hooks/store_cmd.go
📚 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/authmanager-nested-propagation/README.md
🧬 Code graph analysis (17)
internal/exec/terraform_output_utils.go (3)
pkg/auth/hooks.go (1)
AuthManager(24-24)errors/errors.go (1)
ErrInvalidAuthManagerType(65-65)pkg/logger/log.go (1)
Debug(24-26)
internal/exec/terraform.go (2)
pkg/auth/hooks.go (1)
AuthManager(24-24)pkg/auth/types/interfaces.go (1)
AuthManager(118-216)
internal/exec/describe_component_auth_override_test.go (4)
pkg/schema/schema.go (3)
AuthContext(540-548)AWSAuthContext(552-567)ConfigAndStacksInfo(569-662)pkg/auth/types/mock_interfaces.go (1)
NewMockAuthManager(357-361)internal/exec/describe_component.go (1)
ExecuteDescribeComponentParams(202-209)pkg/config/config.go (1)
InitCliConfig(25-64)
internal/exec/mock_terraform_output_getter.go (1)
pkg/schema/schema.go (2)
AtmosConfiguration(27-66)AuthContext(540-548)
internal/exec/terraform_nested_auth_helper.go (7)
pkg/schema/schema.go (1)
AtmosConfiguration(27-66)pkg/logger/log.go (1)
Debug(24-26)pkg/config/const.go (1)
AuthSectionName(84-84)internal/exec/describe_component.go (1)
ExecuteDescribeComponentParams(202-209)errors/errors.go (3)
ErrDescribeComponent(80-80)ErrAuthManager(454-454)ErrAuthConsole(61-61)pkg/auth/config_helpers.go (1)
MergeComponentAuthFromConfig(138-160)pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManager(174-216)
pkg/hooks/store_cmd_nil_handling_test.go (1)
pkg/schema/schema.go (2)
AtmosConfiguration(27-66)AuthContext(540-548)
internal/exec/terraform_state_utils.go (5)
pkg/auth/types/interfaces.go (1)
AuthManager(118-216)errors/errors.go (1)
ErrInvalidAuthManagerType(65-65)pkg/logger/log.go (1)
Debug(24-26)internal/exec/describe_component.go (2)
ExecuteDescribeComponent(212-228)ExecuteDescribeComponentParams(202-209)pkg/describe/describe_component.go (1)
ExecuteDescribeComponent(9-26)
internal/exec/mock_terraform_state_getter.go (1)
pkg/schema/schema.go (2)
AtmosConfiguration(27-66)AuthContext(540-548)
internal/exec/yaml_func_terraform_output.go (1)
pkg/schema/schema.go (1)
AuthContext(540-548)
internal/exec/yaml_func_terraform_state.go (2)
pkg/schema/schema.go (1)
AuthContext(540-548)pkg/auth/types/interfaces.go (1)
AuthManager(118-216)
internal/exec/terraform_output_getter.go (1)
internal/exec/terraform_output_utils.go (1)
GetTerraformOutput(530-649)
internal/exec/describe_component_authmanager_propagation_test.go (3)
pkg/schema/schema.go (3)
AuthContext(540-548)AWSAuthContext(552-567)ConfigAndStacksInfo(569-662)pkg/auth/types/mock_interfaces.go (1)
NewMockAuthManager(357-361)internal/exec/describe_component.go (2)
ExecuteDescribeComponent(212-228)ExecuteDescribeComponentParams(202-209)
internal/exec/terraform_state_getter.go (1)
internal/exec/terraform_state_utils.go (1)
GetTerraformState(33-146)
internal/exec/describe_component_nested_authmanager_test.go (3)
pkg/schema/schema.go (3)
AuthContext(540-548)AWSAuthContext(552-567)ConfigAndStacksInfo(569-662)pkg/auth/types/mock_interfaces.go (1)
NewMockAuthManager(357-361)internal/exec/describe_component.go (1)
ExecuteDescribeComponentParams(202-209)
pkg/schema/schema.go (1)
pkg/auth/types/interfaces.go (1)
AuthManager(118-216)
pkg/hooks/store_cmd_test.go (1)
pkg/schema/schema.go (1)
AuthContext(540-548)
pkg/hooks/store_cmd.go (1)
pkg/schema/schema.go (1)
AuthContext(540-548)
🪛 LanguageTool
docs/terraform-output-yaml-func-authentication-flow.md
[grammar] ~133-~133: Please add a punctuation mark at the end of paragraph.
Context: ...ted levels** use the same authenticated session This ensures that deeply nested compon...
(PUNCTUATION_PARAGRAPH_END)
[uncategorized] ~541-~541: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... identity selection happens once at the top level - Transitive permissions - Top-leve...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~542-~542: Since ownership is already implied, this phrasing may be redundant.
Context: ...nitial component, nested components use their own auth ## How Credentials Flow ### 1. A...
(PRP_OWN)
docs/terraform-state-yaml-func-authentication-flow.md
[grammar] ~136-~136: Please add a punctuation mark at the end of paragraph.
Context: ...ted levels** use the same authenticated session This ensures that deeply nested compon...
(PUNCTUATION_PARAGRAPH_END)
[uncategorized] ~434-~434: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... identity selection happens once at the top level - Transitive permissions - Top-leve...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~435-~435: Since ownership is already implied, this phrasing may be redundant.
Context: ...nitial component, nested components use their own auth ## How Credentials Flow ### 1. A...
(PRP_OWN)
docs/fixes/nested-terraform-state-auth-context-propagation.md
[grammar] ~346-~346: Please add a punctuation mark at the end of paragraph.
Context: ...aform.output` - All mock generators and tests Example in terraform.go: ```go fu...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~696-~696: Consider using a typographic opening quote here.
Context: ...ml-func-authentication-flow.md` - Added "Component-Level Auth Override" section -...
(EN_QUOTES)
[typographical] ~696-~696: Consider using a typographic close quote here.
Context: ...- Added "Component-Level Auth Override" section -docs/terraform-output-yaml-f...
(EN_QUOTES)
[typographical] ~697-~697: Consider using a typographic opening quote here.
Context: ...ml-func-authentication-flow.md` - Added "Component-Level Auth Override in Nested ...
(EN_QUOTES)
[typographical] ~697-~697: Consider using a typographic close quote here.
Context: ...-Level Auth Override in Nested Functions" section ## Related Issues - PR #1769:...
(EN_QUOTES)
tests/fixtures/scenarios/authmanager-nested-propagation/README.md
[style] ~81-~81: Since ownership is already implied, this phrasing may be redundant.
Context: ...ple components in a chain can each have their own auth overrides. ``` Level 1: multi-aut...
(PRP_OWN)
[style] ~117-~117: Since ownership is already implied, this phrasing may be redundant.
Context: ...e-componentuses account 555555555555 (its own auth) - Whenmixed-override-component`...
(PRP_OWN)
🪛 markdownlint-cli2 (0.18.1)
docs/terraform-output-yaml-func-authentication-flow.md
532-532: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/terraform-state-yaml-func-authentication-flow.md
425-425: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/fixes/nested-terraform-state-auth-context-propagation.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
169-169: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
516-516: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
tests/fixtures/scenarios/authmanager-nested-propagation/README.md
31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
50-50: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
83-83: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
107-107: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
158-158: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
170-170: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
207-207: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
368-368: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (15)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Lint (golangci)
- GitHub Check: Build (macos)
- GitHub Check: Build (windows)
- GitHub Check: Build (linux)
- GitHub Check: Run pre-commit hooks
- GitHub Check: Review Dependency Licenses
- GitHub Check: autofix
- GitHub Check: website-deploy-preview
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
internal/exec/terraform_nested_auth_helper.go (1)
124-170: Wrap the underlying errors you’re surfacingRight now the fmt.Errorf calls only return the sentinel and drop the original err, so callers lose the diagnostic chain. Please wrap the inner err with %w alongside the sentinel.
[suggested diff]- return nil, fmt.Errorf("%w: failed to get component config for auth resolution", errUtils.ErrDescribeComponent) + return nil, fmt.Errorf("%w: failed to get component config for auth resolution: %w", errUtils.ErrDescribeComponent, err) @@ - return parentAuthManager, fmt.Errorf("%w: failed to merge component auth config", errUtils.ErrAuthManager) + return parentAuthManager, fmt.Errorf("%w: failed to merge component auth config: %w", errUtils.ErrAuthManager, err) @@ - return parentAuthManager, fmt.Errorf("%w: failed to create component-specific AuthManager", errUtils.ErrAuthConsole) + return parentAuthManager, fmt.Errorf("%w: failed to create component-specific AuthManager: %w", errUtils.ErrAuthConsole, err)Based on learnings
🧹 Nitpick comments (5)
tests/fixtures/scenarios/authmanager-nested-propagation/README.md (1)
31-400: Consider annotating the fenced blocks. Markdownlint (MD040) is already flagging the bare ``` fences; sprinkling inyaml, `text`, or `bash` where appropriate will keep doc linting happy.docs/terraform-output-yaml-func-authentication-flow.md (2)
70-252: Excellent documentation of nested authentication flow.The nested function authentication section is comprehensive and well-structured. The examples clearly illustrate different nesting scenarios (microservices, infrastructure layering, multi-region deployments) and the authentication propagation mechanism is explained step-by-step.
One minor improvement: Line 133 is missing punctuation at the end of the "Key Points" section's last item before the paragraph explaining the behavior.
Apply this diff to fix the punctuation:
-4. **All nested levels** use the same authenticated session +4. **All nested levels** use the same authenticated session.
403-543: Comprehensive component-level auth override documentation.This section effectively documents the component-level auth override feature for nested functions. The multi-account example clearly illustrates when and why this feature is useful, and the algorithm reference with pseudo-code provides implementation clarity.
Minor improvements needed:
- Line 532: Add language identifier to fenced code block
- Line 541: Consider hyphenating "top-level" as a compound adjective
- Line 542: The phrase "their own auth" is slightly redundant given the possessive "their"
Apply these diffs:
- ```bash + ```shell # Enable debug logging to see auth resolution ATMOS_LOGS_LEVEL=Debug atmos describe component my-component -s stack ```-- **Single top-level identity** - When using `--identity` flag, it applies at the top level only +- **Single top-level identity** - When using `--identity` flag, it applies at the top-level only-- **Transitive permissions** - Top-level identity must have permissions for initial component, nested components use their own auth +- **Transitive permissions** - Top-level identity must have permissions for initial component, nested components use component-specific authdocs/terraform-state-yaml-func-authentication-flow.md (2)
74-217: Well-documented nested function authentication.The nested function authentication section maintains consistency with the
!terraform.outputdocumentation while providing relevant examples specific to!terraform.stateusage. The Transit Gateway hub-spoke architecture example is particularly valuable for AWS networking scenarios.Same minor punctuation issue as the output documentation:
Apply this diff:
-4. **All nested levels** use the same authenticated session +4. **All nested levels** use the same authenticated session.
248-436: Excellent component-level auth documentation.This section effectively documents component-level auth overrides for
!terraform.statefunctions. The cross-account state reading use cases are particularly relevant since reading remote state often involves cross-account scenarios.Same minor improvements as the terraform-output documentation:
Apply these diffs:
- ```bash + ```shell # Enable debug logging to see auth resolution ATMOS_LOGS_LEVEL=Debug atmos describe component my-component -s stack ```-- **Single top-level identity** - When using `--identity` flag, it applies at the top level only +- **Single top-level identity** - When using `--identity` flag, it applies at the top-level only-- **Transitive permissions** - Top-level identity must have permissions for initial component, nested components use their own auth +- **Transitive permissions** - Top-level identity must have permissions for initial component, nested components use component-specific auth
📜 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 (31)
NOTICE(8 hunks)docs/fixes/nested-terraform-state-auth-context-propagation.md(1 hunks)docs/terraform-output-yaml-func-authentication-flow.md(2 hunks)docs/terraform-state-yaml-func-authentication-flow.md(2 hunks)errors/errors.go(1 hunks)examples/quick-start-advanced/Dockerfile(1 hunks)go.mod(5 hunks)internal/exec/describe_component_auth_override_test.go(1 hunks)internal/exec/describe_component_authmanager_propagation_test.go(1 hunks)internal/exec/describe_component_nested_authmanager_test.go(1 hunks)internal/exec/mock_terraform_output_getter.go(1 hunks)internal/exec/mock_terraform_state_getter.go(1 hunks)internal/exec/terraform.go(1 hunks)internal/exec/terraform_nested_auth_helper.go(1 hunks)internal/exec/terraform_output_getter.go(2 hunks)internal/exec/terraform_output_utils.go(5 hunks)internal/exec/terraform_state_getter.go(2 hunks)internal/exec/terraform_state_utils.go(4 hunks)internal/exec/yaml_func_terraform_output.go(1 hunks)internal/exec/yaml_func_terraform_output_authcontext_test.go(4 hunks)internal/exec/yaml_func_terraform_state.go(1 hunks)internal/exec/yaml_func_terraform_state_authcontext_test.go(3 hunks)pkg/hooks/store_cmd.go(2 hunks)pkg/hooks/store_cmd_nil_handling_test.go(9 hunks)pkg/hooks/store_cmd_test.go(1 hunks)pkg/schema/schema.go(1 hunks)tests/fixtures/scenarios/authmanager-nested-propagation/README.md(1 hunks)tests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml(1 hunks)tests/fixtures/scenarios/authmanager-nested-propagation/stacks/test.yaml(1 hunks)tests/fixtures/scenarios/authmanager-propagation/stacks/test.yaml(1 hunks)website/docs/integrations/atlantis.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*_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/yaml_func_terraform_state_authcontext_test.gointernal/exec/describe_component_auth_override_test.gopkg/hooks/store_cmd_test.gointernal/exec/describe_component_authmanager_propagation_test.gopkg/hooks/store_cmd_nil_handling_test.gointernal/exec/yaml_func_terraform_output_authcontext_test.gointernal/exec/describe_component_nested_authmanager_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/yaml_func_terraform_state_authcontext_test.goerrors/errors.gointernal/exec/describe_component_auth_override_test.gopkg/schema/schema.gointernal/exec/terraform_output_utils.gointernal/exec/terraform.gointernal/exec/yaml_func_terraform_output.gointernal/exec/terraform_state_utils.gopkg/hooks/store_cmd.gointernal/exec/terraform_nested_auth_helper.gointernal/exec/mock_terraform_output_getter.gopkg/hooks/store_cmd_test.gointernal/exec/terraform_state_getter.gointernal/exec/describe_component_authmanager_propagation_test.gointernal/exec/mock_terraform_state_getter.gointernal/exec/yaml_func_terraform_state.gopkg/hooks/store_cmd_nil_handling_test.gointernal/exec/yaml_func_terraform_output_authcontext_test.gointernal/exec/terraform_output_getter.gointernal/exec/describe_component_nested_authmanager_test.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
errors/errors.gopkg/schema/schema.gointernal/exec/terraform_output_utils.gointernal/exec/terraform.gointernal/exec/yaml_func_terraform_output.gointernal/exec/terraform_state_utils.gopkg/hooks/store_cmd.gointernal/exec/terraform_nested_auth_helper.gointernal/exec/mock_terraform_output_getter.gointernal/exec/terraform_state_getter.gointernal/exec/mock_terraform_state_getter.gointernal/exec/yaml_func_terraform_state.gointernal/exec/terraform_output_getter.go
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/schema/schema.gopkg/hooks/store_cmd.gopkg/hooks/store_cmd_test.gopkg/hooks/store_cmd_nil_handling_test.go
go.{mod,sum}
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
go.{mod,sum}: Manage dependencies with Go modules
Keep dependencies up to date
Files:
go.mod
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/integrations/atlantis.mdx
🧠 Learnings (71)
📓 Common learnings
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-17T07:10:26.295Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 863
File: internal/exec/stack_utils.go:201-223
Timestamp: 2024-12-17T07:10:26.295Z
Learning: Unit tests for remote state backend functionality are located in the `examples/tests` directory.
Applied to files:
tests/fixtures/scenarios/authmanager-propagation/stacks/test.yaml
📚 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/yaml_func_terraform_state_authcontext_test.gopkg/schema/schema.gointernal/exec/terraform_output_utils.gointernal/exec/terraform.gointernal/exec/terraform_state_utils.gopkg/hooks/store_cmd.gointernal/exec/terraform_nested_auth_helper.gopkg/hooks/store_cmd_test.gointernal/exec/terraform_state_getter.gointernal/exec/mock_terraform_state_getter.gointernal/exec/yaml_func_terraform_state.gopkg/hooks/store_cmd_nil_handling_test.gointernal/exec/yaml_func_terraform_output_authcontext_test.gointernal/exec/terraform_output_getter.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/yaml_func_terraform_state_authcontext_test.gointernal/exec/describe_component_auth_override_test.gopkg/schema/schema.gointernal/exec/terraform_output_utils.gointernal/exec/terraform_state_utils.gopkg/hooks/store_cmd.gointernal/exec/terraform_nested_auth_helper.gointernal/exec/mock_terraform_output_getter.gopkg/hooks/store_cmd_test.gointernal/exec/terraform_state_getter.gointernal/exec/describe_component_authmanager_propagation_test.gointernal/exec/mock_terraform_state_getter.gopkg/hooks/store_cmd_nil_handling_test.gointernal/exec/terraform_output_getter.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/yaml_func_terraform_state_authcontext_test.gopkg/hooks/store_cmd_test.gointernal/exec/yaml_func_terraform_output_authcontext_test.gointernal/exec/describe_component_nested_authmanager_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 : Every new feature must include comprehensive unit tests
Applied to files:
internal/exec/describe_component_auth_override_test.gopkg/hooks/store_cmd_test.gointernal/exec/describe_component_authmanager_propagation_test.gointernal/exec/describe_component_nested_authmanager_test.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
internal/exec/describe_component_auth_override_test.gopkg/hooks/store_cmd_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:
internal/exec/describe_component_auth_override_test.gointernal/exec/terraform_state_utils.goexamples/quick-start-advanced/Dockerfilepkg/hooks/store_cmd_test.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:
NOTICEexamples/quick-start-advanced/Dockerfile
📚 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 go.{mod,sum} : Keep dependencies up to date
Applied to files:
NOTICEgo.mod
📚 Learning: 2024-11-18T13:59:10.824Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_model_component.go:3-20
Timestamp: 2024-11-18T13:59:10.824Z
Learning: When replacing significant dependencies like `go-getter` that require extensive changes, prefer to address them in separate PRs.
Applied to files:
NOTICEgo.mod
📚 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 go.{mod,sum} : Manage dependencies with Go modules
Applied to files:
NOTICEgo.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: 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:
NOTICEgo.mod
📚 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:
internal/exec/terraform_output_utils.gopkg/hooks/store_cmd.gointernal/exec/terraform_output_getter.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/terraform_state_utils.go
📚 Learning: 2025-04-26T15:54:10.506Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1195
File: internal/exec/terraform_clean.go:99-99
Timestamp: 2025-04-26T15:54:10.506Z
Learning: The error variable `ErrRelPath` is defined in `internal/exec/terraform_clean_util.go` and is used across files in the `exec` package, including in `terraform_clean.go`. This is part of an approach to standardize error handling in the codebase.
Applied to files:
internal/exec/terraform_output_utils.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.gointernal/exec/terraform.gointernal/exec/terraform_state_utils.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:
internal/exec/terraform_output_utils.gointernal/exec/terraform_state_utils.gointernal/exec/terraform_nested_auth_helper.gopkg/hooks/store_cmd_nil_handling_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/terraform_output_utils.gointernal/exec/terraform_nested_auth_helper.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/terraform_state_utils.gogo.mod
📚 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/terraform_output_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:
internal/exec/terraform_output_utils.gointernal/exec/terraform_state_utils.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/terraform_output_utils.gointernal/exec/terraform_state_utils.gointernal/exec/terraform_nested_auth_helper.go
📚 Learning: 2024-12-03T04:01:16.446Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:0-0
Timestamp: 2024-12-03T04:01:16.446Z
Learning: In the `terraform.output.mdx` documentation file (`website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx`), the cache invalidation and cache scope behavior for the `!terraform.output` function are already described.
Applied to files:
docs/fixes/nested-terraform-state-auth-context-propagation.mddocs/terraform-output-yaml-func-authentication-flow.mddocs/terraform-state-yaml-func-authentication-flow.md
📚 Learning: 2025-09-09T02:14:36.708Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: internal/auth/types/whoami.go:14-15
Timestamp: 2025-09-09T02:14:36.708Z
Learning: The WhoamiInfo struct in internal/auth/types/whoami.go requires the Credentials field to be JSON-serializable for keystore unmarshaling operations, despite security concerns about credential exposure.
Applied to files:
internal/exec/terraform.go
📚 Learning: 2024-12-17T07:08:41.288Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 863
File: internal/exec/yaml_func_terraform_output.go:34-38
Timestamp: 2024-12-17T07:08:41.288Z
Learning: In the `processTagTerraformOutput` function within `internal/exec/yaml_func_terraform_output.go`, parameters are separated by spaces and do not contain spaces. Therefore, using `strings.Fields()` for parsing is acceptable, and there's no need to handle parameters with spaces.
Applied to files:
internal/exec/yaml_func_terraform_output.go
📚 Learning: 2024-11-30T22:07:08.610Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/yaml_func_terraform_output.go:35-40
Timestamp: 2024-11-30T22:07:08.610Z
Learning: In the Go function `processTagTerraformOutput` in `internal/exec/yaml_func_terraform_output.go`, parameters cannot contain spaces. The code splits the input by spaces, and if the parameters contain spaces, `len(parts) != 3` will fail and show an error to the user.
Applied to files:
internal/exec/yaml_func_terraform_output.go
📚 Learning: 2025-09-30T19:03:50.738Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 1560
File: pkg/utils/string_utils.go:43-64
Timestamp: 2025-09-30T19:03:50.738Z
Learning: In the Atmos codebase, YAML tags like !terraform.output rely on positional arguments, so the SplitStringByDelimiter function in pkg/utils/string_utils.go must preserve empty strings (even after trimming quotes) to maintain the correct number of positional arguments. Filtering out empty values after trimming would collapse the array and break these function calls.
Applied to files:
internal/exec/yaml_func_terraform_output.gopkg/hooks/store_cmd.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/yaml_func_terraform_output.godocs/terraform-output-yaml-func-authentication-flow.mdinternal/exec/terraform_output_getter.go
📚 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:
internal/exec/yaml_func_terraform_output.godocs/terraform-output-yaml-func-authentication-flow.mddocs/terraform-state-yaml-func-authentication-flow.md
📚 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:
internal/exec/terraform_state_utils.gopkg/hooks/store_cmd.gopkg/hooks/store_cmd_test.godocs/terraform-output-yaml-func-authentication-flow.mdinternal/exec/yaml_func_terraform_state.gopkg/hooks/store_cmd_nil_handling_test.godocs/terraform-state-yaml-func-authentication-flow.mdtests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml
📚 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:
internal/exec/terraform_state_utils.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:
internal/exec/terraform_state_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:
internal/exec/terraform_state_utils.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_state_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:
pkg/hooks/store_cmd.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 **/*.go : Wrap errors with context using fmt.Errorf("context: %w", err)
Applied to files:
internal/exec/terraform_nested_auth_helper.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/terraform_nested_auth_helper.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:
internal/exec/terraform_nested_auth_helper.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 **/*.go : Follow Go error handling idioms and use meaningful error messages
Applied to files:
internal/exec/terraform_nested_auth_helper.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Applied to files:
internal/exec/terraform_nested_auth_helper.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_nested_auth_helper.go
📚 Learning: 2025-04-04T02:03:23.676Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
Applied to files:
internal/exec/terraform_nested_auth_helper.go
📚 Learning: 2024-11-12T03:15:15.627Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-12T03:15:15.627Z
Learning: It is acceptable to set `ARG ATMOS_VERSION` to a future version like `1.105.0` in `examples/quick-start-advanced/Dockerfile` if that will be the next release.
Applied to files:
examples/quick-start-advanced/Dockerfilewebsite/docs/integrations/atlantis.mdx
📚 Learning: 2024-11-23T00:13:22.004Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 801
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-23T00:13:22.004Z
Learning: When updating the `ATMOS_VERSION` in Dockerfiles, the team prefers to pin to the next future version when the PR merges, even if the version is not yet released.
Applied to files:
examples/quick-start-advanced/Dockerfilewebsite/docs/integrations/atlantis.mdx
📚 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:
examples/quick-start-advanced/Dockerfilewebsite/docs/integrations/atlantis.mdxtests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml
📚 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:
examples/quick-start-advanced/Dockerfilewebsite/docs/integrations/atlantis.mdxtests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
examples/quick-start-advanced/Dockerfilewebsite/docs/integrations/atlantis.mdx
📚 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:
examples/quick-start-advanced/Dockerfiletests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml
📚 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:
examples/quick-start-advanced/Dockerfile
📚 Learning: 2025-09-10T17:34:52.568Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/providers/github/oidc.go:96-100
Timestamp: 2025-09-10T17:34:52.568Z
Learning: The ATMOS_ environment variable binding guideline applies to Atmos configuration variables, not external service-required environment variables like GitHub Actions OIDC variables (GITHUB_ACTIONS, ACTIONS_ID_TOKEN_*) which must use their standard names.
Applied to files:
examples/quick-start-advanced/Dockerfilewebsite/docs/integrations/atlantis.mdx
📚 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:
examples/quick-start-advanced/Dockerfiletests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml
📚 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:
examples/quick-start-advanced/Dockerfilewebsite/docs/integrations/atlantis.mdx
📚 Learning: 2024-10-25T20:26:56.449Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 729
File: .goreleaser.yml:26-26
Timestamp: 2024-10-25T20:26:56.449Z
Learning: There is no inconsistency in the version path in `build.sh`; it already uses the correct path `github.com/cloudposse/atmos/pkg/version.Version`.
Applied to files:
examples/quick-start-advanced/Dockerfile
📚 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:
pkg/hooks/store_cmd_test.gointernal/exec/describe_component_authmanager_propagation_test.gointernal/exec/describe_component_nested_authmanager_test.go
📚 Learning: 2025-11-10T23:23:39.771Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/registry/aqua/aqua_test.go:417-442
Timestamp: 2025-11-10T23:23:39.771Z
Learning: In Atmos toolchain AquaRegistry, tests should not hit real GitHub. Use the options pattern via WithGitHubBaseURL to inject an httptest server URL and make GetLatestVersion/GetAvailableVersions deterministic.
Applied to files:
pkg/hooks/store_cmd_test.gopkg/hooks/store_cmd_nil_handling_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: Consider using testify/mock for creating mock implementations
Applied to files:
pkg/hooks/store_cmd_test.go
📚 Learning: 2025-10-11T04:03:41.651Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1611
File: go.mod:3-3
Timestamp: 2025-10-11T04:03:41.651Z
Learning: The Go module "go" directive supports three-part version numbers (e.g., "go 1.24.8") since Go 1.21+. The format "go 1.N.P" is valid and recommended for specifying patch versions. Earlier Go versions only accepted major.minor format, but modern tooling fully supports the patch component.
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: 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_state_getter.gointernal/exec/terraform_output_getter.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:
docs/terraform-output-yaml-func-authentication-flow.mddocs/terraform-state-yaml-func-authentication-flow.mdtests/fixtures/scenarios/authmanager-nested-propagation/README.mdtests/fixtures/scenarios/authmanager-nested-propagation/stacks/test.yaml
📚 Learning: 2024-12-12T15:15:46.457Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos-cli-imports/atmos.yaml:7-7
Timestamp: 2024-12-12T15:15:46.457Z
Learning: In example configuration files, such as `examples/demo-atmos-cli-imports/atmos.yaml`, it's acceptable to use `refs/heads/main` in remote URLs.
Applied to files:
website/docs/integrations/atlantis.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:
docs/terraform-state-yaml-func-authentication-flow.md
📚 Learning: 2025-04-25T20:54:19.701Z
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
Applied to files:
docs/terraform-state-yaml-func-authentication-flow.md
📚 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/authmanager-nested-propagation/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/authmanager-nested-propagation/README.md
📚 Learning: 2025-01-25T15:21:40.413Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos-cli-imports/atmos.yaml:8-8
Timestamp: 2025-01-25T15:21:40.413Z
Learning: In Atmos, when a directory is specified for configuration loading (e.g., in the `import` section of atmos.yaml), all files within that directory should be treated as Atmos configurations. Do not suggest restricting file extensions in directory-based glob patterns.
Applied to files:
tests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml
📚 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:
tests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml
📚 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:
tests/fixtures/scenarios/authmanager-nested-propagation/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/authmanager-nested-propagation/atmos.yaml
📚 Learning: 2025-01-08T19:01:32.938Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 899
File: examples/tests/test-vendor/test-components/main.tf:1-7
Timestamp: 2025-01-08T19:01:32.938Z
Learning: In the examples/tests directory of the atmos project, code examples are intentionally kept minimal and simple to facilitate understanding. Avoid suggesting additional complexity or validations that might make the examples harder to follow.
Applied to files:
tests/fixtures/scenarios/authmanager-nested-propagation/atmos.yaml
🪛 LanguageTool
docs/fixes/nested-terraform-state-auth-context-propagation.md
[grammar] ~346-~346: Please add a punctuation mark at the end of paragraph.
Context: ...aform.output` - All mock generators and tests Example in terraform.go: ```go fu...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~696-~696: Consider using a typographic opening quote here.
Context: ...ml-func-authentication-flow.md` - Added "Component-Level Auth Override" section -...
(EN_QUOTES)
[typographical] ~696-~696: Consider using a typographic close quote here.
Context: ...- Added "Component-Level Auth Override" section -docs/terraform-output-yaml-f...
(EN_QUOTES)
[typographical] ~697-~697: Consider using a typographic opening quote here.
Context: ...ml-func-authentication-flow.md` - Added "Component-Level Auth Override in Nested ...
(EN_QUOTES)
[typographical] ~697-~697: Consider using a typographic close quote here.
Context: ...-Level Auth Override in Nested Functions" section ## Related Issues - PR #1769:...
(EN_QUOTES)
docs/terraform-output-yaml-func-authentication-flow.md
[grammar] ~133-~133: Please add a punctuation mark at the end of paragraph.
Context: ...ted levels** use the same authenticated session This ensures that deeply nested compon...
(PUNCTUATION_PARAGRAPH_END)
[uncategorized] ~541-~541: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... identity selection happens once at the top level - Transitive permissions - Top-leve...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~542-~542: Since ownership is already implied, this phrasing may be redundant.
Context: ...nitial component, nested components use their own auth ## How Credentials Flow ### 1. A...
(PRP_OWN)
docs/terraform-state-yaml-func-authentication-flow.md
[grammar] ~136-~136: Please add a punctuation mark at the end of paragraph.
Context: ...ted levels** use the same authenticated session This ensures that deeply nested compon...
(PUNCTUATION_PARAGRAPH_END)
[uncategorized] ~434-~434: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... identity selection happens once at the top level - Transitive permissions - Top-leve...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~435-~435: Since ownership is already implied, this phrasing may be redundant.
Context: ...nitial component, nested components use their own auth ## How Credentials Flow ### 1. A...
(PRP_OWN)
tests/fixtures/scenarios/authmanager-nested-propagation/README.md
[style] ~81-~81: Since ownership is already implied, this phrasing may be redundant.
Context: ...ple components in a chain can each have their own auth overrides. ``` Level 1: multi-aut...
(PRP_OWN)
[style] ~117-~117: Since ownership is already implied, this phrasing may be redundant.
Context: ...e-componentuses account 555555555555 (its own auth) - Whenmixed-override-component`...
(PRP_OWN)
🪛 markdownlint-cli2 (0.18.1)
docs/fixes/nested-terraform-state-auth-context-propagation.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
169-169: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
516-516: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
docs/terraform-output-yaml-func-authentication-flow.md
532-532: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/terraform-state-yaml-func-authentication-flow.md
425-425: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
tests/fixtures/scenarios/authmanager-nested-propagation/README.md
31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
50-50: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
83-83: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
107-107: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
158-158: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
170-170: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
207-207: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
368-368: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (20)
- GitHub Check: [mock-windows] tests/fixtures/scenarios/complete
- GitHub Check: [mock-macos] examples/demo-vendoring
- GitHub Check: [mock-windows] examples/demo-atlantis
- GitHub Check: [mock-windows] examples/demo-vendoring
- GitHub Check: [mock-macos] examples/demo-atlantis
- GitHub Check: [mock-windows] examples/demo-context
- GitHub Check: [mock-macos] examples/demo-component-versions
- GitHub Check: [mock-macos] examples/demo-context
- GitHub Check: [mock-linux] examples/demo-vendoring
- GitHub Check: [mock-linux] tests/fixtures/scenarios/complete
- GitHub Check: [mock-windows] examples/demo-component-versions
- GitHub Check: [mock-macos] tests/fixtures/scenarios/complete
- GitHub Check: [mock-linux] examples/demo-component-versions
- GitHub Check: [k3s] demo-helmfile
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: [localstack] demo-localstack
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: [lint] Dockerfile
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
…text - Add --verify=false flag to helm plugin install commands to fix CI failure (helm now requires verification flag when installing from GitHub URLs) - Preserve underlying error context in terraform_nested_auth_helper.go by wrapping both static and original errors (addresses CodeRabbitAI review) Fixes acceptance test failures on linux, macOS, and windows runners. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/exec/terraform_nested_auth_helper.go (2)
70-77: Consider clarifying error handling semantics.The function returns both
parentAuthManageranderrwhen component config retrieval fails. This pattern is unusual in Go—typically when returning an error, other return values should be nil. While this might be an intentional graceful-degradation strategy (fall back to parent auth even when component config fails), it creates ambiguity for callers: should they check the error first or trust the returned AuthManager?Consider one of these approaches:
- Document clearly in the function comment that callers should use the returned AuthManager even when err != nil
- Return
nil, errif you want to enforce error checking- Return
parentAuthManager, nilif the fallback is acceptable and shouldn't be treated as an errorThe same pattern appears at lines 150 and 169, so the recommendation applies there as well.
158-161: Consider replacing magic strings with named constants.The empty string for
identityNameand"__NO_SELECT__"forselectValuerely on internal behavior ofCreateAndAuthenticateManager. While the comments explain the intent, this coupling is fragile. If the called function's logic changes, this code might break silently.Consider defining constants at the package level:
const ( autoDetectIdentity = "" noSelectValue = "__NO_SELECT__" )Then use them here:
- componentAuthManager, err := auth.CreateAndAuthenticateManager( - "", // Empty identity triggers auto-detection from component's auth config - mergedAuthConfig, // Merged component + global auth - "__NO_SELECT__", // Non-matching value prevents selector (since "" != "__NO_SELECT__") - ) + componentAuthManager, err := auth.CreateAndAuthenticateManager( + autoDetectIdentity, // Triggers auto-detection from component's auth config + mergedAuthConfig, // Merged component + global auth + noSelectValue, // Prevents selector by using non-matching value + )This makes the intent explicit and easier to maintain.
📜 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)
.github/workflows/test.yml(2 hunks)internal/exec/terraform_nested_auth_helper.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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_nested_auth_helper.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
internal/exec/terraform_nested_auth_helper.go
🧠 Learnings (14)
📓 Common learnings
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: 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_nested_auth_helper.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/terraform_nested_auth_helper.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/terraform_nested_auth_helper.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 **/*.go : Wrap errors with context using fmt.Errorf("context: %w", err)
Applied to files:
internal/exec/terraform_nested_auth_helper.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/terraform_nested_auth_helper.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/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:
internal/exec/terraform_nested_auth_helper.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/terraform_nested_auth_helper.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:
internal/exec/terraform_nested_auth_helper.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 **/*.go : Follow Go error handling idioms and use meaningful error messages
Applied to files:
internal/exec/terraform_nested_auth_helper.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Applied to files:
internal/exec/terraform_nested_auth_helper.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_nested_auth_helper.go
📚 Learning: 2025-04-04T02:03:23.676Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1185
File: internal/exec/yaml_func_store.go:26-26
Timestamp: 2025-04-04T02:03:23.676Z
Learning: The Atmos codebase currently uses `log.Fatal` for error handling in multiple places. The maintainers are aware this isn't an ideal pattern (should only be used in main() or init() functions) and plan to address it comprehensively in a separate PR. CodeRabbit should not flag these issues or push for immediate changes until that refactoring is complete.
Applied to files:
internal/exec/terraform_nested_auth_helper.go
🧬 Code graph analysis (1)
internal/exec/terraform_nested_auth_helper.go (7)
pkg/schema/schema.go (1)
AtmosConfiguration(27-66)pkg/logger/log.go (1)
Debug(24-26)pkg/config/const.go (1)
AuthSectionName(84-84)internal/exec/describe_component.go (1)
ExecuteDescribeComponentParams(202-209)errors/errors.go (3)
ErrDescribeComponent(80-80)ErrAuthManager(454-454)ErrAuthConsole(61-61)pkg/auth/config_helpers.go (1)
MergeComponentAuthFromConfig(138-160)pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManager(174-216)
⏰ 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: Review Dependency Licenses
- GitHub Check: Build (linux)
- GitHub Check: Build (windows)
- GitHub Check: website-deploy-preview
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: autofix
- GitHub Check: Summary
🔇 Additional comments (4)
.github/workflows/test.yml (1)
169-169: Clarify resolved: this is a documented fix for CI environment GPG verification failure.Your concern about security posture is valid. However, the git history shows this change was introduced in commit
a197c17titled "fix: Add --verify=false to helm plugin install and preserve error context"—indicating it addresses a CI failure rather than an unexplained degradation.The web context confirms
--verify=falseis the standard solution when Helm's default GPG verification of plugin tarballs fails in CI environments due to missing.provsignature files. This is an accepted tradeoff for CI reliability in such contexts.The change is justified and documented in commit history. No further clarification needed from the developer.
internal/exec/terraform_nested_auth_helper.go (3)
124-124: Error wrapping addressed correctly.The past review comment about preserving error context has been resolved. The error now wraps both the sentinel and original error using two
%wverbs, which is valid in Go 1.20+.
150-150: Error wrapping addressed correctly.Both error wrapping issues from the past review have been resolved. Lines 150 and 169 now properly wrap both the sentinel error and the original error using two
%wverbs, preserving the full error chain for debugging anderrors.Ischecks.Also applies to: 169-169
18-183: Solid implementation with good documentation.The nested auth helper functions are well-structured and thoroughly documented. The logic for determining when to inherit parent authentication versus creating component-specific auth is clear and makes sense. The decision to check for a default identity before creating a component-specific AuthManager (lines 91-101) is particularly thoughtful—it prevents the selector from showing up unnecessarily.
The error wrapping issues from the previous review have been properly addressed. The remaining suggestions are optional improvements around API clarity and maintainability rather than correctness concerns.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1786 +/- ##
==========================================
+ Coverage 70.87% 70.95% +0.08%
==========================================
Files 439 440 +1
Lines 40656 40789 +133
==========================================
+ Hits 28814 28941 +127
- Misses 9400 9405 +5
- Partials 2442 2443 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Added comprehensive unit and integration tests to improve patch coverage: Coverage improvements: - GetStaticRemoteStateOutput: 0% → 93.3% (+93.3 points) - GetTerraformOutput: 66.7% → 83.3% (+16.6 points) - GetTerraformState: 58.7% → 71.7% (+13.0 points) - hasDefaultIdentity: 80% → 100% (+20 points) New test files: - terraform_nested_auth_helper_test.go: Unit tests for hasDefaultIdentity, GetStaticRemoteStateOutput, and error handling paths - terraform_state_output_coverage_test.go: Error path tests, cache tests, invalid auth manager type tests - terraform_auth_integration_coverage_test.go: Integration tests using real test fixtures Also fixed helm-diff plugin installation in CI workflow: - Use helmfile-action's helm-plugins parameter for cross-platform install - Install specific version (v3.13.2) for kubectl test job 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/auth/manager_helpers_test.go (1)
851-877: Consider testing the actual function behavior.The test verifies error type checking but doesn't call
autoDetectDefaultIdentityto confirm it actually propagatesErrUserAborted. While you've documented the limitation, consider adding a test that invokes the function with a mock that returnsErrUserAbortedto verify the propagation behavior.Example approach:
func TestAutoDetectDefaultIdentity_PropagatesUserAbort(t *testing.T) { // Create an authConfig that would trigger GetDefaultIdentity authConfig := &schema.AuthConfig{ Identities: map[string]schema.Identity{ "default-1": {Kind: "aws/user", Default: true}, "default-2": {Kind: "aws/user", Default: true}, }, } // In CI mode with multiple defaults, GetDefaultIdentity returns error t.Setenv("CI", "true") identityName, err := autoDetectDefaultIdentity("", authConfig) // Verify behavior when GetDefaultIdentity fails // (actual abort can't be tested, but error handling path can) assert.Empty(t, identityName) // For non-abort errors, err should be nil (backward compatibility) assert.NoError(t, err) }This would verify the function's error handling logic more directly.
📜 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)
errors/exit_code_test.go(1 hunks)pkg/auth/manager_helpers_test.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/auth/manager_helpers_test.go
**/*_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:
pkg/auth/manager_helpers_test.goerrors/exit_code_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:
pkg/auth/manager_helpers_test.goerrors/exit_code_test.go
🧠 Learnings (6)
📓 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: 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: 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.
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.
📚 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:
pkg/auth/manager_helpers_test.goerrors/exit_code_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:
pkg/auth/manager_helpers_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:
pkg/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:
pkg/auth/manager_helpers_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 : Every new feature must include comprehensive unit tests
Applied to files:
pkg/auth/manager_helpers_test.go
🧬 Code graph analysis (2)
pkg/auth/manager_helpers_test.go (1)
errors/errors.go (1)
ErrUserAborted(461-461)
errors/exit_code_test.go (1)
errors/exit_code.go (3)
ExitCodeSIGINT(12-12)WithExitCode(40-48)GetExitCode(60-96)
⏰ 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 (windows)
- GitHub Check: Build (linux)
- GitHub Check: autofix
- GitHub Check: Review Dependency Licenses
- GitHub Check: Lint (golangci)
- GitHub Check: website-deploy-preview
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (3)
pkg/auth/manager_helpers_test.go (1)
4-4: LGTM!Import is needed for error wrapping in the new test.
errors/exit_code_test.go (2)
156-158: LGTM – documents POSIX convention for SIGINT exit code.The test validates the constant and serves as documentation for the 128 + signal_number convention.
162-172: Solid regression test for user abort exit handling.The test correctly validates the ErrUserAborted + ExitCodeSIGINT flow. It covers the happy path well.
Optionally, you could add a test case showing ErrUserAborted survives multiple wrappings with ExitCodeSIGINT (e.g., wrap it, then wrap again with errors.Wrap), though TestWithExitCode_Chaining already validates this pattern generically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (4)
tests/fixtures/scenarios/authmanager-nested-propagation/README.md (2)
11-11: Simplify phrasing for clarity.The phrase "define their own authentication" is slightly redundant. Consider tightening it to something like "optionally specify custom authentication."
1-50: Consider whether to anchor this README to implementation docs rather than duplicating test details.This is thorough documentation—which is great! However, based on prior feedback from similar PRs, comprehensive test fixture READMEs can add documentation rot if test coverage evolves independently. The detailed test case breakdown (scenarios 1–5, 18 components) overlaps with what's already documented in the implementation tests and architecture docs.
Consider condensing this to focus on fixture purpose and usage, then anchor readers to
docs/fixes/nested-terraform-state-auth-context-propagation.mdfor the detailed technical narrative. Keep the structure and related-files sections as-is.docs/terraform-yaml-functions-authentication-flow.md (2)
55-68: Clarify key difference in execution model. The comparison table is helpful, but the distinction between "AWS SDK Go v2 directly" vs "Terraform binary subprocess" for credentials deserves explicit emphasis—it affects performance, caching, and troubleshooting. Consider adding a brief note that this execution difference can impact timeout behavior in nested scenarios.
563-603: Style note: "Default identity required" line reads awkwardly. Line 600 states "Default identity required - Component auth override only works if..." This could read more naturally as a single bullet point instead of a dash-separated item. Also, the phrasing "their own auth" at line 603 is slightly redundant (implied by context). Consider rephrasing for conciseness.
📜 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 (4)
docs/fixes/nested-terraform-state-auth-context-propagation.md(1 hunks)docs/terraform-state-yaml-func-authentication-flow.md(0 hunks)docs/terraform-yaml-functions-authentication-flow.md(1 hunks)tests/fixtures/scenarios/authmanager-nested-propagation/README.md(1 hunks)
💤 Files with no reviewable changes (1)
- docs/terraform-state-yaml-func-authentication-flow.md
🧰 Additional context used
🧠 Learnings (7)
📓 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: 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: 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.
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.
📚 Learning: 2024-12-03T04:01:16.446Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:0-0
Timestamp: 2024-12-03T04:01:16.446Z
Learning: In the `terraform.output.mdx` documentation file (`website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx`), the cache invalidation and cache scope behavior for the `!terraform.output` function are already described.
Applied to files:
docs/fixes/nested-terraform-state-auth-context-propagation.mddocs/terraform-yaml-functions-authentication-flow.md
📚 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/authmanager-nested-propagation/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/authmanager-nested-propagation/README.mddocs/terraform-yaml-functions-authentication-flow.md
📚 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:
docs/terraform-yaml-functions-authentication-flow.md
📚 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:
docs/terraform-yaml-functions-authentication-flow.md
📚 Learning: 2025-10-11T19:12:38.832Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: tests/snapshots/TestCLICommands_atmos_workflow_invalid_step_type.stderr.golden:0-0
Timestamp: 2025-10-11T19:12:38.832Z
Learning: Usage Examples sections in error output are appropriate for command usage errors (incorrect syntax, missing arguments, invalid flags) but not for configuration validation errors (malformed workflow files, invalid settings in atmos.yaml). Configuration errors should focus on explaining what's wrong with the config, not command usage patterns.
Applied to files:
docs/terraform-yaml-functions-authentication-flow.md
🪛 LanguageTool
docs/fixes/nested-terraform-state-auth-context-propagation.md
[grammar] ~346-~346: Please add a punctuation mark at the end of paragraph.
Context: ...aform.output` - All mock generators and tests Example in terraform.go: ```go fu...
(PUNCTUATION_PARAGRAPH_END)
[style] ~696-~696: Using many exclamation marks might seem excessive (in this case: 21 exclamation marks for a text that’s 9253 characters long)
Context: ...md- Comprehensive guide covering both!terraform.stateand!terraform.output` authentication, includ...
(EN_EXCESSIVE_EXCLAMATION)
tests/fixtures/scenarios/authmanager-nested-propagation/README.md
[style] ~11-~11: Since ownership is already implied, this phrasing may be redundant.
Context: ... overrides** allow components to define their own authentication - Auth inheritance e...
(PRP_OWN)
docs/terraform-yaml-functions-authentication-flow.md
[grammar] ~204-~204: Please add a punctuation mark at the end of paragraph.
Context: ...ted levels** use the same authenticated session This ensures that deeply nested compon...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~401-~401: Please add a punctuation mark at the end of paragraph.
Context: ...MDS) - AWS credential files in standard locations ## Component-Level Auth Configuration ...
(PUNCTUATION_PARAGRAPH_END)
[uncategorized] ~599-~599: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... identity selection happens once at the top level - Default identity required - Compo...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~603-~603: Since ownership is already implied, this phrasing may be redundant.
Context: ...tial component, nested components use their own auth ## How Credentials Flow ### 1. A...
(PRP_OWN)
🪛 markdownlint-cli2 (0.18.1)
docs/fixes/nested-terraform-state-auth-context-propagation.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
169-169: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
516-516: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
tests/fixtures/scenarios/authmanager-nested-propagation/README.md
28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/terraform-yaml-functions-authentication-flow.md
590-590: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
717-717: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
723-723: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
729-729: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
737-737: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
742-742: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
749-749: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
754-754: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
820-820: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
837-837: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (9)
- GitHub Check: Build (windows)
- GitHub Check: Build (macos)
- GitHub Check: Build (linux)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: website-deploy-preview
- GitHub Check: autofix
- GitHub Check: Review Dependency Licenses
- GitHub Check: Summary
🔇 Additional comments (26)
tests/fixtures/scenarios/authmanager-nested-propagation/README.md (1)
28-28: Add language identifier to fenced code block.The code block on line 28 should specify a language for syntax highlighting. Based on the directory structure shown,
yamlwould be appropriate.[suggest_minor_issue]
-``` +```yamldocs/terraform-yaml-functions-authentication-flow.md (11)
1-10: Good introductory structure with clear references. The opening section effectively guides readers to related documentation.
209-277: Well-structured nested function scenarios. The three scenarios (microservices, infrastructure layering, transit gateway) effectively illustrate real-world use cases and provide solid context for the authentication propagation model.
403-456: Component-level auth configuration section is clear. The merging logic and behavior explanation helps readers understand precedence and inheritance.
605-708: Credentials flow section provides good technical detail. The step-by-step breakdown and code snippets showing AWS SDK configuration and environment variable preparation are accurate and helpful for implementers.
710-756: Error handling section is comprehensive. Clear categorization of general,!terraform.output-specific, and!terraform.state-specific errors helps troubleshooting.
758-854: Best practices and troubleshooting sections are well-written and actionable. Good coverage of configuration, debugging, and common failure modes (IMDS timeout, S3 access denied).
199-207: Missing punctuation at end of "Key Points" paragraph. Line 204 ends with a key insight ("without requiring separate authentication at each level.") but this is followed immediately by a new section header at line 209. Add a period to complete the thought.This ensures that deeply nested component configurations can execute terraform operations with proper credentials -without requiring separate authentication at each level. +without requiring separate authentication at each level. ### Common Nested Function ScenariosActually, reviewing this more carefully—line 207 ends with a period already. The LanguageTool warning may be a false positive here. Silently ignoring.
458-506: Add language specification to multi-account nested functions example code block. Lines 460–506 contain a YAML code block without a language spec.-```yaml +```yaml # Global auth configuration auth:⛔ Skipped due to learnings
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.Learnt from: mcalhoun Repo: cloudposse/atmos PR: 963 File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286 Timestamp: 2025-04-25T20:54:19.701Z Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
507-561: Add language specification to Go pseudo-code. TheresolveAuthManagerForNestedComponent()function at lines 513–561 is missing a language spec.-```go +```go func resolveAuthManagerForNestedComponent(⛔ Skipped due to learnings
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 **/*.go : Document complex logic with inline commentsLearnt 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.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.
332-401: Add language specifications to code blocks in identity resolution section. Lines 350–357 (YAML example), 378–384 (CLI example), and other code blocks in this section are missing language specifications.Apply diffs like:
-```yaml +```yaml auth: identities: core-auto/terraform: kind: aws/permission-set default: true # ... configuration(Note: This example already has
yamlspecified, but other blocks in the section do not.)⛔ Skipped due to 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: 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: mcalhoun Repo: cloudposse/atmos PR: 963 File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286 Timestamp: 2025-04-25T20:54:19.701Z Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
110-137: Add language specifications to code blocks. Both text blocks at lines 110–122 and 126–137 need language specifications (useplaintextortext).⛔ Skipped due to learnings
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.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.Learnt from: haitham911 Repo: cloudposse/atmos PR: 727 File: internal/exec/terraform_clean.go:407-416 Timestamp: 2024-11-24T19:13:10.287Z Learning: In `internal/exec/terraform_clean.go`, when `getStackTerraformStateFolder` returns an error in the `handleCleanSubCommand` function, the error is logged, and the process continues without returning the error.Learnt from: Cerebrovinny Repo: cloudposse/atmos PR: 1560 File: pkg/utils/string_utils.go:43-64 Timestamp: 2025-09-30T19:03:50.738Z Learning: In the Atmos codebase, YAML tags like !terraform.output rely on positional arguments, so the SplitStringByDelimiter function in pkg/utils/string_utils.go must preserve empty strings (even after trimming quotes) to maintain the correct number of positional arguments. Filtering out empty values after trimming would collapse the array and break these function calls.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.Learnt from: osterman Repo: cloudposse/atmos PR: 1599 File: internal/exec/terraform.go:0-0 Timestamp: 2025-10-11T19:11:58.965Z Learning: For terraform apply interactivity checks in Atmos (internal/exec/terraform.go), use stdin TTY detection (e.g., `IsTTYSupportForStdin()` or checking `os.Stdin`) to determine if user prompts are possible. This is distinct from stdout/stderr TTY checks used for output display (like TUI rendering). User input requires stdin to be a TTY; output display requires stdout/stderr to be a TTY.Learnt from: aknysh Repo: cloudposse/atmos PR: 863 File: internal/exec/yaml_func_terraform_output.go:34-38 Timestamp: 2024-12-17T07:08:41.288Z Learning: In the `processTagTerraformOutput` function within `internal/exec/yaml_func_terraform_output.go`, parameters are separated by spaces and do not contain spaces. Therefore, using `strings.Fields()` for parsing is acceptable, and there's no need to handle parameters with spaces.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.Learnt from: aknysh Repo: cloudposse/atmos PR: 810 File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:0-0 Timestamp: 2024-12-03T04:01:16.446Z Learning: In the `terraform.output.mdx` documentation file (`website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx`), the cache invalidation and cache scope behavior for the `!terraform.output` function are already described.Learnt from: haitham911 Repo: cloudposse/atmos PR: 1195 File: internal/exec/terraform_clean.go:99-99 Timestamp: 2025-04-26T15:54:10.506Z Learning: The error variable `ErrRelPath` is defined in `internal/exec/terraform_clean_util.go` and is used across files in the `exec` package, including in `terraform_clean.go`. This is part of an approach to standardize error handling in the codebase.docs/fixes/nested-terraform-state-auth-context-propagation.md (14)
1-29: Problem statement and symptoms are clear. The error message, debug output pattern, and "BUG: Lost auth context!" annotation effectively communicate the issue.
30-86: Reproduction case is well-structured and actionable. Stack configuration with three components, execution command, and expected vs. actual behavior makes it easy to understand the failure mode.
145-175: Root cause analysis is thorough and well-illustrated. The visual execution flow with checkmarks (✅) and crosses (❌) makes it easy to pinpoint where the bug occurs. The "missing link" diagram at lines 169–175 is particularly insightful.
221-307: Solution 1 (Add AuthManager to ConfigAndStacksInfo) is well-explained. The step-by-step implementation with code examples clearly shows all required changes. This is the recommended approach and the rationale is sound.
346-346: Add punctuation at end of "Files to update" paragraph. Line 346 states "Files to update:" followed by a list, which ends at line 347. The final line before the code example ("Example in terraform.go:") should conclude the thought with a colon or period. Currently reads: "- All mock generators and tests" then immediately "Example in terraform.go:". This is acceptable as-is, but adding context would help.
377-512: Option 2 (Global AuthManager Registry) clearly explains trade-offs. The disadvantages are honestly presented, and the "Potential Fix" explanation at lines 509–512 shows why the approach has fundamental limitations. This strengthens the case for Option 1.
514-545: Implementation plan is clear and phased. Breaking the work into Phase 1–4 (Core Changes, Propagation, Testing, Documentation) provides a good roadmap.
547-600: Testing strategy is comprehensive. The unit test pseudo-code demonstrates proper mock setup and assertion of AuthManager propagation. This clearly shows how the fix can be validated.
602-696: Additional feature section (component-level auth override) is well-documented. Feature overview, implementation details, use case example, testing, benefits, and documentation all support understanding this enhancement. The multi-account example clearly shows how component-level auth overrides enable cross-account scenarios.
696-696: Excessive exclamation marks throughout document (static analysis note). The tone is enthusiastic, but lines like "Comprehensive guide covering both!terraform.stateand!terraform.outputauthentication..." at line 696 have many function names starting with!(which looks like exclamation marks). This is a false positive from the linter—the!is part of function names (!terraform.state,!terraform.output), not style errors. No action needed.
1-700: Overall assessment: Well-structured fix documentation. Both the problem statement and proposed solutions are thoroughly explained with clear execution flows, code examples, and testing strategies. The recommendation for Option 1 is well-justified. Minor markdown formatting issues (missing code block language specs) should be addressed for consistency with markdown best practices, but content is solid and actionable.
350-375: Add language specification to terraform.go code block. Lines 350–375 contain Go code without language specification.-```go +```go func ExecuteTerraform(info schema.ConfigAndStacksInfo) error {⛔ Skipped due to learnings
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.Learnt from: aknysh Repo: cloudposse/atmos PR: 810 File: internal/exec/yaml_func_terraform_output.go:35-40 Timestamp: 2024-11-30T22:07:08.610Z Learning: In the Go function `processTagTerraformOutput` in `internal/exec/yaml_func_terraform_output.go`, parameters cannot contain spaces. The code splits the input by spaces, and if the parameters contain spaces, `len(parts) != 3` will fail and show an error to the user.Learnt from: osterman Repo: cloudposse/atmos PR: 727 File: internal/exec/terraform_clean.go:233-235 Timestamp: 2024-10-31T19:25:41.298Z Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.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.Learnt from: haitham911 Repo: cloudposse/atmos PR: 727 File: internal/exec/terraform_clean.go:407-416 Timestamp: 2024-11-24T19:13:10.287Z Learning: In `internal/exec/terraform_clean.go`, when `getStackTerraformStateFolder` returns an error in the `handleCleanSubCommand` function, the error is logged, and the process continues without returning the error.Learnt from: osterman Repo: cloudposse/atmos PR: 727 File: internal/exec/terraform.go:65-66 Timestamp: 2024-10-31T19:23:45.538Z Learning: The variable `shouldCheckStack` in `ExecuteTerraform` controls whether validation is performed.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: 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.Learnt from: aknysh Repo: cloudposse/atmos PR: 863 File: internal/exec/yaml_func_terraform_output.go:34-38 Timestamp: 2024-12-17T07:08:41.288Z Learning: In the `processTagTerraformOutput` function within `internal/exec/yaml_func_terraform_output.go`, parameters are separated by spaces and do not contain spaces. Therefore, using `strings.Fields()` for parsing is acceptable, and there's no need to handle parameters with spaces.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.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.
393-448: Add language specifications to code blocks in Option 2. Lines 397–410 and 416–447 contain Go code without language specs. Also check lines around auth manager registry implementation.⛔ Skipped due to learnings
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 **/*.go : Document complex logic with inline commentsLearnt 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.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 **/*.go : All code must pass golangci-lint checksLearnt 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.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 appropriateLearnt 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 **/*.go : Follow Go error handling idioms and use meaningful error messages
177-219: Add language specifications to code blocks. Multiple code blocks in the proposed solutions section are missing language specs:
- Lines 150–159: Go code (needs
`go)- Lines 202–219: Go code (needs
`go)⛔ Skipped due to learnings
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 **/*.go : Document complex logic with inline commentsLearnt 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 **/*.go : All code must pass golangci-lint checksLearnt 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 **/*.go : Follow standard Go coding style; run gofmt and goimportsLearnt 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/terraform-yaml-functions-authentication-flow.md (2)
590-590: Add language specification to debug output code block.Line 590 shows a debug logging example without a language specifier. Use
`bashfor consistency with other shell examples in the document.-``` +```bash # Enable debug logging to see auth resolution
717-837: Add language specifications to error message code blocks.Lines 717–837 show error output examples without language specifiers. Update these blocks to use
`textor`plaintext, or`bashif they represent shell output:-``` +```text failed to execute terraform: no credentials availableAnd similarly for other error blocks at lines 723, 729, 737, 742, 749, 754, 820, and 837.
docs/fixes/nested-terraform-state-auth-context-propagation.md (3)
10-16: Add language specification to error message code block.Line 10 shows an error message without a language specifier. Use
`textor`bash:-``` +```text failed to read Terraform state for component tgw/hub in stack core-use2-network
19-28: Add language specification to debug output code block.Line 19 shows debug output without a language specifier. Use
`bash:-``` +```bash DEBU Authentication chain discovered identity=core-network/terraform
169-175: Add language specification to concept diagram.Line 169 shows a conceptual diagram (the "missing link" box) without a language specifier. Use
`textor`plaintext:-``` +```plaintext authContext (credentials) ← Derived from → AuthManager (authentication manager)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
docs/fixes/nested-terraform-state-auth-context-propagation.md(1 hunks)docs/terraform-yaml-functions-authentication-flow.md(1 hunks)internal/exec/terraform.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
internal/exec/terraform.go
🧠 Learnings (16)
📓 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: 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: 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.
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.
📚 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.godocs/terraform-yaml-functions-authentication-flow.md
📚 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.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/terraform.go
📚 Learning: 2024-11-24T19:13:10.287Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:407-416
Timestamp: 2024-11-24T19:13:10.287Z
Learning: In `internal/exec/terraform_clean.go`, when `getStackTerraformStateFolder` returns an error in the `handleCleanSubCommand` function, the error is logged, and the process continues without returning the error.
Applied to files:
internal/exec/terraform.go
📚 Learning: 2025-04-26T15:54:10.506Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 1195
File: internal/exec/terraform_clean.go:99-99
Timestamp: 2025-04-26T15:54:10.506Z
Learning: The error variable `ErrRelPath` is defined in `internal/exec/terraform_clean_util.go` and is used across files in the `exec` package, including in `terraform_clean.go`. This is part of an approach to standardize error handling in the codebase.
Applied to files:
internal/exec/terraform.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/terraform.go
📚 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.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-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:
internal/exec/terraform.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:
internal/exec/terraform.go
📚 Learning: 2024-12-03T04:01:16.446Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:0-0
Timestamp: 2024-12-03T04:01:16.446Z
Learning: In the `terraform.output.mdx` documentation file (`website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx`), the cache invalidation and cache scope behavior for the `!terraform.output` function are already described.
Applied to files:
docs/fixes/nested-terraform-state-auth-context-propagation.mddocs/terraform-yaml-functions-authentication-flow.md
📚 Learning: 2025-04-25T20:54:19.701Z
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
Applied to files:
docs/fixes/nested-terraform-state-auth-context-propagation.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:
docs/terraform-yaml-functions-authentication-flow.md
📚 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:
docs/terraform-yaml-functions-authentication-flow.md
📚 Learning: 2025-10-11T19:12:38.832Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: tests/snapshots/TestCLICommands_atmos_workflow_invalid_step_type.stderr.golden:0-0
Timestamp: 2025-10-11T19:12:38.832Z
Learning: Usage Examples sections in error output are appropriate for command usage errors (incorrect syntax, missing arguments, invalid flags) but not for configuration validation errors (malformed workflow files, invalid settings in atmos.yaml). Configuration errors should focus on explaining what's wrong with the config, not command usage patterns.
Applied to files:
docs/terraform-yaml-functions-authentication-flow.md
🧬 Code graph analysis (1)
internal/exec/terraform.go (6)
errors/errors.go (2)
ErrInvalidComponent(167-167)ErrUserAborted(461-461)pkg/auth/config_helpers.go (1)
MergeComponentAuthFromConfig(138-160)pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManager(180-222)pkg/auth/types/interfaces.go (2)
Identity(71-115)AuthManager(118-216)errors/error_funcs.go (1)
Exit(232-234)errors/exit_code.go (1)
ExitCodeSIGINT(12-12)
🪛 LanguageTool
docs/fixes/nested-terraform-state-auth-context-propagation.md
[grammar] ~346-~346: Please add a punctuation mark at the end of paragraph.
Context: ...aform.output` - All mock generators and tests Example in terraform.go: ```go fu...
(PUNCTUATION_PARAGRAPH_END)
[style] ~696-~696: Using many exclamation marks might seem excessive (in this case: 21 exclamation marks for a text that’s 9267 characters long)
Context: ...md- Comprehensive guide covering both!terraform.stateand!terraform.output` authentication, includ...
(EN_EXCESSIVE_EXCLAMATION)
docs/terraform-yaml-functions-authentication-flow.md
[grammar] ~204-~204: Please add a punctuation mark at the end of paragraph.
Context: ...ted levels** use the same authenticated session This ensures that deeply nested compon...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~401-~401: Please add a punctuation mark at the end of paragraph.
Context: ...MDS) - AWS credential files in standard locations ## Component-Level Auth Configuration ...
(PUNCTUATION_PARAGRAPH_END)
[uncategorized] ~599-~599: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... identity selection happens once at the top level - Default identity required - Compo...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~603-~603: Since ownership is already implied, this phrasing may be redundant.
Context: ...tial component, nested components use their own auth ## How Credentials Flow ### 1. A...
(PRP_OWN)
🪛 markdownlint-cli2 (0.18.1)
docs/fixes/nested-terraform-state-auth-context-propagation.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
169-169: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/terraform-yaml-functions-authentication-flow.md
590-590: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
717-717: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
723-723: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
729-729: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
737-737: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
742-742: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
749-749: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
754-754: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
820-820: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
837-837: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: website-deploy-preview
- GitHub Check: Review Dependency Licenses
- GitHub Check: autofix
- GitHub Check: Summary
🔇 Additional comments (10)
docs/terraform-yaml-functions-authentication-flow.md (3)
200-210: Add period to end of paragraph.Line 204 is missing punctuation at the end of the paragraph. Add a period after "authenticated session" on line 204.
-This ensures that deeply nested component configurations can execute terraform operations with proper credentials +This ensures that deeply nested component configurations can execute terraform operations with proper credentials. -without requiring separate authentication at each level. +Without requiring separate authentication at each level.[suggest_minor_issue]
396-402: Add period to end of paragraph.Line 401 ends a paragraph without punctuation. Add a period after "locations" and before the section heading.
-AWS credential files in standard locations +AWS credential files in standard locations.[suggest_minor_issue]
595-605: Hyphenate compound adjective.Line 599 uses "Single top-level identity" as a dash-separated phrase modifying the concept. For consistency with doc style, consider reformatting this to use a hyphen if it modifies a following noun, or keep as-is if it's a list item (current formatting is acceptable for bullet points).
docs/fixes/nested-terraform-state-auth-context-propagation.md (4)
341-347: Add punctuation after list intro.Line 346 introduces a list with a colon but the paragraph ends without proper continuation. The line reads "Files to update:" followed by a list; this is acceptable as-is for markdown list formatting.
1-30: Well-structured problem statement with clear reproduction case.The document effectively presents the problem with symptoms, debug output patterns, and a detailed reproduction case. The root-cause analysis with execution flow is thorough and makes the issue clear. Good foundational work for implementing the fix.
177-340: Clear solution comparison and recommended path.The comparison between Option 1 (ConfigAndStacksInfo) and Option 2 (Global Registry) is well-reasoned. Option 1 is properly justified as the recommended solution, with concrete implementation steps and code examples. The phased implementation plan is realistic and actionable.
602-697: Comprehensive component-level auth override feature documentation.The additional feature (component-level auth override) is well-integrated into the fix documentation. The use case example clearly demonstrates cross-account scenarios, and testing strategy with concrete test names provides good guidance for implementation.
internal/exec/terraform.go (3)
87-94: Good UX improvement: component validation before auth.The early exit on
ErrInvalidComponentprevents prompting for credentials when the component doesn't exist. The fallback to global auth config for other errors (like permission issues) is a sensible degradation strategy.
108-111: Correct handling of user abort.Exiting immediately with SIGINT code 130 when the user aborts is the right POSIX behavior. This prevents cascading error messages when authentication is deliberately cancelled.
128-131: Core auth propagation mechanism in place.Storing the
AuthManagerininfoenables nested YAML functions (like!terraform.state) to reuse the authenticated session without re-prompting. This is the key enabler for the auth context propagation described in the PR objectives.
|
These changes were released in v1.199.0-rc.1. |
what
!terraform.stateand!terraform.outputYAML function evaluationsAuthManagerfield toConfigAndStacksInfostruct to enable auth propagation through the execution pipelineauth:configurationTerraformStateGetterandTerraformOutputGetterinterfaces to acceptauthManagerparameterwhy
Problem 1: Nested Authentication Propagation
When executing Atmos commands with authentication enabled, nested
!terraform.statefunctions failed with IMDS timeout errors even though the top-level command had valid authenticated credentials. This occurred when a component's configuration contained!terraform.statefunctions that referenced other components which themselves contained!terraform.statefunctions.Root Cause: The
GetTerraformState()function received anauthContextparameter but did not have access to theAuthManager. When processing nested components, it calledExecuteDescribeComponent()without anAuthManager, breaking the authentication chain at level 2+ of nesting.Example Failure:
Solution: Added
AuthManagertoConfigAndStacksInfostruct and threaded it through the entire execution pipeline, enabling all nested function evaluations to access authenticated credentials. Additionally implemented component-level auth override to support cross-account state reading in nested scenarios.Problem 2: Identity Selector Exit Handling
When the identity selector appeared (either from
--identityflag without value, or when processing YAML functions with no default identity configured), pressing Ctrl+C would not exit the program. Instead:huhTUI library but returnedErrUserAbortedautoDetectDefaultIdentity()function intentionally swallowed ALL errors (includingErrUserAborted) for "backward compatibility"("", nil), causing execution to continue without authenticationRoot Cause: The error handling in
pkg/auth/manager_helpers.go:autoDetectDefaultIdentity()was catchingErrUserAbortedfrom the identity selector and converting it to a successful empty result for backward compatibility, preventing proper exit handling.Solution:
autoDetectDefaultIdentity()to propagateErrUserAbortedwhile preserving backward compatibility for other errorsterraform.goandterraform_utils.goto immediately exit with code 130 when user abortsExitCodeSIGINT = 130for POSIX-compliant signal exit codesProblem 3: Authentication Before Component Validation
When running a command with an invalid component name, Atmos would prompt for identity selection before checking if the component exists:
Root Cause: The authentication flow in
ExecuteTerraform()was callingCreateAndAuthenticateManager()before the component existence check, causing unnecessary user interaction for invalid components.Solution: Modified the component auth config retrieval logic to immediately exit if
ExecuteDescribeComponent()returnsErrInvalidComponent, preventing authentication attempts for non-existent components.Benefits:
!terraform.statefunctions now work at any depth with proper authenticationreferences
--identityflag)docs/fixes/nested-terraform-state-auth-context-propagation.mdfor detailed technical analysis and implementation designSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores