-
-
Notifications
You must be signed in to change notification settings - Fork 140
fix: Enable YAML function authentication in terraform commands with --identity flag #1769
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. |
Dependency ReviewThe following issues were found:
License Issuesgo.mod
Scanned Files
|
|
Warning Rate limit exceeded@aknysh has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 54 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (39)
📝 WalkthroughWalkthroughThreads an authenticated AuthManager through stack/component processing by adding authManager parameters, centralizes identity resolution/authentication via auth.CreateAndAuthenticateManager, exports logger setup, adds auth config merge helpers and tests, updates ProcessStacks call sites, docs, deps, and CLI/flag adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI
participant TF as Terraform exec
participant Auth as auth.CreateAndAuthenticateManager
participant PS as ProcessStacks
participant PCC as ProcessComponentConfig
participant YAML as YAML functions
CLI->>TF: invoke execution (identity optional)
TF->>TF: merge component auth with global auth
TF->>Auth: CreateAndAuthenticateManager(identity, mergedAuth)
Auth->>Auth: resolve identity (explicit/auto/interactive/disable)
Auth->>Auth: create AuthManager & Authenticate()
Auth-->>TF: return authenticated AuthManager (or nil)
TF->>PS: ProcessStacks(..., authManager)
PS->>PCC: ProcessComponentConfig(..., authManager)
PCC->>PCC: copy AuthContext from authManager into ConfigAndStacksInfo
PCC->>YAML: evaluate YAML functions with AuthContext
YAML->>YAML: perform !terraform.state / !terraform.output using credentials
sequenceDiagram
autonumber
participant App as App Init
participant Config as InitCliConfig
participant Log as setLogConfig
participant Stacks as processStackConfigs
participant Hooks as Pre-hooks
App->>Config: initialize CLI config
Config->>Log: setLogConfig (reads config/env/flags)
Log->>Log: set global logger level
Config->>Stacks: process stack configs
Stacks->>Hooks: run pre-hooks (now use configured log level)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to focus during review:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 2
🧹 Nitpick comments (2)
pkg/config/config.go (1)
117-132: Consider potential duplication with cmd/root.go SetupLogger.The logger configuration logic here (Lines 119-132) appears similar to the
SetupLoggerfunction incmd/root.go(Lines 287-300). Both perform explicit log level configuration with switch statements handling the same levels.While the timing difference justifies separate calls (early config initialization vs. full logger setup), verify whether this creates maintenance burden with duplicated logic. If both code paths must remain, consider extracting the level-setting logic to a shared helper function.
#!/bin/bash # Compare the log level handling between pkg/config/config.go and cmd/root.go echo "=== pkg/config/config.go setLogConfig ===" rg -A 15 "func setLogConfig" pkg/config/config.go echo -e "\n=== cmd/root.go SetupLogger ===" rg -A 15 "func SetupLogger" cmd/root.godocs/prd/terraform-output-authentication-flow.md (1)
1-261: Excellent documentation of the authentication flow.This doc provides clear end-to-end context for how
!terraform.outputauthentication works, from command execution through credential injection into the Terraform binary. The architecture diagram and critical code sections make it easy to understand the fix.Minor: Consider adding a language identifier to the fenced code block at line 12 for better syntax highlighting:
-``` +```text Terraform Command Execution (with --identity flag)
📜 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 (26)
cmd/identity_flag.go(1 hunks)cmd/root.go(2 hunks)cmd/root_test.go(4 hunks)docs/prd/terraform-command-yaml-function-authentication.md(1 hunks)docs/prd/terraform-output-authentication-flow.md(1 hunks)go.mod(1 hunks)internal/exec/aws_eks_update_kubeconfig.go(2 hunks)internal/exec/describe_component.go(1 hunks)internal/exec/helmfile.go(1 hunks)internal/exec/helmfile_generate_varfile.go(1 hunks)internal/exec/packer.go(1 hunks)internal/exec/packer_output.go(1 hunks)internal/exec/terraform.go(1 hunks)internal/exec/terraform_generate_backend.go(1 hunks)internal/exec/terraform_generate_planfile.go(1 hunks)internal/exec/terraform_generate_varfile.go(1 hunks)internal/exec/terraform_output_utils.go(2 hunks)internal/exec/utils.go(6 hunks)internal/exec/validate_component.go(1 hunks)pkg/auth/hooks.go(1 hunks)pkg/auth/manager_helpers.go(1 hunks)pkg/auth/manager_helpers_test.go(1 hunks)pkg/component/component_processor.go(1 hunks)pkg/config/config.go(3 hunks)pkg/utils/yaml_utils.go(1 hunks)website/src/components/Screengrabs/demo-stacks/write-your-components.ansi(0 hunks)
💤 Files with no reviewable changes (1)
- website/src/components/Screengrabs/demo-stacks/write-your-components.ansi
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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_generate_varfile.gointernal/exec/terraform_generate_planfile.gocmd/identity_flag.gopkg/auth/manager_helpers.gointernal/exec/terraform_generate_backend.gointernal/exec/terraform_output_utils.gopkg/auth/manager_helpers_test.gocmd/root_test.gocmd/root.gointernal/exec/terraform.gointernal/exec/utils.gointernal/exec/packer_output.gopkg/component/component_processor.gopkg/utils/yaml_utils.gointernal/exec/helmfile.gointernal/exec/helmfile_generate_varfile.gointernal/exec/aws_eks_update_kubeconfig.gopkg/config/config.gointernal/exec/packer.gopkg/auth/hooks.gointernal/exec/describe_component.gointernal/exec/validate_component.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_generate_varfile.gointernal/exec/terraform_generate_planfile.gocmd/identity_flag.gopkg/auth/manager_helpers.gointernal/exec/terraform_generate_backend.gointernal/exec/terraform_output_utils.gocmd/root.gointernal/exec/terraform.gointernal/exec/utils.gointernal/exec/packer_output.gopkg/component/component_processor.gopkg/utils/yaml_utils.gointernal/exec/helmfile.gointernal/exec/helmfile_generate_varfile.gointernal/exec/aws_eks_update_kubeconfig.gopkg/config/config.gointernal/exec/packer.gopkg/auth/hooks.gointernal/exec/describe_component.gointernal/exec/validate_component.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands
Implement each CLI command in a separate file under cmd/
Use Viper for managing configuration, environment variables, and flags in the CLI
Keep separation of concerns between CLI interface (cmd) and business logic
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in Cobra command help
Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Follow single responsibility; separate command interface from business logic
Provide meaningful user feedback and include progress indicators for long-running operations
Provide clear error messages to users and troubleshooting hints where appropriate
Files:
cmd/identity_flag.gocmd/root_test.gocmd/root.go
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/auth/manager_helpers.gopkg/auth/manager_helpers_test.gopkg/component/component_processor.gopkg/utils/yaml_utils.gopkg/config/config.gopkg/auth/hooks.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.gocmd/root_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
🧠 Learnings (70)
📓 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.
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-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
internal/exec/terraform_generate_varfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_backend.gocmd/root.gointernal/exec/terraform.gointernal/exec/utils.gointernal/exec/packer_output.gopkg/component/component_processor.gointernal/exec/helmfile.gointernal/exec/helmfile_generate_varfile.gointernal/exec/aws_eks_update_kubeconfig.gopkg/config/config.gointernal/exec/packer.gointernal/exec/validate_component.go
📚 Learning: 2024-10-30T13:25:45.965Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:405-412
Timestamp: 2024-10-30T13:25:45.965Z
Learning: In `internal/exec/terraform_clean.go`, when appending `stackFolders` to `folders` in the `handleCleanSubCommand` function, it's unnecessary to check if `stackFolders` is nil before appending, because in Go, appending a nil slice is safe and does not cause a panic.
Applied to files:
internal/exec/terraform_generate_varfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_backend.gopkg/component/component_processor.gointernal/exec/helmfile.gointernal/exec/helmfile_generate_varfile.gointernal/exec/packer.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_generate_varfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_backend.gointernal/exec/terraform.gointernal/exec/helmfile.gointernal/exec/helmfile_generate_varfile.gointernal/exec/validate_component.go
📚 Learning: 2024-11-19T23:00:45.899Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.
Applied to files:
internal/exec/terraform_generate_varfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_backend.gointernal/exec/packer_output.gopkg/component/component_processor.gointernal/exec/helmfile.gointernal/exec/helmfile_generate_varfile.gointernal/exec/aws_eks_update_kubeconfig.gointernal/exec/packer.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_generate_varfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_backend.gointernal/exec/terraform.gointernal/exec/aws_eks_update_kubeconfig.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
internal/exec/terraform_generate_varfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_backend.gointernal/exec/utils.gointernal/exec/packer_output.gopkg/component/component_processor.gointernal/exec/helmfile.gointernal/exec/helmfile_generate_varfile.gointernal/exec/aws_eks_update_kubeconfig.gointernal/exec/packer.gointernal/exec/validate_component.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_generate_varfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_backend.gointernal/exec/terraform_output_utils.gointernal/exec/aws_eks_update_kubeconfig.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_generate_varfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_backend.gointernal/exec/terraform.gointernal/exec/packer_output.go
📚 Learning: 2024-10-31T19:23:45.538Z
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.
Applied to files:
internal/exec/terraform_generate_varfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_backend.gointernal/exec/terraform.gointernal/exec/aws_eks_update_kubeconfig.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_generate_varfile.gointernal/exec/terraform_generate_planfile.gointernal/exec/terraform_generate_backend.gointernal/exec/terraform.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_generate_varfile.gointernal/exec/terraform_generate_planfile.gocmd/identity_flag.gointernal/exec/terraform_generate_backend.gocmd/root.gointernal/exec/utils.gointernal/exec/packer_output.gointernal/exec/helmfile.gointernal/exec/helmfile_generate_varfile.gointernal/exec/aws_eks_update_kubeconfig.gopkg/config/config.gointernal/exec/packer.gointernal/exec/validate_component.go
📚 Learning: 2025-09-25T01:02:48.697Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/manager.go:304-312
Timestamp: 2025-09-25T01:02:48.697Z
Learning: The auth manager in pkg/auth/manager.go should remain cloud-agnostic and not contain AWS-specific logic or references to specific cloud providers. Keep the manager generic and extensible.
Applied to files:
cmd/identity_flag.gopkg/auth/manager_helpers.gopkg/auth/manager_helpers_test.gointernal/exec/terraform.gointernal/exec/utils.go
📚 Learning: 2025-09-07T18:07:00.549Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
Applied to files:
cmd/identity_flag.gointernal/exec/terraform.gointernal/exec/utils.gopkg/config/config.gopkg/auth/hooks.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Viper for managing configuration, environment variables, and flags in the CLI
Applied to files:
cmd/identity_flag.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
cmd/identity_flag.gocmd/root.gointernal/exec/utils.gointernal/exec/aws_eks_update_kubeconfig.gopkg/config/config.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands
Applied to files:
cmd/identity_flag.gocmd/root_test.gocmd/root.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
cmd/identity_flag.gocmd/root.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Applied to files:
cmd/identity_flag.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.
Applied to files:
cmd/identity_flag.gopkg/config/config.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
cmd/identity_flag.gocmd/root.gointernal/exec/utils.gointernal/exec/aws_eks_update_kubeconfig.gopkg/config/config.go
📚 Learning: 2025-01-18T15:15:41.645Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform.go:37-46
Timestamp: 2025-01-18T15:15:41.645Z
Learning: In the atmos CLI, error handling is intentionally structured to use LogErrorAndExit for consistent error display, avoiding Cobra's default error handling to prevent duplicate error messages.
Applied to files:
cmd/identity_flag.gocmd/root.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_output_utils.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/terraform_output_utils.godocs/prd/terraform-output-authentication-flow.md
📚 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/utils.gointernal/exec/aws_eks_update_kubeconfig.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 meaningful user feedback and include progress indicators for long-running operations
Applied to files:
internal/exec/terraform_output_utils.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
📚 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.gocmd/root_test.gopkg/config/config.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.gocmd/root_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 cmd/**/*.go : Keep separation of concerns between CLI interface (cmd) and business logic
Applied to files:
cmd/root_test.go
📚 Learning: 2024-11-10T19:28:17.365Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:0-0
Timestamp: 2024-11-10T19:28:17.365Z
Learning: When TTY is not supported, log the downgrade message at the Warn level using `u.LogWarning(cliConfig, ...)` instead of `fmt.Println`.
Applied to files:
cmd/root_test.go
📚 Learning: 2025-01-30T16:56:43.004Z
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 980
File: pkg/utils/log_utils.go:62-63
Timestamp: 2025-01-30T16:56:43.004Z
Learning: In pkg/utils/log_utils.go, LogTrace is intentionally redirected to LogDebug since charmbracelet logger doesn't support Trace level, maintaining backward compatibility with the original LogTrace functionality.
Applied to files:
cmd/root_test.gopkg/utils/yaml_utils.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:
cmd/root.gointernal/exec/utils.gointernal/exec/aws_eks_update_kubeconfig.gopkg/config/config.go
📚 Learning: 2024-12-13T15:28:13.630Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/version.go:34-44
Timestamp: 2024-12-13T15:28:13.630Z
Learning: In `cmd/version.go`, when handling the `--check` flag in the `versionCmd`, avoid using `CheckForAtmosUpdateAndPrintMessage(cliConfig)` as it updates the cache timestamp, which may not be desired in this context.
Applied to files:
cmd/root.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:
cmd/root.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:
cmd/root.gointernal/exec/utils.gopkg/config/config.go
📚 Learning: 2025-02-03T05:57:18.407Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/cmd_utils.go:121-148
Timestamp: 2025-02-03T05:57:18.407Z
Learning: The Atmos CLI should fail fast (exit) when encountering configuration errors, including command alias configuration issues, to prevent undefined behavior. Use LogErrorAndExit instead of returning errors in such cases.
Applied to files:
cmd/root.gopkg/config/config.go
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
cmd/root.gopkg/component/component_processor.gointernal/exec/helmfile.gointernal/exec/aws_eks_update_kubeconfig.gointernal/exec/packer.gointernal/exec/validate_component.go
📚 Learning: 2024-12-11T18:46:02.483Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/terraform.go:39-39
Timestamp: 2024-12-11T18:46:02.483Z
Learning: `cliConfig` is initialized in `cmd/root.go` and can be used across the `cmd` package.
Applied to files:
cmd/root.go
📚 Learning: 2025-02-21T20:56:05.539Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1077
File: pkg/downloader/custom_github_detector.go:0-0
Timestamp: 2025-02-21T20:56:05.539Z
Learning: The `github.com/charmbracelet/log` package should be imported with the alias `log` according to the project's import alias configuration.
Applied to files:
cmd/root.gopkg/config/config.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.godocs/prd/terraform-output-authentication-flow.md
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
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.gointernal/exec/aws_eks_update_kubeconfig.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/utils.gointernal/exec/packer_output.gopkg/component/component_processor.gointernal/exec/describe_component.gointernal/exec/validate_component.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/utils.gointernal/exec/aws_eks_update_kubeconfig.gopkg/config/config.gogo.mod
📚 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/utils.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
internal/exec/utils.gointernal/exec/packer_output.gointernal/exec/aws_eks_update_kubeconfig.gopkg/config/config.gointernal/exec/packer.go
📚 Learning: 2025-05-22T15:42:10.906Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1261
File: internal/exec/utils.go:639-640
Timestamp: 2025-05-22T15:42:10.906Z
Learning: In the Atmos codebase, when appending slices with `args := append(configAndStacksInfo.CliArgs, configAndStacksInfo.AdditionalArgsAndFlags...)`, it's intentional that the result is not stored back in the original slice. This pattern is used when the merged result serves a different purpose than the original slices, such as when creating a filtered version for component section assignments.
Applied to files:
internal/exec/packer_output.gopkg/component/component_processor.gointernal/exec/helmfile_generate_varfile.gointernal/exec/packer.gointernal/exec/validate_component.go
📚 Learning: 2025-10-13T18:13:54.020Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1622
File: pkg/perf/perf.go:140-184
Timestamp: 2025-10-13T18:13:54.020Z
Learning: In pkg/perf/perf.go, the `trackWithSimpleStack` function intentionally skips ownership checks at call stack depth > 1 to avoid expensive `getGoroutineID()` calls on every nested function. This is a performance optimization for the common single-goroutine execution case (most Atmos commands), accepting the rare edge case of potential metric corruption if multi-goroutine execution occurs at depth > 1. The ~19× performance improvement justifies this trade-off.
Applied to files:
internal/exec/packer_output.gopkg/component/component_processor.gointernal/exec/helmfile.gointernal/exec/helmfile_generate_varfile.gointernal/exec/aws_eks_update_kubeconfig.gointernal/exec/packer.gointernal/exec/validate_component.go
📚 Learning: 2025-10-08T06:48:07.499Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1602
File: internal/exec/stack_processor_utils.go:968-1003
Timestamp: 2025-10-08T06:48:07.499Z
Learning: The `FindComponentDependenciesLegacy` function in `internal/exec/stack_processor_utils.go` is legacy code that is not actively used and is kept only for backward compatibility purposes.
Applied to files:
pkg/component/component_processor.gointernal/exec/helmfile_generate_varfile.go
📚 Learning: 2025-08-16T23:33:07.477Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:651-652
Timestamp: 2025-08-16T23:33:07.477Z
Learning: In the cloudposse/atmos Go codebase, ExecuteDescribeDependents expects a pointer to AtmosConfiguration (*schema.AtmosConfiguration), so when calling it with a value returned by cfg.InitCliConfig (which returns schema.AtmosConfiguration), the address-of operator (&) is necessary: ExecuteDescribeDependents(&atmosConfig, ...).
Applied to files:
pkg/component/component_processor.gointernal/exec/helmfile.gointernal/exec/aws_eks_update_kubeconfig.gointernal/exec/packer.gointernal/exec/validate_component.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/prd/terraform-command-yaml-function-authentication.mddocs/prd/terraform-output-authentication-flow.md
📚 Learning: 2024-10-20T00:41:57.135Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:93-98
Timestamp: 2024-10-20T00:41:57.135Z
Learning: When downloading schema files in `internal/exec/validate_stacks.go`, use a consistent temporary file name to overwrite the file each time and avoid creating multiple temporary files.
Applied to files:
internal/exec/helmfile_generate_varfile.gointernal/exec/validate_component.go
📚 Learning: 2025-01-09T22:27:25.538Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/validate_stacks.go:20-23
Timestamp: 2025-01-09T22:27:25.538Z
Learning: The validate commands in Atmos can have different help handling implementations. Specifically, validate_component.go and validate_stacks.go are designed to handle help requests differently, with validate_stacks.go including positional argument checks while validate_component.go does not.
Applied to files:
internal/exec/aws_eks_update_kubeconfig.gointernal/exec/validate_component.go
📚 Learning: 2024-10-27T04:34:08.011Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:183-187
Timestamp: 2024-10-27T04:34:08.011Z
Learning: In the `getStackTerraformStateFolder` function, it's acceptable and not an error if no Terraform state folders are found for a stack.
Applied to files:
internal/exec/aws_eks_update_kubeconfig.go
📚 Learning: 2025-02-21T20:56:20.761Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1077
File: pkg/downloader/custom_github_detector_test.go:0-0
Timestamp: 2025-02-21T20:56:20.761Z
Learning: The `github.com/charmbracelet/log` package should be imported with the alias `log`, not `clog`.
Applied to files:
pkg/config/config.go
📚 Learning: 2025-03-12T21:38:42.699Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1139
File: pkg/config/go-homedir/homedir.go:183-196
Timestamp: 2025-03-12T21:38:42.699Z
Learning: The code in pkg/config/go-homedir is a direct fork of the mitchellh/go-homedir package and was intentionally imported as-is without modifications to maintain consistency with the original source. Security concerns or other improvements may be addressed in future PRs.
Applied to files:
pkg/config/config.go
📚 Learning: 2024-12-05T22:33:54.807Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 820
File: cmd/list_stacks.go:55-56
Timestamp: 2024-12-05T22:33:54.807Z
Learning: In the atmos project, the `u.LogErrorAndExit` function logs the error and exits the command execution appropriately within flag completion functions.
Applied to files:
pkg/config/config.go
📚 Learning: 2025-09-05T14:57:37.360Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 1448
File: cmd/ansible.go:26-28
Timestamp: 2025-09-05T14:57:37.360Z
Learning: The Atmos codebase uses a consistent pattern for commands that delegate to external tools: `PersistentFlags().Bool("", false, doubleDashHint)` where doubleDashHint provides help text about using double dashes to separate Atmos options from native command arguments. This pattern is used across terraform, packer, helmfile, atlantis, aws, and ansible commands.
Applied to files:
pkg/config/config.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/prd/terraform-output-authentication-flow.md
📚 Learning: 2024-12-03T03:49:30.395Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:104-110
Timestamp: 2024-12-03T03:49:30.395Z
Learning: In the documentation for `!terraform.output`, warnings about template variable availability are already covered in other sections, so no need to suggest adding them here.
Applied to files:
docs/prd/terraform-output-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/prd/terraform-output-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/prd/terraform-output-authentication-flow.md
📚 Learning: 2024-12-03T03:52:02.524Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:144-145
Timestamp: 2024-12-03T03:52:02.524Z
Learning: Avoid adding context timeouts to Terraform commands in `execTerraformOutput` because their execution time can vary from seconds to hours, and making it configurable would require redesigning the command interface.
Applied to files:
docs/prd/terraform-output-authentication-flow.md
📚 Learning: 2024-10-20T00:57:53.500Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:0-0
Timestamp: 2024-10-20T00:57:53.500Z
Learning: In `internal/exec/validate_stacks.go`, when downloading the Atmos JSON Schema file to the temp directory, the temporary file is overwritten each time, so explicit removal is not necessary.
Applied to files:
internal/exec/packer.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: 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-04-11T22:06:46.999Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
internal/exec/validate_component.go
🧬 Code graph analysis (22)
internal/exec/terraform_generate_varfile.go (1)
internal/exec/utils.go (1)
ProcessStacks(308-742)
internal/exec/terraform_generate_planfile.go (1)
internal/exec/utils.go (1)
ProcessStacks(308-742)
cmd/identity_flag.go (4)
pkg/schema/schema_auth.go (1)
AuthConfig(4-12)pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManager(30-75)pkg/config/const.go (1)
IdentityFlagSelectValue(126-126)cmd/auth.go (1)
IdentityFlagSelectValue(14-14)
pkg/auth/manager_helpers.go (6)
pkg/schema/schema_auth.go (1)
AuthConfig(4-12)errors/errors.go (2)
ErrAuthNotConfigured(382-382)ErrFailedToInitializeAuthManager(400-400)pkg/schema/schema.go (2)
ConfigAndStacksInfo(544-627)AuthContext(515-523)pkg/auth/credentials/store.go (1)
NewCredentialStore(42-46)pkg/auth/validation/validator.go (1)
NewValidator(19-21)pkg/auth/manager.go (1)
NewAuthManager(98-141)
internal/exec/terraform_generate_backend.go (1)
internal/exec/utils.go (1)
ProcessStacks(308-742)
internal/exec/terraform_output_utils.go (3)
pkg/logger/log.go (2)
Debug(24-26)Level(203-203)pkg/utils/json_utils.go (1)
ConvertFromJSON(170-187)pkg/schema/schema.go (1)
Logs(421-424)
pkg/auth/manager_helpers_test.go (7)
pkg/schema/schema_auth.go (2)
AuthConfig(4-12)IdentityVia(60-63)pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManager(30-75)errors/errors.go (4)
ErrAuthNotConfigured(382-382)ErrIdentityNotFound(408-408)ErrIdentityNotInConfig(443-443)ErrIdentitySelectionRequiresTTY(414-414)pkg/auth/credentials/store.go (1)
NewCredentialStore(42-46)pkg/auth/validation/validator.go (1)
NewValidator(19-21)pkg/schema/schema.go (2)
ConfigAndStacksInfo(544-627)AuthContext(515-523)pkg/auth/manager.go (1)
NewAuthManager(98-141)
cmd/root_test.go (1)
cmd/root.go (1)
SetupLogger(286-357)
cmd/root.go (1)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)
internal/exec/terraform.go (3)
pkg/auth/manager_helpers.go (1)
CreateAndAuthenticateManager(30-75)pkg/config/const.go (1)
IdentityFlagSelectValue(126-126)internal/exec/utils.go (1)
ProcessStacks(308-742)
internal/exec/utils.go (2)
pkg/auth/types/interfaces.go (1)
AuthManager(118-216)pkg/schema/schema.go (1)
AuthContext(515-523)
internal/exec/packer_output.go (1)
internal/exec/utils.go (1)
ProcessStacks(308-742)
pkg/component/component_processor.go (2)
internal/exec/utils.go (1)
ProcessStacks(308-742)pkg/config/const.go (2)
HelmfileComponentType(49-49)PackerComponentType(50-50)
pkg/utils/yaml_utils.go (1)
pkg/logger/log.go (1)
Debug(24-26)
internal/exec/helmfile.go (1)
internal/exec/utils.go (1)
ProcessStacks(308-742)
internal/exec/helmfile_generate_varfile.go (1)
internal/exec/utils.go (1)
ProcessStacks(308-742)
internal/exec/aws_eks_update_kubeconfig.go (1)
internal/exec/utils.go (1)
ProcessStacks(308-742)
pkg/config/config.go (2)
pkg/schema/schema.go (1)
Logs(421-424)pkg/logger/log.go (6)
Level(203-203)SetLevel(84-86)TraceLevel(211-211)DebugLevel(213-213)InfoLevel(215-215)WarnLevel(217-217)
internal/exec/packer.go (1)
internal/exec/utils.go (1)
ProcessStacks(308-742)
pkg/auth/hooks.go (1)
pkg/logger/log.go (1)
Debug(24-26)
internal/exec/describe_component.go (1)
internal/exec/utils.go (1)
ProcessStacks(308-742)
internal/exec/validate_component.go (1)
internal/exec/utils.go (1)
ProcessStacks(308-742)
🪛 LanguageTool
docs/prd/terraform-command-yaml-function-authentication.md
[grammar] ~368-~368: Please add a punctuation mark at the end of paragraph.
Context: ...es profile-based credentials instead of IMDS 4. ✅ COMPLETE - Multi-account role...
(PUNCTUATION_PARAGRAPH_END)
[typographical] ~402-~402: Consider using a typographic opening quote here.
Context: ...leted - Verified no other instances of "create but not authenticate" bug - All a...
(EN_QUOTES)
[typographical] ~402-~402: Consider using a typographic close quote here.
Context: ...nstances of "create but not authenticate" bug - All authentication code paths rev...
(EN_QUOTES)
[typographical] ~650-~650: Consider using a typographic opening quote here.
Context: ...ase to ensure no other instances of the "create but not authenticate" bug: **Loc...
(EN_QUOTES)
[typographical] ~650-~650: Consider using a typographic close quote here.
Context: ...nces of the "create but not authenticate" bug: **Locations Where AuthManager is ...
(EN_QUOTES)
docs/prd/terraform-output-authentication-flow.md
[grammar] ~83-~83: Please add a punctuation mark at the end of paragraph.
Context: ...ole assumption using AWS SDK within the binary ## Critical Code Sections ### 1. Auth...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~165-~165: Please add a punctuation mark at the end of paragraph.
Context: ...ig populates stackInfo.AuthContext from AuthManager ✅ YAML Function Access: processTag...
(PUNCTUATION_PARAGRAPH_END)
[style] ~254-~254: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 2947 characters long)
Context: ...ion Our fix correctly handles both !terraform.state and !terraform.output YAML functions: 1. **...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/terraform-command-yaml-function-authentication.md
56-56: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
81-81: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
413-413: Bare URL used
(MD034, no-bare-urls)
414-414: Bare URL used
(MD034, no-bare-urls)
619-619: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
640-640: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/prd/terraform-output-authentication-flow.md
12-12: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
94-94: Hard tabs
Column: 1
(MD010, no-hard-tabs)
95-95: Hard tabs
Column: 1
(MD010, no-hard-tabs)
96-96: Hard tabs
Column: 1
(MD010, no-hard-tabs)
97-97: Hard tabs
Column: 1
(MD010, no-hard-tabs)
109-109: Hard tabs
Column: 1
(MD010, no-hard-tabs)
122-122: Hard tabs
Column: 1
(MD010, no-hard-tabs)
123-123: Hard tabs
Column: 1
(MD010, no-hard-tabs)
124-124: Hard tabs
Column: 1
(MD010, no-hard-tabs)
125-125: Hard tabs
Column: 1
(MD010, no-hard-tabs)
126-126: Hard tabs
Column: 1
(MD010, no-hard-tabs)
128-128: Hard tabs
Column: 1
(MD010, no-hard-tabs)
129-129: Hard tabs
Column: 1
(MD010, no-hard-tabs)
130-130: Hard tabs
Column: 1
(MD010, no-hard-tabs)
131-131: Hard tabs
Column: 1
(MD010, no-hard-tabs)
132-132: Hard tabs
Column: 1
(MD010, no-hard-tabs)
133-133: Hard tabs
Column: 1
(MD010, no-hard-tabs)
134-134: Hard tabs
Column: 1
(MD010, no-hard-tabs)
135-135: Hard tabs
Column: 1
(MD010, no-hard-tabs)
136-136: Hard tabs
Column: 1
(MD010, no-hard-tabs)
137-137: Hard tabs
Column: 1
(MD010, no-hard-tabs)
148-148: Hard tabs
Column: 1
(MD010, no-hard-tabs)
149-149: Hard tabs
Column: 1
(MD010, no-hard-tabs)
150-150: Hard tabs
Column: 1
(MD010, no-hard-tabs)
151-151: Hard tabs
Column: 1
(MD010, no-hard-tabs)
200-200: 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). (12)
- GitHub Check: Build (macos)
- GitHub Check: Build (linux)
- GitHub Check: Build (windows)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Review Dependency Licenses
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Run pre-commit hooks
- GitHub Check: autofix
- GitHub Check: website-deploy-preview
- 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: 1
📜 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 (1)
NOTICE(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 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-11-01T20:24:29.557Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1714
File: NOTICE:0-0
Timestamp: 2025-11-01T20:24:29.557Z
Learning: In the cloudposse/atmos repository, the NOTICE file is programmatically generated and should not be manually edited. Issues with dependency license URLs in NOTICE will be resolved when upstream package metadata is corrected.
Applied to files:
NOTICE
🪛 GitHub Actions: Dependency Review
NOTICE
[error] 1-1: NOTICE file is out of date. Run './scripts/generate-notice.sh' locally and commit the changes.
⏰ 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). (4)
- GitHub Check: Build (windows)
- GitHub Check: Build (linux)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
Addresses coderabbitai review comments on PR #1769: 1. Fix authManager not populated in detectComponentType - Added authManager: params.AuthManager to baseParams initialization - This ensures describe component commands properly thread AuthManager - Fixes auth pre-hooks not receiving AuthManager when using --identity flag 2. Regenerate NOTICE file - Updated with ./scripts/generate-notice.sh - Resolves pipeline failure from outdated NOTICE file 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
@coderabbitai full review please (and check if all your commends have been addressed) |
|
@aknysh: I'll perform a full review of the PR and verify that all previous comments have been addressed. ✅ Actions performedFull review triggered. |
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 (4)
pkg/auth/config_helpers_test.go (1)
122-137: Add an assertion for IdentityCaseMap rebuildOnce IdentityCaseMap picks up component identities, please extend this test (or add its own case) to assert the merged config maps the lowercase key back to the original component identity. That keeps the behavior from regressing when we touch the merge helper again.
internal/exec/terraform.go (1)
76-95: Consider logging when component config fetch fails.The silent fallback to global auth config when
ExecuteDescribeComponentfails is intentional and safe, but adding a debug log would help troubleshooting.Add logging for the error case:
}) if err == nil { // Merge component-specific auth with global auth. mergedAuthConfig, err = auth.MergeComponentAuthFromConfig(&atmosConfig.Auth, componentConfig, &atmosConfig, cfg.AuthSectionName) if err != nil { return err } + } else { + log.Debug("Failed to get component config for auth merging, using global auth", "component", info.ComponentFromArg, "error", err) } // If error getting component config, continue with global auth config. }docs/terraform-state-yaml-func-authentication-flow.md (2)
232-266: Replace hard tabs with spaces in YAML examples.Lines 242-266 use hard tabs. Use spaces for consistency and to avoid potential YAML parsing issues.
Based on coding guidelines
Apply this pattern:
# atmos.yaml auth: - identities: - dev: - default: true - kind: aws/permission-set - via: - provider: aws-sso + identities: + dev: + default: true + kind: aws/permission-set + via: + provider: aws-sso
306-337: Add language specifiers to fenced code blocks.Code blocks at lines 306, 320, and 331-336 are missing language identifiers, which disables syntax highlighting.
Add language identifiers:
Example state structure: -``` +```json { "resources": [{ ... }Access with:
-+yaml
vpc_id: !terraform.state vpc vpc_idEnable debug logging: -``` +```bash atmos terraform plan component -s stack --logs-level=Debug ATMOS_LOGS_LEVEL=Debug atmos terraform plan component -s stack</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **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. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between c08ae7398e2ef9530dd82912b4488377ec634f99 and f4cae61213878a4333d50444b84fe2e00c6e1601. </details> <details> <summary>📒 Files selected for processing (10)</summary> * `docs/prd/terraform-command-yaml-function-authentication.md` (1 hunks) * `docs/terraform-output-yaml-func-authentication-flow.md` (1 hunks) * `docs/terraform-state-yaml-func-authentication-flow.md` (1 hunks) * `errors/errors.go` (1 hunks) * `internal/exec/terraform.go` (1 hunks) * `pkg/auth/config_helpers.go` (1 hunks) * `pkg/auth/config_helpers_test.go` (1 hunks) * `pkg/auth/manager_helpers.go` (1 hunks) * `pkg/auth/manager_helpers_test.go` (1 hunks) * `pkg/utils/yaml_utils.go` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (2)</summary> * pkg/utils/yaml_utils.go * pkg/auth/manager_helpers_test.go </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (4)</summary> <details> <summary>**/*.go</summary> **📄 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.go` - `internal/exec/terraform.go` - `pkg/auth/config_helpers_test.go` - `pkg/auth/config_helpers.go` - `pkg/auth/manager_helpers.go` </details> <details> <summary>**/!(*_test).go</summary> **📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)** > Document all exported functions, types, and methods with Go doc comments Files: - `errors/errors.go` - `internal/exec/terraform.go` - `pkg/auth/config_helpers.go` - `pkg/auth/manager_helpers.go` </details> <details> <summary>pkg/**/*.go</summary> **📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)** > Place business logic in pkg rather than in cmd Files: - `pkg/auth/config_helpers_test.go` - `pkg/auth/config_helpers.go` - `pkg/auth/manager_helpers.go` </details> <details> <summary>**/*_test.go</summary> **📄 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/config_helpers_test.go` </details> </details><details> <summary>🧠 Learnings (28)</summary> <details> <summary>📚 Learning: 2025-04-03T10:57:04.602Z</summary>Learnt from: haitham911
Repo: cloudposse/atmos PR: 1091
File: pkg/config/load_config_args.go:61-82
Timestamp: 2025-04-03T10:57:04.602Z
Learning: In the Atmos configuration system, errors inmergeDefaultImportsandmergeImportsfunctions are intentionally only logged as debug messages without returning errors, as they are considered non-critical. Only errors in critical functions likemergeConfigFileare returned to halt execution.**Applied to files:** - `errors/errors.go` </details> <details> <summary>📚 Learning: 2025-10-22T14:55:44.014Z</summary>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:** - `errors/errors.go` </details> <details> <summary>📚 Learning: 2024-10-23T21:36:40.262Z</summary>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 likecheckAtmosConfigincmd/cmd_utils.go, avoid suggesting refactoring to return errors instead of callingos.Exitif such changes would significantly increase the scope due to the need to update multiple call sites.**Applied to files:** - `errors/errors.go` </details> <details> <summary>📚 Learning: 2025-06-02T14:12:02.710Z</summary>Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to usePrintErrorMarkdownfollowed by returning specific error variables (likeErrWorkflowNoSteps,ErrInvalidFromStep,ErrInvalidWorkflowStepType,ErrWorkflowStepFailed) instead ofPrintErrorMarkdownAndExit. This pattern allows proper error testing without the function terminating the process withos.Exit, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.**Applied to files:** - `errors/errors.go` </details> <details> <summary>📚 Learning: 2025-01-18T15:15:41.645Z</summary>Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform.go:37-46
Timestamp: 2025-01-18T15:15:41.645Z
Learning: In the atmos CLI, error handling is intentionally structured to use LogErrorAndExit for consistent error display, avoiding Cobra's default error handling to prevent duplicate error messages.**Applied to files:** - `errors/errors.go` </details> <details> <summary>📚 Learning: 2025-10-03T18:02:08.535Z</summary>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.go` - `docs/terraform-state-yaml-func-authentication-flow.md` - `docs/terraform-output-yaml-func-authentication-flow.md` </details> <details> <summary>📚 Learning: 2025-09-29T02:20:11.636Z</summary>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 fromanytomap[string]anywithout adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.**Applied to files:** - `internal/exec/terraform.go` </details> <details> <summary>📚 Learning: 2025-06-23T02:14:30.937Z</summary>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 bothatmos describe affectedandatmos terraform apply --affected.**Applied to files:** - `internal/exec/terraform.go` </details> <details> <summary>📚 Learning: 2025-09-25T01:02:48.697Z</summary>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` - `pkg/auth/config_helpers.go` - `pkg/auth/manager_helpers.go` </details> <details> <summary>📚 Learning: 2025-10-10T23:51:36.597Z</summary>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-fileforinit, do not gate them based on command name (e.g., checking ifinfo.Command == "tofu"orinfo.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` </details> <details> <summary>📚 Learning: 2024-12-07T16:19:01.683Z</summary>Learnt from: aknysh
Repo: cloudposse/atmos PR: 825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: Ininternal/exec/terraform.go, skipping stack validation when help flags are present is not necessary.**Applied to files:** - `internal/exec/terraform.go` </details> <details> <summary>📚 Learning: 2024-12-07T16:16:13.038Z</summary>Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: Ininternal/exec/helmfile_generate_varfile.go, the--helpcommand (./atmos helmfile generate varfile --help) works correctly without requiring stack configurations, and the only change needed was to makeProcessCommandLineArgsexportable by capitalizing its name.**Applied to files:** - `internal/exec/terraform.go` </details> <details> <summary>📚 Learning: 2025-11-08T19:56:18.660Z</summary>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.AtmosConfigurationparameter, it should read configuration values fromatmosConfig.Settingsfields rather than using directos.Getenv()orviper.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.AtmosGithubTokeninstead ofos.Getenv("ATMOS_GITHUB_TOKEN")in functions likegetGHCRAuthin internal/exec/oci_utils.go.**Applied to files:** - `internal/exec/terraform.go` - `pkg/auth/config_helpers_test.go` - `pkg/auth/config_helpers.go` </details> <details> <summary>📚 Learning: 2024-10-31T19:23:45.538Z</summary>Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:65-66
Timestamp: 2024-10-31T19:23:45.538Z
Learning: The variableshouldCheckStackinExecuteTerraformcontrols whether validation is performed.**Applied to files:** - `internal/exec/terraform.go` </details> <details> <summary>📚 Learning: 2024-11-13T21:37:07.852Z</summary>Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In theinternal/exec/describe_stacks.gofile of theatmosproject written in Go, avoid extracting the stack name handling logic into a helper function within theExecuteDescribeStacksmethod, even if the logic appears duplicated.**Applied to files:** - `internal/exec/terraform.go` </details> <details> <summary>📚 Learning: 2025-09-23T02:30:42.362Z</summary>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/config_helpers_test.go` </details> <details> <summary>📚 Learning: 2025-09-23T02:30:42.362Z</summary>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/config_helpers_test.go` </details> <details> <summary>📚 Learning: 2025-09-23T02:30:42.362Z</summary>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/config_helpers_test.go` </details> <details> <summary>📚 Learning: 2024-12-03T04:01:16.446Z</summary>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 theterraform.output.mdxdocumentation file (website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx), the cache invalidation and cache scope behavior for the!terraform.outputfunction are already described.**Applied to files:** - `docs/terraform-state-yaml-func-authentication-flow.md` - `docs/terraform-output-yaml-func-authentication-flow.md` </details> <details> <summary>📚 Learning: 2024-12-01T00:33:20.298Z</summary>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: Inexamples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml,!exec atmos terraform outputis used in examples to demonstrate its usage, even though!terraform.outputis the recommended approach according to the documentation.**Applied to files:** - `docs/terraform-state-yaml-func-authentication-flow.md` - `docs/terraform-output-yaml-func-authentication-flow.md` </details> <details> <summary>📚 Learning: 2025-04-25T20:54:19.701Z</summary>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 forread_role_arnandwrite_role_arnoptions is essential to enable cross-account access, allowing users to run operations liketerraform planin multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.**Applied to files:** - `docs/prd/terraform-command-yaml-function-authentication.md` - `docs/terraform-output-yaml-func-authentication-flow.md` </details> <details> <summary>📚 Learning: 2025-09-07T18:07:00.549Z</summary>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:** - `docs/prd/terraform-command-yaml-function-authentication.md` - `pkg/auth/manager_helpers.go` </details> <details> <summary>📚 Learning: 2025-03-12T21:38:42.699Z</summary>Learnt from: osterman
Repo: cloudposse/atmos PR: 1139
File: pkg/config/go-homedir/homedir.go:183-196
Timestamp: 2025-03-12T21:38:42.699Z
Learning: The code in pkg/config/go-homedir is a direct fork of the mitchellh/go-homedir package and was intentionally imported as-is without modifications to maintain consistency with the original source. Security concerns or other improvements may be addressed in future PRs.**Applied to files:** - `pkg/auth/config_helpers.go` </details> <details> <summary>📚 Learning: 2024-12-02T21:26:32.337Z</summary>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 likepkg/config/config.go, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.**Applied to files:** - `pkg/auth/config_helpers.go` </details> <details> <summary>📚 Learning: 2024-12-03T05:29:07.718Z</summary>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 theexecTerraformOutputfunction is acceptable for retrievingterraform outputfor a component in a stack.**Applied to files:** - `docs/terraform-output-yaml-func-authentication-flow.md` </details> <details> <summary>📚 Learning: 2024-12-03T05:18:49.169Z</summary>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 likeexecTerraformOutputto 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.md` </details> <details> <summary>📚 Learning: 2024-12-03T03:52:02.524Z</summary>Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:144-145
Timestamp: 2024-12-03T03:52:02.524Z
Learning: Avoid adding context timeouts to Terraform commands inexecTerraformOutputbecause their execution time can vary from seconds to hours, and making it configurable would require redesigning the command interface.**Applied to files:** - `docs/terraform-output-yaml-func-authentication-flow.md` </details> <details> <summary>📚 Learning: 2025-10-11T19:11:58.965Z</summary>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 checkingos.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.**Applied to files:** - `docs/terraform-output-yaml-func-authentication-flow.md` </details> </details><details> <summary>🧬 Code graph analysis (4)</summary> <details> <summary>internal/exec/terraform.go (4)</summary><blockquote> <details> <summary>pkg/auth/config_helpers.go (2)</summary> * `CopyGlobalAuthConfig` (15-47) * `MergeComponentAuthFromConfig` (113-135) </details> <details> <summary>internal/exec/describe_component.go (2)</summary> * `ExecuteDescribeComponent` (212-228) * `ExecuteDescribeComponentParams` (202-209) </details> <details> <summary>pkg/auth/manager_helpers.go (1)</summary> * `CreateAndAuthenticateManager` (174-216) </details> <details> <summary>internal/exec/utils.go (1)</summary> * `ProcessStacks` (308-742) </details> </blockquote></details> <details> <summary>pkg/auth/config_helpers_test.go (4)</summary><blockquote> <details> <summary>pkg/schema/schema_auth.go (2)</summary> * `AuthConfig` (4-12) * `KeyringConfig` (15-18) </details> <details> <summary>pkg/schema/schema.go (3)</summary> * `Logs` (422-425) * `AtmosConfiguration` (27-65) * `AtmosSettings` (268-289) </details> <details> <summary>pkg/auth/config_helpers.go (3)</summary> * `CopyGlobalAuthConfig` (15-47) * `MergeComponentAuthFromConfig` (113-135) * `AuthConfigToMap` (51-62) </details> <details> <summary>pkg/config/const.go (1)</summary> * `AuthSectionName` (84-84) </details> </blockquote></details> <details> <summary>pkg/auth/config_helpers.go (4)</summary><blockquote> <details> <summary>pkg/schema/schema_auth.go (1)</summary> * `AuthConfig` (4-12) </details> <details> <summary>pkg/schema/schema.go (2)</summary> * `Logs` (422-425) * `AtmosConfiguration` (27-65) </details> <details> <summary>errors/errors.go (3)</summary> * `ErrEncode` (176-176) * `ErrMerge` (175-175) * `ErrDecode` (177-177) </details> <details> <summary>pkg/merge/merge.go (1)</summary> * `Merge` (389-427) </details> </blockquote></details> <details> <summary>pkg/auth/manager_helpers.go (8)</summary><blockquote> <details> <summary>pkg/schema/schema_auth.go (1)</summary> * `AuthConfig` (4-12) </details> <details> <summary>pkg/schema/schema.go (2)</summary> * `ConfigAndStacksInfo` (545-628) * `AuthContext` (516-524) </details> <details> <summary>pkg/auth/credentials/store.go (1)</summary> * `NewCredentialStore` (42-46) </details> <details> <summary>pkg/auth/validation/validator.go (1)</summary> * `NewValidator` (19-21) </details> <details> <summary>pkg/auth/manager.go (1)</summary> * `NewAuthManager` (98-141) </details> <details> <summary>errors/errors.go (2)</summary> * `ErrFailedToInitializeAuthManager` (415-415) * `ErrAuthNotConfigured` (397-397) </details> <details> <summary>pkg/config/const.go (1)</summary> * `IdentityFlagDisabledValue` (137-137) </details> <details> <summary>pkg/logger/log.go (1)</summary> * `Debug` (24-26) </details> </blockquote></details> </details><details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/terraform-state-yaml-func-authentication-flow.md</summary> [typographical] ~34-~34: To join two clauses or introduce examples, consider using an em dash. Context: ....output`: 1. **Identity specification** - Uses `--identity` flag if provided 2. **... (DASH_RULE) --- [typographical] ~35-~35: To join two clauses or introduce examples, consider using an em dash. Context: ...` flag if provided 2. **Auto-detection** - Finds default identity from configuratio... (DASH_RULE) --- [typographical] ~36-~36: To join two clauses or introduce examples, consider using an em dash. Context: ...nfiguration 3. **Interactive selection** - Prompts user if no defaults (TTY mode on... (DASH_RULE) --- [typographical] ~37-~37: To join two clauses or introduce examples, consider using an em dash. Context: ...mode only) 4. **External auth fallback** - Uses environment variables, Leapp, or IM... (DASH_RULE) --- [typographical] ~38-~38: To join two clauses or introduce examples, consider using an em dash. Context: ..., Leapp, or IMDS 5. **Explicit disable** - Respects `--identity=off` to skip Atmos ... (DASH_RULE) --- [style] ~105-~105: Since ownership is already implied, this phrasing may be redundant. Context: ...erraform.output`. Components can define their own auth configuration that overrides globa... (PRP_OWN) --- [typographical] ~201-~201: To join two clauses or introduce examples, consider using an em dash. Context: ...errors: 1. **No credentials available** - Occurs when AuthContext is not provided ... (DASH_RULE) --- [typographical] ~206-~206: To join two clauses or introduce examples, consider using an em dash. Context: ...lable ``` 2. **Invalid credentials** - AWS SDK cannot authenticate ``` op... (DASH_RULE) --- [typographical] ~211-~211: To join two clauses or introduce examples, consider using an em dash. Context: ...Id ``` 3. **Missing S3 permissions** - IAM role lacks required permissions `... (DASH_RULE) --- [typographical] ~216-~216: To join two clauses or introduce examples, consider using an em dash. Context: ...nied ``` 4. **State file not found** - State doesn't exist at expected location... (DASH_RULE) --- [typographical] ~221-~221: To join two clauses or introduce examples, consider using an em dash. Context: ...chKey ``` 5. **Attribute not found** - Requested attribute doesn't exist in sta... (DASH_RULE) --- [style] ~324-~324: Using many exclamation marks might seem excessive (in this case: 15 exclamation marks for a text that’s 5416 characters long) Context: ...pc_id ``` ### Authentication Works for !terraform.output but Not !terraform.state Both functions use the ... (EN_EXCESSIVE_EXCLAMATION) --- [typographical] ~328-~328: To join two clauses or introduce examples, consider using an em dash. Context: ...other doesn't: 1. Check IAM permissions - might have different requirements 2. Ver... (DASH_RULE) </details> <details> <summary>docs/prd/terraform-command-yaml-function-authentication.md</summary> [typographical] ~53-~53: To join two clauses or introduce examples, consider using an em dash. Context: ...hContext ### Impact **Severity:** High - Blocks production use of `!terraform.sta... (DASH_RULE) --- [uncategorized] ~77-~77: The preposition ‘to’ seems more likely in this position. Context: ... Return authenticated AuthManager ready for use **FR-003: Backward Compatibility**... (AI_HYDRA_LEO_REPLACE_FOR_TO) --- [uncategorized] ~102-~102: The preposition ‘to’ seems more likely in this position. Context: ... code - Centralize authentication logic in `pkg/auth/` - Follow existing authentic... (AI_HYDRA_LEO_REPLACE_IN_TO) --- [uncategorized] ~118-~118: Possible missing comma found. Context: ...Manager through the terraform execution pipeline following the pattern established by PR... (AI_HYDRA_LEO_MISSING_COMMA) --- [uncategorized] ~133-~133: The preposition ‘to’ seems more likely in this position. Context: ...t `auth.CreateAndAuthenticateManager()` in `pkg/auth/manager_helpers.go` - Central... (AI_HYDRA_LEO_REPLACE_IN_TO) --- [typographical] ~170-~170: To join two clauses or introduce examples, consider using an em dash. Context: ...es) - `pkg/auth/manager_helpers_test.go` - Unit tests **Core Implementation:** - `... (DASH_RULE) --- [typographical] ~174-~174: To join two clauses or introduce examples, consider using an em dash. Context: ...d AuthManager - `internal/exec/utils.go` - Add AuthManager parameter to ProcessStac... (DASH_RULE) --- [typographical] ~175-~175: To join two clauses or introduce examples, consider using an em dash. Context: ...ComponentConfig - `cmd/identity_flag.go` - Refactor to use shared helper **Process... (DASH_RULE) --- [typographical] ~260-~260: Consider using a typographic opening quote here. Context: ...nsive audit found no other instances of "create but not authenticate" bug - ✅ Arc... (EN_QUOTES) --- [typographical] ~260-~260: Consider using a typographic close quote here. Context: ...nstances of "create but not authenticate" bug - ✅ Architectural improvements (no ... (EN_QUOTES) --- [typographical] ~266-~266: To join two clauses or introduce examples, consider using an em dash. Context: ...://github.com/cloudposse/atmos/pull/1742 - Fixed describe commands - **Authenticati... (DASH_RULE) --- [typographical] ~271-~271: To join two clauses or introduce examples, consider using an em dash. Context: ... **Phase 1 (Investigation):** ✅ Complete - Issue identified and PRD written - **Pha... (DASH_RULE) --- [typographical] ~272-~272: To join two clauses or introduce examples, consider using an em dash. Context: ...**Phase 2 (Implementation):** ✅ Complete - Function signatures updated, AuthManager... (DASH_RULE) --- [typographical] ~273-~273: To join two clauses or introduce examples, consider using an em dash. Context: ... **Phase 3 (Bug Discovery):** ✅ Complete - Found and fixed missing authentication c... (DASH_RULE) --- [typographical] ~274-~274: To join two clauses or introduce examples, consider using an em dash. Context: ... - **Phase 4 (Refactoring):** ✅ Complete - Consolidated duplicate helper functions ... (DASH_RULE) --- [typographical] ~275-~275: To join two clauses or introduce examples, consider using an em dash. Context: ...ctions - **Phase 5 (Audit):** ✅ Complete - Verified no other instances of the bug -... (DASH_RULE) --- [typographical] ~276-~276: To join two clauses or introduce examples, consider using an em dash. Context: ... bug - **Phase 6 (Testing):** ✅ Complete - Manual testing in infra-live confirmed f... (DASH_RULE) --- [typographical] ~277-~277: To join two clauses or introduce examples, consider using an em dash. Context: ...- **Phase 7 (Verification):** ✅ Complete - Build, tests, and code quality checks pa... (DASH_RULE) --- [typographical] ~304-~304: To join two clauses or introduce examples, consider using an em dash. Context: ...es, or IMDS **Benefits:** - Improved UX - No `--identity` flag needed when default... (DASH_RULE) --- [typographical] ~305-~305: To join two clauses or introduce examples, consider using an em dash. Context: ...ured - Single authentication per command - No repeated prompts - CI/CD friendly - A... (DASH_RULE) --- [typographical] ~306-~306: To join two clauses or introduce examples, consider using an em dash. Context: ...d - No repeated prompts - CI/CD friendly - Auto-detects non-interactive environment... (DASH_RULE) --- [typographical] ~307-~307: To join two clauses or introduce examples, consider using an em dash. Context: ... non-interactive environments - Flexible - Works with both Atmos Auth and external ... (DASH_RULE) </details> <details> <summary>docs/terraform-output-yaml-func-authentication-flow.md</summary> [typographical] ~32-~32: To join two clauses or introduce examples, consider using an em dash. Context: ...is order: 1. **Identity specification** - Uses `--identity` flag if provided 2. **... (DASH_RULE) --- [typographical] ~33-~33: To join two clauses or introduce examples, consider using an em dash. Context: ...` flag if provided 2. **Auto-detection** - Finds default identity from configuratio... (DASH_RULE) --- [typographical] ~34-~34: To join two clauses or introduce examples, consider using an em dash. Context: ...nfiguration 3. **Interactive selection** - Prompts user if no defaults (TTY mode on... (DASH_RULE) --- [typographical] ~35-~35: To join two clauses or introduce examples, consider using an em dash. Context: ... (TTY mode only) 5. **Explicit disable** - Respects `--identity=off` to skip Atmos ... (DASH_RULE) --- [typographical] ~105-~105: To join two clauses or introduce examples, consider using an em dash. Context: ... ``` **Behavior:** - **Single default** - Uses it automatically - **Multiple defau... (DASH_RULE) --- [typographical] ~106-~106: To join two clauses or introduce examples, consider using an em dash. Context: ...it automatically - **Multiple defaults** - Prompts user in TTY mode, fails in CI - ... (DASH_RULE) --- [typographical] ~107-~107: To join two clauses or introduce examples, consider using an em dash. Context: ... TTY mode, fails in CI - **No defaults** - Prompts user in TTY mode, uses external ... (DASH_RULE) --- [grammar] ~135-~135: 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) --- [typographical] ~141-~141: To join two clauses or introduce examples, consider using an em dash. Context: ...levels: 1. **Global** (in `atmos.yaml`) - Applies to all components 2. **Stack-lev... (DASH_RULE) --- [typographical] ~142-~142: To join two clauses or introduce examples, consider using an em dash. Context: ...nents 2. **Stack-level** (in stack YAML) - Applies to stack 3. **Component-level** ... (DASH_RULE) --- [typographical] ~143-~143: To join two clauses or introduce examples, consider using an em dash. Context: ...Component-level** (in component section) - Highest precedence ### Example: Compone... (DASH_RULE) --- [typographical] ~272-~272: To join two clauses or introduce examples, consider using an em dash. Context: ...l lookup - `AWS_SHARED_CREDENTIALS_FILE` - Path to Atmos-managed credentials - `AWS... (DASH_RULE) --- [typographical] ~273-~273: To join two clauses or introduce examples, consider using an em dash. Context: ...-managed credentials - `AWS_CONFIG_FILE` - Path to Atmos-managed config - `AWS_REGI... (DASH_RULE) --- [typographical] ~274-~274: To join two clauses or introduce examples, consider using an em dash. Context: ...h to Atmos-managed config - `AWS_REGION` - AWS region for API calls - `AWS_EC2_META... (DASH_RULE) --- [typographical] ~275-~275: To join two clauses or introduce examples, consider using an em dash. Context: ...calls - `AWS_EC2_METADATA_DISABLED=true` - Disables IMDS fallback ### 4. Terraform... (DASH_RULE) --- [style] ~301-~301: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 6306 characters long) Context: ...ences from !terraform.state | Aspect | !terraform.state | !terraform.output | |--------|-----------... (EN_EXCESSIVE_EXCLAMATION) --- [typographical] ~315-~315: To join two clauses or introduce examples, consider using an em dash. Context: ...errors: 1. **No credentials available** - Occurs when AuthContext is not provided ... (DASH_RULE) --- [typographical] ~320-~320: To join two clauses or introduce examples, consider using an em dash. Context: ...lable ``` 2. **Invalid credentials** - Terraform cannot authenticate to S3 back... (DASH_RULE) --- [typographical] ~325-~325: To join two clauses or introduce examples, consider using an em dash. Context: ...Id ``` 3. **Missing S3 permissions** - IAM role lacks required permissions `... (DASH_RULE) --- [typographical] ~330-~330: To join two clauses or introduce examples, consider using an em dash. Context: ...``` 4. **Backend initialization fails** - Cannot connect to S3 backend ``` E... (DASH_RULE) --- [typographical] ~335-~335: To join two clauses or introduce examples, consider using an em dash. Context: ...required ``` 5. **Output not found** - Requested output doesn't exist in state ... (DASH_RULE) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>docs/terraform-state-yaml-func-authentication-flow.md</summary> 242-242: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 243-243: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 244-244: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 245-245: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 260-260: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 261-261: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 262-262: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 263-263: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 264-264: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 265-265: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 266-266: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 316-316: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 321-321: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 326-326: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 331-331: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 336-336: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>docs/prd/terraform-command-yaml-function-authentication.md</summary> 242-242: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 243-243: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 244-244: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 245-245: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 260-260: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 261-261: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 262-262: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 263-263: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 264-264: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 265-265: Hard tabs Column: 1 (MD010, no-hard-tabs) --- 266-266: Hard tabs Column: 1 (MD010, no-hard-tabs) </details> </details> </details> <details> <summary>⏰ 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)</summary> * GitHub Check: Build (linux) * GitHub Check: Build (windows) * GitHub Check: website-deploy-preview * GitHub Check: Review Dependency Licenses * GitHub Check: Lint (golangci) * GitHub Check: Analyze (go) * GitHub Check: autofix * GitHub Check: Summary </details> <details> <summary>🔇 Additional comments (6)</summary><blockquote> <details> <summary>errors/errors.go (1)</summary><blockquote> `176-177`: **LGTM!** The new error sentinels are correctly defined and follow the existing pattern in the file. They'll be wrapped with context when used, per Go error handling conventions. </blockquote></details> <details> <summary>internal/exec/terraform.go (4)</summary><blockquote> `70-73`: **Clean initialization of merged auth config.** Starting with a copy of global auth before merging component-specific config is the right approach. --- `97-103`: **Solid authentication setup.** Delegating to the centralized helper and using the merged auth config ensures both global and component-specific identities are available. --- `105-116`: **Nice hook integration.** Storing the auto-detected identity back into `info.Identity` prevents redundant prompts in hooks while preserving explicitly specified identities. --- `119-119`: **Core fix in place.** Threading `authManager` through to `ProcessStacks` enables YAML functions like `!terraform.state` to access authenticated AWS credentials, resolving the IMDS timeout issue. </blockquote></details> <details> <summary>docs/terraform-state-yaml-func-authentication-flow.md (1)</summary><blockquote> `1-337`: **Comprehensive authentication flow documentation.** The documentation clearly explains the authentication flow, credential propagation, and troubleshooting steps. The call flow diagram aligns with the code changes in terraform.go, and cross-references to related docs are appropriate. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
@coderabbitai full review please, and check if your recent comments were correctly addressed |
|
@aknysh: I'll perform a comprehensive review of all changes in this PR and verify that previous comments have been correctly addressed. The review will cover:
I understand—I will not approve until the review is complete and only if all previous issues are addressed with no new concerns found. 🧠 Learnings used✅ Actions performedFull review triggered. |
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
📜 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)
pkg/auth/config_helpers.go(1 hunks)pkg/auth/config_helpers_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/auth/config_helpers_test.go
🧰 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/config_helpers.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/config_helpers.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/auth/config_helpers.go
🧠 Learnings (7)
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/auth/config_helpers.go
📚 Learning: 2025-03-12T21:38:42.699Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1139
File: pkg/config/go-homedir/homedir.go:183-196
Timestamp: 2025-03-12T21:38:42.699Z
Learning: The code in pkg/config/go-homedir is a direct fork of the mitchellh/go-homedir package and was intentionally imported as-is without modifications to maintain consistency with the original source. Security concerns or other improvements may be addressed in future PRs.
Applied to files:
pkg/auth/config_helpers.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
pkg/auth/config_helpers.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
pkg/auth/config_helpers.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/config_helpers.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:
pkg/auth/config_helpers.go
📚 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:
pkg/auth/config_helpers.go
🧬 Code graph analysis (1)
pkg/auth/config_helpers.go (4)
pkg/schema/schema_auth.go (1)
AuthConfig(4-12)pkg/schema/schema.go (2)
Logs(422-425)AtmosConfiguration(27-65)errors/errors.go (3)
ErrEncode(176-176)ErrMerge(175-175)ErrDecode(177-177)pkg/merge/merge.go (1)
Merge(389-427)
⏰ 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: website-deploy-preview
- GitHub Check: Review Dependency Licenses
- GitHub Check: autofix
- 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/utils/yaml_utils.go (1)
432-451: Function doesn't fulfill its contract.The function name and doc comment say it "prints the provided value as YAML document to a file descriptor," but the implementation only logs via
log.Debugand returnsnil. It doesn't accept a file descriptor parameter, doesn't write to stdout/stderr, and doesn't callPrintMessagelike the otherPrint*functions in this file.Consider one of these fixes:
- If logging is the intended behavior, rename the function and update the doc:
-// PrintAsYAMLToFileDescriptor prints the provided value as YAML document to a file descriptor. -func PrintAsYAMLToFileDescriptor(atmosConfig *schema.AtmosConfiguration, data any) error { +// LogYAMLData logs the provided value as YAML at debug level. +func LogYAMLData(atmosConfig *schema.AtmosConfiguration, data any) error {
- If actual output is intended, add the missing
PrintMessagecall:// Log the YAML data. // Note: This logs multiline YAML which will be formatted by the logger. // For large data structures, this may produce verbose output. log.Debug("PrintAsYAMLToFileDescriptor", "data", y) + PrintMessage(y) return nil
- If writing to a file descriptor is intended, accept a
io.Writerparameter:-func PrintAsYAMLToFileDescriptor(atmosConfig *schema.AtmosConfiguration, data any) error { +func PrintAsYAMLToFileDescriptor(atmosConfig *schema.AtmosConfiguration, data any, w io.Writer) error { // ... conversion code ... log.Debug("PrintAsYAMLToFileDescriptor", "data", y) + _, err := w.Write([]byte(y)) + return err - return nil }
♻️ Duplicate comments (1)
pkg/auth/config_helpers.go (1)
95-107: Guard against nil globalAuthConfig and copy the map.Line 95 accesses
globalAuthConfig.IdentityCaseMapwithout checking ifglobalAuthConfigis nil, which will panic. Additionally, line 96 directly assigns the map without copying, causing aliasing issues where mutations tofinalAuthConfig.IdentityCaseMapaffect the caller's original map.Apply this fix:
- if globalAuthConfig.IdentityCaseMap != nil { - finalAuthConfig.IdentityCaseMap = globalAuthConfig.IdentityCaseMap + if globalAuthConfig != nil && globalAuthConfig.IdentityCaseMap != nil { + finalAuthConfig.IdentityCaseMap = make(map[string]string, len(globalAuthConfig.IdentityCaseMap)) + for k, v := range globalAuthConfig.IdentityCaseMap { + finalAuthConfig.IdentityCaseMap[k] = v + } } if finalAuthConfig.IdentityCaseMap == nil { finalAuthConfig.IdentityCaseMap = make(map[string]string, len(finalAuthConfig.Identities)) } // Add entries for all identities in the merged result. for name := range finalAuthConfig.Identities { lowered := strings.ToLower(name) if _, ok := finalAuthConfig.IdentityCaseMap[lowered]; !ok { finalAuthConfig.IdentityCaseMap[lowered] = name } }
🧹 Nitpick comments (4)
docs/terraform-output-yaml-func-authentication-flow.md (2)
239-268: Replace hard tabs with spaces in code examples.The Go code examples contain hard tabs (lines 242-246, 260-266). Use spaces for consistency with the rest of the documentation.
Apply this pattern to fix the indentation:
- // Populate AuthContext from AuthManager if provided. - if authManager != nil { - managerStackInfo := authManager.GetStackInfo() - if managerStackInfo != nil && managerStackInfo.AuthContext != nil { - configAndStacksInfo.AuthContext = managerStackInfo.AuthContext - } - } + // Populate AuthContext from AuthManager if provided. + if authManager != nil { + managerStackInfo := authManager.GetStackInfo() + if managerStackInfo != nil && managerStackInfo.AuthContext != nil { + configAndStacksInfo.AuthContext = managerStackInfo.AuthContext + } + }
316-338: Add language identifiers to error example code blocks.The fenced code blocks showing error messages (lines 316, 321, 326, 331, 336) should specify a language identifier for proper syntax highlighting.
Apply this pattern:
- ``` - failed to execute terraform: no credentials available - ``` + ```text + failed to execute terraform: no credentials available + ```docs/terraform-state-yaml-func-authentication-flow.md (2)
123-155: Replace hard tabs with spaces in code examples.The Go code blocks contain hard tabs. Replace them with spaces for consistency.
Apply the same fix pattern as in the terraform-output documentation.
202-222: Add language identifiers to error example code blocks.Specify
textas the language for error message code blocks (lines 202, 207, 212, 217, 222) to enable proper rendering.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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
📜 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/prd/terraform-command-yaml-function-authentication.md(1 hunks)pkg/auth/config_helpers.go(1 hunks)pkg/auth/config_helpers_test.go(1 hunks)pkg/flags/global_registry.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pkg/flags/global_registry.go
🧰 Additional context used
📓 Path-based instructions (4)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/auth/config_helpers.gopkg/auth/config_helpers_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/config_helpers.gopkg/auth/config_helpers_test.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/auth/config_helpers.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/config_helpers_test.go
🧠 Learnings (20)
📓 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-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/auth/config_helpers.gopkg/auth/config_helpers_test.go
📚 Learning: 2025-03-12T21:38:42.699Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1139
File: pkg/config/go-homedir/homedir.go:183-196
Timestamp: 2025-03-12T21:38:42.699Z
Learning: The code in pkg/config/go-homedir is a direct fork of the mitchellh/go-homedir package and was intentionally imported as-is without modifications to maintain consistency with the original source. Security concerns or other improvements may be addressed in future PRs.
Applied to files:
pkg/auth/config_helpers.gopkg/auth/config_helpers_test.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
pkg/auth/config_helpers.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/config_helpers.godocs/prd/terraform-command-yaml-function-authentication.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:
pkg/auth/config_helpers.go
📚 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:
pkg/auth/config_helpers.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
pkg/auth/config_helpers.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/config_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 : Test both happy paths and error conditions
Applied to files:
pkg/auth/config_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/config_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 : Document all exported functions, types, and methods with Go doc comments
Applied to files:
pkg/auth/config_helpers_test.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:
docs/prd/terraform-command-yaml-function-authentication.md
📚 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:
docs/prd/terraform-command-yaml-function-authentication.md
📚 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:
docs/prd/terraform-command-yaml-function-authentication.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/prd/terraform-command-yaml-function-authentication.md
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
docs/prd/terraform-command-yaml-function-authentication.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/prd/terraform-command-yaml-function-authentication.md
📚 Learning: 2024-11-22T12:38:33.132Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
Applied to files:
docs/prd/terraform-command-yaml-function-authentication.md
📚 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:
docs/prd/terraform-command-yaml-function-authentication.md
🧬 Code graph analysis (2)
pkg/auth/config_helpers.go (4)
pkg/schema/schema_auth.go (1)
AuthConfig(4-12)pkg/schema/schema.go (2)
Logs(422-425)AtmosConfiguration(27-65)errors/errors.go (3)
ErrEncode(176-176)ErrMerge(175-175)ErrDecode(177-177)pkg/merge/merge.go (1)
Merge(389-427)
pkg/auth/config_helpers_test.go (4)
pkg/schema/schema_auth.go (2)
AuthConfig(4-12)KeyringConfig(15-18)pkg/schema/schema.go (3)
Logs(422-425)AtmosConfiguration(27-65)AtmosSettings(268-289)pkg/auth/config_helpers.go (3)
CopyGlobalAuthConfig(16-48)MergeComponentAuthFromConfig(129-151)AuthConfigToMap(52-63)pkg/config/const.go (1)
AuthSectionName(84-84)
🪛 LanguageTool
docs/prd/terraform-command-yaml-function-authentication.md
[typographical] ~53-~53: To join two clauses or introduce examples, consider using an em dash.
Context: ...hContext ### Impact Severity: High - Blocks production use of `!terraform.sta...
(DASH_RULE)
[uncategorized] ~77-~77: The preposition ‘to’ seems more likely in this position.
Context: ... Return authenticated AuthManager ready for use FR-003: Backward Compatibility...
(AI_HYDRA_LEO_REPLACE_FOR_TO)
[uncategorized] ~102-~102: The preposition ‘to’ seems more likely in this position.
Context: ... code - Centralize authentication logic in pkg/auth/ - Follow existing authentic...
(AI_HYDRA_LEO_REPLACE_IN_TO)
[uncategorized] ~118-~118: Possible missing comma found.
Context: ...Manager through the terraform execution pipeline following the pattern established by PR...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~133-~133: The preposition ‘to’ seems more likely in this position.
Context: ...t auth.CreateAndAuthenticateManager() in pkg/auth/manager_helpers.go - Central...
(AI_HYDRA_LEO_REPLACE_IN_TO)
[typographical] ~170-~170: To join two clauses or introduce examples, consider using an em dash.
Context: ...es) - pkg/auth/manager_helpers_test.go - Unit tests Core Implementation: - `...
(DASH_RULE)
[typographical] ~174-~174: To join two clauses or introduce examples, consider using an em dash.
Context: ...d AuthManager - internal/exec/utils.go - Add AuthManager parameter to ProcessStac...
(DASH_RULE)
[typographical] ~175-~175: To join two clauses or introduce examples, consider using an em dash.
Context: ...ComponentConfig - cmd/identity_flag.go - Refactor to use shared helper **Process...
(DASH_RULE)
[typographical] ~260-~260: Consider using a typographic opening quote here.
Context: ...nsive audit found no other instances of "create but not authenticate" bug - ✅ Arc...
(EN_QUOTES)
[typographical] ~260-~260: Consider using a typographic close quote here.
Context: ...nstances of "create but not authenticate" bug - ✅ Architectural improvements (no ...
(EN_QUOTES)
[typographical] ~266-~266: To join two clauses or introduce examples, consider using an em dash.
Context: ...://github.com//pull/1742 - Fixed describe commands - **Authenticati...
(DASH_RULE)
[typographical] ~271-~271: To join two clauses or introduce examples, consider using an em dash.
Context: ... Phase 1 (Investigation): ✅ Complete - Issue identified and PRD written - **Pha...
(DASH_RULE)
[typographical] ~272-~272: To join two clauses or introduce examples, consider using an em dash.
Context: ...Phase 2 (Implementation): ✅ Complete - Function signatures updated, AuthManager...
(DASH_RULE)
[typographical] ~273-~273: To join two clauses or introduce examples, consider using an em dash.
Context: ... Phase 3 (Bug Discovery): ✅ Complete - Found and fixed missing authentication c...
(DASH_RULE)
[typographical] ~274-~274: To join two clauses or introduce examples, consider using an em dash.
Context: ... - Phase 4 (Refactoring): ✅ Complete - Consolidated duplicate helper functions ...
(DASH_RULE)
[typographical] ~275-~275: To join two clauses or introduce examples, consider using an em dash.
Context: ...ctions - Phase 5 (Audit): ✅ Complete - Verified no other instances of the bug -...
(DASH_RULE)
[typographical] ~276-~276: To join two clauses or introduce examples, consider using an em dash.
Context: ... bug - Phase 6 (Testing): ✅ Complete - Manual testing in infra-live confirmed f...
(DASH_RULE)
[typographical] ~277-~277: To join two clauses or introduce examples, consider using an em dash.
Context: ...- Phase 7 (Verification): ✅ Complete - Build, tests, and code quality checks pa...
(DASH_RULE)
[typographical] ~304-~304: To join two clauses or introduce examples, consider using an em dash.
Context: ...es, or IMDS Benefits: - Improved UX - No --identity flag needed when default...
(DASH_RULE)
[typographical] ~305-~305: To join two clauses or introduce examples, consider using an em dash.
Context: ...ured - Single authentication per command - No repeated prompts - CI/CD friendly - A...
(DASH_RULE)
[typographical] ~306-~306: To join two clauses or introduce examples, consider using an em dash.
Context: ...d - No repeated prompts - CI/CD friendly - Auto-detects non-interactive environment...
(DASH_RULE)
[typographical] ~307-~307: To join two clauses or introduce examples, consider using an em dash.
Context: ... non-interactive environments - Flexible - Works with both Atmos Auth and external ...
(DASH_RULE)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/terraform-command-yaml-function-authentication.md
35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
68-68: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
74-74: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
79-79: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
100-100: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
105-105: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
110-110: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
120-120: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
128-128: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
132-132: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
137-137: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
142-142: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
149-149: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
156-156: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
193-193: Hard tabs
Column: 1
(MD010, no-hard-tabs)
194-194: Hard tabs
Column: 1
(MD010, no-hard-tabs)
195-195: Hard tabs
Column: 1
(MD010, no-hard-tabs)
197-197: Hard tabs
Column: 1
(MD010, no-hard-tabs)
198-198: Hard tabs
Column: 1
(MD010, no-hard-tabs)
199-199: Hard tabs
Column: 1
(MD010, no-hard-tabs)
200-200: Hard tabs
Column: 1
(MD010, no-hard-tabs)
202-202: Hard tabs
Column: 1
(MD010, no-hard-tabs)
203-203: Hard tabs
Column: 1
(MD010, no-hard-tabs)
204-204: Hard tabs
Column: 1
(MD010, no-hard-tabs)
205-205: Hard tabs
Column: 1
(MD010, no-hard-tabs)
206-206: Hard tabs
Column: 1
(MD010, no-hard-tabs)
207-207: Hard tabs
Column: 1
(MD010, no-hard-tabs)
208-208: Hard tabs
Column: 1
(MD010, no-hard-tabs)
209-209: Hard tabs
Column: 1
(MD010, no-hard-tabs)
210-210: Hard tabs
Column: 1
(MD010, no-hard-tabs)
211-211: Hard tabs
Column: 1
(MD010, no-hard-tabs)
213-213: Hard tabs
Column: 1
(MD010, no-hard-tabs)
214-214: Hard tabs
Column: 1
(MD010, no-hard-tabs)
215-215: Hard tabs
Column: 1
(MD010, no-hard-tabs)
216-216: Hard tabs
Column: 1
(MD010, no-hard-tabs)
217-217: Hard tabs
Column: 1
(MD010, no-hard-tabs)
219-219: Hard tabs
Column: 1
(MD010, no-hard-tabs)
220-220: Hard tabs
Column: 1
(MD010, no-hard-tabs)
221-221: Hard tabs
Column: 1
(MD010, no-hard-tabs)
222-222: Hard tabs
Column: 1
(MD010, no-hard-tabs)
223-223: Hard tabs
Column: 1
(MD010, no-hard-tabs)
225-225: Hard tabs
Column: 1
(MD010, no-hard-tabs)
265-265: Bare URL used
(MD034, no-bare-urls)
266-266: Bare URL used
(MD034, no-bare-urls)
283-283: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
287-287: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
291-291: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
295-295: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
299-299: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: autofix
- GitHub Check: website-deploy-preview
- GitHub Check: Review Dependency Licenses
- GitHub Check: Summary
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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 (2)
pkg/flags/global_registry.go (1)
221-229: Past review comment still applies.The Description on line 227 should document the interactive selection feature enabled by NoOptDefVal. This was already flagged in previous reviews.
pkg/auth/config_helpers.go (1)
21-24: Keyring.Spec still aliases the original map.Line 23 shallow-copies the Keyring struct, so
Keyring.Spec(amap[string]any) points to the same underlying map. Any mutation of the returned config's Keyring.Spec will affect the global config. Deep-copy the Spec map before assigning.Apply this diff:
config := &schema.AuthConfig{ Logs: globalAuth.Logs, - Keyring: globalAuth.Keyring, + Keyring: schema.KeyringConfig{ + Type: globalAuth.Keyring.Type, + }, } + + if globalAuth.Keyring.Spec != nil { + config.Keyring.Spec = make(map[string]any, len(globalAuth.Keyring.Spec)) + for k, v := range globalAuth.Keyring.Spec { + config.Keyring.Spec[k] = v + } + }
🧹 Nitpick comments (3)
docs/terraform-output-yaml-func-authentication-flow.md (3)
239-247: Replace hard tabs with spaces in code snippet.The Go code example uses hard tabs. Use spaces for consistency with the rest of the documentation.
Apply this diff:
```go // Populate AuthContext from AuthManager if provided. if authManager != nil { - managerStackInfo := authManager.GetStackInfo() - if managerStackInfo != nil && managerStackInfo.AuthContext != nil { - configAndStacksInfo.AuthContext = managerStackInfo.AuthContext - } + managerStackInfo := authManager.GetStackInfo() + if managerStackInfo != nil && managerStackInfo.AuthContext != nil { + configAndStacksInfo.AuthContext = managerStackInfo.AuthContext + } }--- `257-268`: **Replace hard tabs with spaces in code snippet.** The Go code example uses hard tabs. Use spaces for consistency. Apply this diff: ```diff ```go // Add auth-based environment variables if authContext is provided. if authContext != nil && authContext.AWS != nil { - environMap = awsCloud.PrepareEnvironment( - environMap, - authContext.AWS.Profile, - authContext.AWS.CredentialsFile, - authContext.AWS.ConfigFile, - authContext.AWS.Region, - ) + environMap = awsCloud.PrepareEnvironment( + environMap, + authContext.AWS.Profile, + authContext.AWS.CredentialsFile, + authContext.AWS.ConfigFile, + authContext.AWS.Region, + ) }--- `316-338`: **Add language specifiers to error code blocks.** The error message code blocks should specify a language (e.g., `text`, `console`, or `bash`) for better syntax highlighting and clarity. For example, change: ````markdownfailed to execute terraform: no credentials available
To:
```text failed to execute terraform: no credentials available ```Apply this pattern to the error blocks at lines 316, 321, 326, 331, 336, and 391.
|
These changes were released in v1.198.0-test.12. |
what
!terraform.state,!terraform.output) fail to access AWS credentials when using--identityflag with terraform commands--identity value(space-separated) fails with "Unknown command" error in describe commandsauth.CreateAndAuthenticateManager()helper to eliminate code duplication--logs-levelflagwhy
Critical Bug: Terraform commands didn't create or authenticate AuthManager from
--identityflag, causing YAML functions to fail with AWS IMDS timeout errors when attempting to access S3 state.Root Causes:
ExecuteTerraform()didn't create AuthManager frominfo.IdentityProcessStacks()andProcessComponentConfig()didn't accept AuthManager parameterAuthenticate(), leaving AuthContext emptyIdentity Flag Parsing Bug:
4. NoOptDefVal parsing issue - Cobra's NoOptDefVal feature caused space-separated flag values (
--identity value) to be misinterpreted as subcommands when used with describe commands that have positional argumentspkg/flags/global_registry.goandcmd/describe.go--identity=valueand--identity valueformats now work correctlySecondary Issues:
5. Logger timing - Auth pre-hooks executed before global logger configured with
--logs-levelflag6. Log misalignment - Multiline YAML/JSON values and concurrent spinner writes caused misaligned debug messages (200+ character indents)
Solution:
pkg/auth/manager_helpers.goto eliminate duplicate code betweencmd/identity_flag.goandinternal/exec/terraform.goprocessStackConfigs()so auth hooks see correct log levelImpact:
--identityflag--identity=valueand--identity valueformats work correctly in all commands--logs-levelflagreferences
docs/prd/terraform-command-yaml-function-authentication.md- Comprehensive documentation of implementation, bug discovery, and verificationSummary by CodeRabbit
New Features
Documentation
Improvements
Tests
Bug Fixes & Other