feat: Add native Azure authentication support #1768
Conversation
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned Files
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughAdds comprehensive Azure authentication: new Azure providers (CLI, device-code), subscription identity, Azure credential types, file/MSAL cache and env/setup utilities, console URL generation, threads AuthContext through Terraform backend/provider generation, many tests, dependency updates, and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as Atmos CLI
participant Provider as Azure Provider
participant Cache as DeviceCodeCache/MSAL
participant Files as Azure FileManager
participant Azure as Entra ID
User->>CLI: atmos auth login (azure/device-code | azure/cli)
CLI->>Provider: Authenticate(ctx)
Provider->>Cache: loadCachedToken()
alt cached & valid
Cache-->>Provider: cached token
else
Provider->>Azure: device-code / MSAL flow → request tokens
Azure-->>Provider: access tokens (management/graph/keyvault)
Provider->>Cache: saveCachedToken()
end
Provider->>Files: WriteCredentials()
Files-->>Provider: credentials path
Provider-->>CLI: return AzureCredentials
CLI-->>User: login complete
sequenceDiagram
participant Component
participant Generator as Terraform Generator
participant AuthCtx as AuthContext
participant Backend as Backend/Provider Config
Component->>Generator: Generate backends/providers (includes AuthContext)
Generator->>AuthCtx: read Azure fields (CredentialsFile, TenantID, SubscriptionID, Location)
AuthCtx-->>Generator: auth data
Generator->>Backend: inject auth into backend/provider config
Backend-->>Generator: generated config
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (19)📚 Learning: 2025-11-10T20:03:56.875ZApplied to files:
📚 Learning: 2024-12-13T16:51:37.868ZApplied to files:
📚 Learning: 2025-10-10T23:51:36.597ZApplied to files:
📚 Learning: 2024-11-10T18:37:10.032ZApplied to files:
📚 Learning: 2025-10-11T19:11:58.965ZApplied to files:
📚 Learning: 2025-06-07T19:27:40.807ZApplied to files:
📚 Learning: 2024-10-31T19:25:41.298ZApplied to files:
📚 Learning: 2025-09-25T01:02:48.697ZApplied to files:
📚 Learning: 2025-09-09T02:14:36.708ZApplied to files:
📚 Learning: 2024-10-28T01:51:30.811ZApplied to files:
📚 Learning: 2024-12-02T21:26:32.337ZApplied to files:
📚 Learning: 2024-12-13T16:48:00.294ZApplied to files:
📚 Learning: 2024-12-13T15:28:13.630ZApplied to files:
📚 Learning: 2025-09-08T01:25:44.958ZApplied to files:
📚 Learning: 2025-11-08T19:56:18.660ZApplied to files:
📚 Learning: 2025-09-08T01:25:44.958ZApplied to files:
📚 Learning: 2025-09-08T01:25:44.958ZApplied to files:
📚 Learning: 2025-09-08T01:25:44.958ZApplied to files:
📚 Learning: 2024-12-25T20:28:47.526ZApplied to files:
🧬 Code graph analysis (5)pkg/auth/providers/azure/device_code.go (8)
pkg/auth/cloud/azure/setup_test.go (6)
pkg/auth/cloud/azure/setup.go (9)
pkg/auth/cloud/azure/files_test.go (4)
pkg/auth/cloud/azure/files.go (5)
⏰ 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). (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/exec/terraform_output_utils.go (1)
313-330: Add Azure environment preparation to match AWS implementation.The
execTerraformOutputfunction receivesauthContextwith both AWS and Azure contexts available, but only callsawsCloud.PrepareEnvironment()whenauthContext.AWSis present. Since AzurePrepareEnvironmentfunction exists with similar functionality, adding a parallel check forauthContext.Azureis necessary for complete Azure support:if authContext != nil && authContext.Azure != nil { log.Debug("Adding auth-based environment variables", "profile", authContext.Azure.Profile, "subscription_id", authContext.Azure.SubscriptionID, "tenant_id", authContext.Azure.TenantID, ) // Use shared Azure environment preparation helper. environMap = azureCloud.PrepareEnvironment( environMap, authContext.Azure.SubscriptionID, authContext.Azure.TenantID, authContext.Azure.Location, authContext.Azure.CredentialsFile, "", // accessToken ) }Also verify the import is available for the Azure cloud package (similar to how
awsCloudis imported).
🧹 Nitpick comments (1)
internal/exec/terraform_output_utils.go (1)
68-200: Consider returning errors instead of panicking.The authContextWrapper satisfies the AuthManager interface by panicking in unused methods. While this works for the current narrow use case, it creates a runtime hazard if future code accidentally calls these methods. Consider returning clear errors instead:
func (a *authContextWrapper) GetCachedCredentials(ctx context.Context, identityName string) (*auth_types.WhoamiInfo, error) { return nil, fmt.Errorf("authContextWrapper does not support GetCachedCredentials - only for propagating existing auth context") }This provides the same "don't call me" semantics but fails gracefully at the call site rather than crashing the process.
📜 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/auth_console.go(2 hunks)go.mod(1 hunks)internal/exec/terraform_generate_backend.go(1 hunks)internal/exec/terraform_generate_backends.go(1 hunks)internal/exec/terraform_generate_backends_test.go(6 hunks)internal/exec/terraform_output_utils.go(2 hunks)internal/exec/terraform_utils.go(2 hunks)internal/exec/utils.go(2 hunks)internal/exec/utils_test.go(1 hunks)pkg/auth/cloud/azure/console.go(1 hunks)pkg/auth/cloud/azure/env.go(1 hunks)pkg/auth/cloud/azure/files.go(1 hunks)pkg/auth/cloud/azure/setup.go(1 hunks)pkg/auth/factory/factory.go(3 hunks)pkg/auth/identities/azure/subscription.go(1 hunks)pkg/auth/providers/azure/cli.go(1 hunks)pkg/auth/providers/azure/device_code.go(1 hunks)pkg/auth/providers/azure/device_code_cache.go(1 hunks)pkg/auth/types/azure_credentials.go(1 hunks)pkg/auth/types/azure_credentials_test.go(1 hunks)pkg/auth/types/constants.go(1 hunks)pkg/schema/schema.go(2 hunks)website/blog/2025-11-07-azure-authentication-support.mdx(1 hunks)website/blog/authors.yml(1 hunks)website/docs/cli/commands/auth/auth-login.mdx(1 hunks)website/docs/cli/commands/auth/tutorials/azure-authentication.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use cases
Files:
website/docs/cli/commands/auth/auth-login.mdxwebsite/docs/cli/commands/auth/tutorials/azure-authentication.mdxwebsite/blog/2025-11-07-azure-authentication-support.mdxwebsite/blog/authors.yml
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/auth/factory/factory.gopkg/auth/types/constants.gopkg/schema/schema.gopkg/auth/cloud/azure/env.gopkg/auth/types/azure_credentials.gopkg/auth/types/azure_credentials_test.gopkg/auth/providers/azure/cli.gopkg/auth/cloud/azure/setup.gopkg/auth/providers/azure/device_code.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/cloud/azure/console.gopkg/auth/identities/azure/subscription.gopkg/auth/cloud/azure/files.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/factory/factory.gointernal/exec/terraform_output_utils.gopkg/auth/types/constants.gopkg/schema/schema.gointernal/exec/terraform_utils.gointernal/exec/terraform_generate_backends_test.gopkg/auth/cloud/azure/env.gointernal/exec/utils.gopkg/auth/types/azure_credentials.gocmd/auth_console.gopkg/auth/types/azure_credentials_test.gointernal/exec/terraform_generate_backends.gopkg/auth/providers/azure/cli.gopkg/auth/cloud/azure/setup.gopkg/auth/providers/azure/device_code.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/cloud/azure/console.gointernal/exec/utils_test.gopkg/auth/identities/azure/subscription.gopkg/auth/cloud/azure/files.gointernal/exec/terraform_generate_backend.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/factory/factory.gointernal/exec/terraform_output_utils.gopkg/auth/types/constants.gopkg/schema/schema.gointernal/exec/terraform_utils.gopkg/auth/cloud/azure/env.gointernal/exec/utils.gopkg/auth/types/azure_credentials.gocmd/auth_console.gointernal/exec/terraform_generate_backends.gopkg/auth/providers/azure/cli.gopkg/auth/cloud/azure/setup.gopkg/auth/providers/azure/device_code.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/cloud/azure/console.gopkg/auth/identities/azure/subscription.gopkg/auth/cloud/azure/files.gointernal/exec/terraform_generate_backend.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
Files:
internal/exec/terraform_generate_backends_test.gopkg/auth/types/azure_credentials_test.gointernal/exec/utils_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands
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/auth_console.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 (36)
📓 Common learnings
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
📚 Learning: 2025-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:
website/docs/cli/commands/auth/auth-login.mdxwebsite/blog/2025-11-07-azure-authentication-support.mdx
📚 Learning: 2025-09-10T17:34:52.568Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/providers/github/oidc.go:96-100
Timestamp: 2025-09-10T17:34:52.568Z
Learning: The ATMOS_ environment variable binding guideline applies to Atmos configuration variables, not external service-required environment variables like GitHub Actions OIDC variables (GITHUB_ACTIONS, ACTIONS_ID_TOKEN_*) which must use their standard names.
Applied to files:
website/docs/cli/commands/auth/auth-login.mdx
📚 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/factory/factory.gopkg/schema/schema.gocmd/auth_console.gopkg/auth/providers/azure/cli.gopkg/auth/cloud/azure/files.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/factory/factory.gopkg/auth/cloud/azure/env.gocmd/auth_console.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:
pkg/auth/factory/factory.gocmd/auth_console.go
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
internal/exec/terraform_output_utils.gointernal/exec/terraform_utils.gointernal/exec/terraform_generate_backends_test.gointernal/exec/utils.gointernal/exec/terraform_generate_backends.gointernal/exec/utils_test.gointernal/exec/terraform_generate_backend.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_output_utils.gointernal/exec/terraform_utils.gointernal/exec/terraform_generate_backends_test.gointernal/exec/terraform_generate_backends.gointernal/exec/utils_test.gointernal/exec/terraform_generate_backend.go
📚 Learning: 2024-12-03T03:52:02.524Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:144-145
Timestamp: 2024-12-03T03:52:02.524Z
Learning: Avoid adding context timeouts to Terraform commands in `execTerraformOutput` because their execution time can vary from seconds to hours, and making it configurable would require redesigning the command interface.
Applied to files:
internal/exec/terraform_output_utils.gointernal/exec/terraform_utils.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
internal/exec/terraform_output_utils.gointernal/exec/terraform_utils.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.gointernal/exec/terraform_utils.go
📚 Learning: 2025-10-27T01:54:35.665Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1714
File: website/blog/2025-10-26-zero-config-terminal-output.md:6-6
Timestamp: 2025-10-27T01:54:35.665Z
Learning: In the Atmos blog (website/blog/), blog post authors in frontmatter should always be set to the actual committer or PR opener (e.g., "osterman"), not generic organization names like "cloudposse" or "atmos". Authors must exist in website/blog/authors.yml.
Applied to files:
website/blog/authors.yml
📚 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_utils.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.
Applied to files:
pkg/auth/cloud/azure/env.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain XDG compliance implementation is complete with GetXDGCacheDir() and GetXDGTempCacheDir() functions in toolchain/xdg_cache.go, updated installer.go and toolchain_clean.go to use these helpers, and changed cache paths from ~/.cache/tools-cache to ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain when XDG_CACHE_HOME is not set).
Applied to files:
pkg/auth/cloud/azure/env.gopkg/auth/providers/azure/device_code_cache.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
internal/exec/utils.gointernal/exec/terraform_generate_backends.gointernal/exec/utils_test.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/types/azure_credentials.gopkg/auth/types/azure_credentials_test.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
cmd/auth_console.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
cmd/auth_console.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/auth_console.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: ErrWrappingFormat is correctly defined as "%w: %w" in the errors package and is used throughout the codebase to wrap two error types together. The usage fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) is the correct pattern when both arguments are error types.
Applied to files:
cmd/auth_console.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/auth_console.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: The user confirmed that the errors package has an error string wrapping format, contradicting the previous learning about ErrWrappingFormat being invalid. The current usage of fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) appears to be the correct pattern.
Applied to files:
cmd/auth_console.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/types/azure_credentials_test.gointernal/exec/utils_test.go
📚 Learning: 2025-01-09T22:37:01.004Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.
Applied to files:
internal/exec/terraform_generate_backends.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_backends.go
📚 Learning: 2024-11-30T22:07:08.610Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/yaml_func_terraform_output.go:35-40
Timestamp: 2024-11-30T22:07:08.610Z
Learning: In the Go function `processTagTerraformOutput` in `internal/exec/yaml_func_terraform_output.go`, parameters cannot contain spaces. The code splits the input by spaces, and if the parameters contain spaces, `len(parts) != 3` will fail and show an error to the user.
Applied to files:
internal/exec/terraform_generate_backends.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_backends.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_backends.gointernal/exec/terraform_generate_backend.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: XDG Base Directory Specification compliance implementation for atmos toolchain is complete: created toolchain/xdg_cache.go with GetXDGCacheDir() and GetXDGTempCacheDir() functions, updated toolchain/installer.go and cmd/toolchain_clean.go to use these XDG helpers, and changed all cache paths from hardcoded ~/.cache/tools-cache to XDG-compliant ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback).
Applied to files:
pkg/auth/providers/azure/device_code_cache.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: Final XDG Base Directory Specification implementation for atmos toolchain is complete and verified: toolchain/xdg_cache.go provides GetXDGCacheDir() and GetXDGTempCacheDir() functions, all hardcoded ~/.cache/tools-cache paths have been replaced with XDG-compliant paths using ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback), and tests have been updated to expect the new path structure.
Applied to files:
pkg/auth/providers/azure/device_code_cache.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
pkg/auth/providers/azure/device_code_cache.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:
internal/exec/utils_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:
internal/exec/utils_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 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
🧬 Code graph analysis (15)
pkg/auth/factory/factory.go (3)
pkg/auth/providers/azure/cli.go (1)
NewCLIProvider(39-76)pkg/auth/providers/azure/device_code.go (1)
NewDeviceCodeProvider(54-97)pkg/auth/identities/azure/subscription.go (1)
NewSubscriptionIdentity(27-64)
internal/exec/terraform_utils.go (1)
pkg/schema/schema.go (1)
AuthContext(515-525)
pkg/auth/cloud/azure/env.go (2)
pkg/perf/perf.go (1)
Track(121-138)pkg/logger/log.go (1)
Debug(24-26)
internal/exec/utils.go (1)
pkg/schema/schema.go (1)
AuthContext(515-525)
pkg/auth/types/azure_credentials.go (3)
errors/errors.go (2)
ErrInvalidAuthConfig(383-383)ErrAuthenticationFailed(388-388)pkg/auth/types/whoami.go (1)
WhoamiInfo(6-22)pkg/auth/types/interfaces.go (1)
ValidationInfo(268-283)
cmd/auth_console.go (3)
pkg/auth/types/constants.go (3)
ProviderKindAzureOIDC(13-13)ProviderKindAzureCLI(14-14)ProviderKindAzureDeviceCode(15-15)pkg/auth/cloud/azure/console.go (1)
NewConsoleURLGenerator(27-30)pkg/auth/cloud/aws/console.go (1)
NewConsoleURLGenerator(46-57)
pkg/auth/types/azure_credentials_test.go (3)
pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)errors/errors.go (2)
ErrInvalidAuthConfig(383-383)ErrAuthenticationFailed(388-388)pkg/auth/types/whoami.go (1)
WhoamiInfo(6-22)
pkg/auth/providers/azure/cli.go (4)
errors/errors.go (4)
ErrInvalidProviderConfig(387-387)ErrInvalidProviderKind(386-386)ErrAuthenticationFailed(388-388)ErrInvalidAuthConfig(383-383)pkg/auth/types/interfaces.go (1)
ICredentials(254-265)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/cloud/azure/env.go (1)
PrepareEnvironment(81-147)
pkg/auth/cloud/azure/setup.go (7)
pkg/auth/types/interfaces.go (1)
ICredentials(254-265)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/cloud/azure/files.go (1)
NewAzureFileManager(82-99)errors/errors.go (2)
ErrAuthenticationFailed(388-388)ErrInvalidAuthConfig(383-383)pkg/schema/schema.go (3)
AuthContext(515-525)ConfigAndStacksInfo(567-650)AzureAuthContext(548-565)pkg/config/homedir/homedir.go (1)
Dir(36-63)pkg/auth/cloud/azure/env.go (1)
PrepareEnvironment(81-147)
pkg/auth/providers/azure/device_code.go (8)
internal/tui/templates/term/term_writer.go (1)
IsTTYSupportForStderr(127-129)pkg/auth/providers/azure/device_code_cache.go (1)
CacheStorage(37-48)errors/errors.go (3)
ErrInvalidProviderConfig(387-387)ErrInvalidProviderKind(386-386)ErrAuthenticationFailed(388-388)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/telemetry/ci.go (1)
IsCI(81-86)pkg/utils/url_utils.go (1)
OpenUrl(13-39)pkg/ui/theme/colors.go (4)
ColorCyan(12-12)ColorGray(10-10)ColorGreen(11-11)ColorBlue(14-14)pkg/auth/cloud/azure/env.go (1)
PrepareEnvironment(81-147)
pkg/auth/providers/azure/device_code_cache.go (2)
pkg/xdg/xdg.go (1)
GetXDGCacheDir(32-34)pkg/logger/log.go (1)
Debug(24-26)
pkg/auth/cloud/azure/console.go (4)
pkg/perf/perf.go (1)
Track(121-138)pkg/auth/types/interfaces.go (1)
ConsoleURLOptions(297-314)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)errors/errors.go (1)
ErrInvalidAuthConfig(383-383)
pkg/auth/identities/azure/subscription.go (6)
errors/errors.go (3)
ErrInvalidIdentityConfig(385-385)ErrInvalidIdentityKind(384-384)ErrAuthenticationFailed(388-388)pkg/auth/types/interfaces.go (2)
ICredentials(254-265)PostAuthenticateParams(61-68)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/cloud/azure/env.go (1)
PrepareEnvironment(81-147)pkg/auth/cloud/azure/setup.go (4)
SetupFiles(22-40)UpdateAzureCLIFiles(216-263)SetAuthContextParams(43-50)SetAuthContext(54-106)pkg/auth/cloud/azure/files.go (1)
NewAzureFileManager(82-99)
pkg/auth/cloud/azure/files.go (5)
pkg/logger/log.go (2)
Debug(24-26)Warn(44-46)pkg/perf/perf.go (1)
Track(121-138)pkg/config/homedir/homedir.go (1)
Dir(36-63)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)errors/errors.go (1)
ErrAuthenticationFailed(388-388)
internal/exec/terraform_generate_backend.go (1)
pkg/schema/schema.go (1)
AuthContext(515-525)
🪛 LanguageTool
website/docs/cli/commands/auth/tutorials/azure-authentication.mdx
[typographical] ~86-~86: Consider using a typographic opening quote here.
Context: ... 1. Atmos displays a device code (e.g., "WDDD-HRQV") 2. Browser opens to Microsof...
(EN_QUOTES)
[typographical] ~86-~86: Consider using a typographic close quote here.
Context: ...displays a device code (e.g., "WDDD-HRQV") 2. Browser opens to Microsoft's device...
(EN_QUOTES)
[grammar] ~295-~295: Ensure spelling is correct
Context: ... scope } ``` ### azuread Provider The azuread provider automatically uses Graph API t...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~471-~471: You have used the passive voice repeatedly in nearby sentences. To make your writing clearer and easier to read, consider using active voice.
Context: ... provider compatibility - Both storages are secured by your operating system ### Least Pri...
(REP_PASSIVE_VOICE)
[typographical] ~489-~489: Consider using typographic quotation marks here.
Context: ...→ Monitoring → Sign-in logs - Filter by "Application" to see Atmos authentications ## Migrat...
(EN_QUOTES)
website/blog/2025-11-07-azure-authentication-support.mdx
[grammar] ~374-~374: Please add a punctuation mark at the end of paragraph.
Context: ...b.com/cloudposse/atmos/issues) - Slack Community Special...
(PUNCTUATION_PARAGRAPH_END)
⏰ 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). (10)
- GitHub Check: Build (windows)
- GitHub Check: Build (macos)
- GitHub Check: Build (linux)
- GitHub Check: autofix
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Review Dependency Licenses
- GitHub Check: Run pre-commit hooks
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (27)
go.mod (1)
12-12: Dependency is current and secure—no changes needed.The v1.3.0 version you've added is the latest stable version, and no known security advisories exist for it. The dependency meets the guideline to keep dependencies up to date.
pkg/auth/types/constants.go (1)
13-15: LGTM!The new Azure provider kind constants follow the established naming pattern and are consistent with the existing AWS provider kinds.
website/docs/cli/commands/auth/auth-login.mdx (2)
96-97: LGTM!Clear explanation of Azure device code flow with helpful context about Terraform provider compatibility.
98-115: Tutorial links verified. Both referenced files exist in the repository:
website/docs/cli/commands/auth/tutorials/migrating-from-leapp.mdx✓website/docs/cli/commands/auth/tutorials/azure-authentication.mdx✓The documentation links are valid.
internal/exec/utils_test.go (1)
263-263: LGTM!Test correctly updated to accommodate the new authContext parameter in generateComponentBackendConfig signature.
cmd/auth_console.go (2)
17-17: LGTM!Import follows the established aliasing pattern used for AWS cloud auth.
214-217: LGTM!Azure console support properly integrated with all three Azure provider kinds. The implementation follows the AWS pattern and is consistent with the generator constructors shown in the relevant code snippets.
pkg/auth/factory/factory.go (3)
8-10: LGTM!Azure package imports follow the established aliasing pattern used for AWS.
30-33: LGTM!Azure provider registration properly integrated. The factory pattern is consistent with existing AWS providers, and the constructors match the signatures shown in the relevant code snippets.
58-59: LGTM!Azure subscription identity registration follows the established factory pattern. The constructor signature matches the snippet from pkg/auth/identities/azure/subscription.go.
internal/exec/terraform_output_utils.go (2)
273-273: LGTM!AuthContext properly propagated to backend config generation, consistent with signature updates across the codebase.
294-294: LGTM!AuthContext properly propagated to provider overrides generation.
internal/exec/terraform_generate_backends.go (1)
353-353: The authContext parameter is unused—propagating it here wouldn't help.The
generateComponentBackendConfigfunction acceptsauthContextbut never uses it in its implementation. It only processesbackendType,backendConfig, andterraformWorkspace. Additionally, thegenerate-backendscommand is purely code generation (writing.tfand.tf.jsonfiles), so authentication is not required for this workflow. Thenilvalue is intentional and correct. Unliketerraform generate backend(singular), which callsProcessCommandLineArgsto populate the full info context, thegenerate-backendscommand iterates through all components without needing auth credentials.Likely an incorrect or invalid review comment.
internal/exec/terraform_generate_backends_test.go (1)
489-489: LGTM! Test signature updates are correct.All call sites properly updated to pass
nilfor the newauthContextparameter. This is appropriate for unit tests that focus on backend generation logic rather than auth integration.Also applies to: 673-673, 699-699, 722-722, 751-751, 771-771
internal/exec/terraform_generate_backend.go (1)
77-77: LGTM! Auth context properly propagated.The call correctly passes
info.AuthContextthrough to backend config generation, enabling Azure (and future cloud provider) authentication integration.pkg/schema/schema.go (2)
546-565: LGTM! Schema design is clean and consistent.The
AzureAuthContextstruct follows the same pattern asAWSAuthContext, with clear field documentation. Field selection aligns well with Azure's authentication model (tenant, subscription, location).
519-520: Good integration with AuthContext.The Azure field properly extends
AuthContextfor multi-cloud support, maintaining consistency with the existing AWS field pattern.Based on learnings: The schema supports subscription-level authentication. Future enhancement for cross-tenant/subscription authentication (similar to AWS cross-account access with role ARNs) may require additional fields like
ReadSubscriptionIDorAssumeRoleConfig.website/docs/cli/commands/auth/tutorials/azure-authentication.mdx (1)
1-516: Excellent documentation! Comprehensive and well-structured.This tutorial provides everything users need to understand and implement Azure authentication:
- Clear explanations of all three auth methods
- Practical code examples for each scenario
- Multi-subscription workflow guidance
- Terraform provider compatibility details
- Security considerations and troubleshooting
The migration guidance from
az loginis particularly helpful for existing Azure users.website/blog/2025-11-07-azure-authentication-support.mdx (1)
1-376: Great blog post! Effectively communicates the new feature.The post strikes a good balance between technical detail and accessibility. The Quick Start example immediately shows value, and the comprehensive token support explanation addresses a common pain point (KeyVault timeouts) that users will appreciate.
internal/exec/utils.go (2)
732-732: LGTM! Signature extended to support auth context.The
authContextparameter enables future Azure-specific backend configuration (e.g., using managed identity, setting Azure-specific environment variables). The parameter is currently unused in the function body, which is fine for this initial integration phase.
775-775: LGTM! Provider overrides signature updated consistently.Similar to
generateComponentBackendConfig, this signature extension prepares for Azure-specific provider configuration. The parameter will be utilized as Azure provider override logic is implemented.internal/exec/terraform_utils.go (1)
94-94: LGTM! Auth context properly threaded through.Both generator calls correctly pass
info.AuthContext, enabling Azure authentication to influence backend and provider configuration. The auth context propagates naturally from the stack processing flow.Also applies to: 117-117
pkg/auth/types/azure_credentials_test.go (5)
14-34: LGTM! Clean table-driven test for expiration logic.The test covers all key scenarios: empty expiration, invalid format, past expiration, and future expiration. This approach is idiomatic and thorough.
36-59: LGTM! GetExpiration test covers all branches.Tests properly verify nil return for empty expiration, successful parsing with time tolerance, and error handling with proper error wrapping verification using
errors.Is.
61-77: LGTM! BuildWhoamiInfo test validates field mapping.Correctly verifies that subscription ID maps to account, location maps to region, and expiration is parsed and set. Time assertion includes tolerance for test stability.
79-113: Validate tests acknowledge Azure credential limitation.The tests properly document that
Validaterequires real Azure credentials and will fail in the test environment. The tests verify method structure and error behavior.Consider using a mock Azure SDK or marking these as integration tests if you want to expand test coverage beyond structure verification.
115-158: LGTM! Good coverage of token and field scenarios.These tests ensure:
- Multiple token types (Management, Graph API, KeyVault) are properly stored
- Credentials work with only required fields, optional fields can be empty
This validates the flexible design of
AzureCredentials.
jamengual
force-pushed
the
azure-login-support
branch
from
940d003 to
bb29ea8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
website/docs/cli/commands/auth/tutorials/azure-authentication.mdx (1)
86-86: Optional: Consider typographic quotes for consistency.If you want to polish the docs further, using typographic quotes ("" instead of "") would be more consistent with professional documentation styling. This is purely cosmetic though - the content itself is excellent.
website/blog/2025-11-07-azure-authentication-support.mdx (1)
374-374: Optional: Add punctuation at paragraph end.Minor style point: the Feedback section could use a period at the end of the Slack Community line for consistency.
📜 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)
cmd/auth_console_test.go(1 hunks)internal/exec/utils_test.go(2 hunks)website/blog/2025-11-07-azure-authentication-support.mdx(1 hunks)website/docs/cli/commands/auth/tutorials/azure-authentication.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/exec/utils_test.go
🧰 Additional context used
📓 Path-based instructions (4)
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/auth_console_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
Files:
cmd/auth_console_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:
cmd/auth_console_test.go
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use cases
Files:
website/blog/2025-11-07-azure-authentication-support.mdxwebsite/docs/cli/commands/auth/tutorials/azure-authentication.mdx
🧠 Learnings (3)
📓 Common learnings
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.
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/providers/github/oidc.go:96-100
Timestamp: 2025-09-10T17:34:52.568Z
Learning: The ATMOS_ environment variable binding guideline applies to Atmos configuration variables, not external service-required environment variables like GitHub Actions OIDC variables (GITHUB_ACTIONS, ACTIONS_ID_TOKEN_*) which must use their standard names.
📚 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:
cmd/auth_console_test.go
📚 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:
website/blog/2025-11-07-azure-authentication-support.mdx
🧬 Code graph analysis (1)
cmd/auth_console_test.go (1)
pkg/auth/types/constants.go (1)
ProviderKindAzureOIDC(13-13)
🪛 LanguageTool
website/blog/2025-11-07-azure-authentication-support.mdx
[grammar] ~374-~374: Please add a punctuation mark at the end of paragraph.
Context: ...b.com/cloudposse/atmos/issues) - Slack Community Special...
(PUNCTUATION_PARAGRAPH_END)
website/docs/cli/commands/auth/tutorials/azure-authentication.mdx
[typographical] ~86-~86: Consider using a typographic opening quote here.
Context: ... 1. Atmos displays a device code (e.g., "WDDD-HRQV") 2. Browser opens to Microsof...
(EN_QUOTES)
[typographical] ~86-~86: Consider using a typographic close quote here.
Context: ...displays a device code (e.g., "WDDD-HRQV") 2. Browser opens to Microsoft's device...
(EN_QUOTES)
[grammar] ~295-~295: Ensure spelling is correct
Context: ... scope } ``` ### azuread Provider The azuread provider automatically uses Graph API t...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~471-~471: You have used the passive voice repeatedly in nearby sentences. To make your writing clearer and easier to read, consider using active voice.
Context: ... provider compatibility - Both storages are secured by your operating system ### Least Pri...
(REP_PASSIVE_VOICE)
[typographical] ~489-~489: Consider using typographic quotation marks here.
Context: ...→ Monitoring → Sign-in logs - Filter by "Application" to see Atmos authentications ## Migrat...
(EN_QUOTES)
⏰ 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). (10)
- GitHub Check: website-deploy-preview
- GitHub Check: Build (macos)
- GitHub Check: Build (windows)
- GitHub Check: Build (linux)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Review Dependency Licenses
- GitHub Check: autofix
- GitHub Check: Run pre-commit hooks
- GitHub Check: Summary
🔇 Additional comments (3)
cmd/auth_console_test.go (1)
465-468: Nice update - Azure OIDC console access is now live.The test correctly reflects that Azure OIDC now supports console provider generation. This aligns well with the broader Azure authentication support added in this PR.
website/docs/cli/commands/auth/tutorials/azure-authentication.mdx (1)
1-522: Solid documentation - comprehensive and practical.This guide covers all three Azure authentication methods thoroughly with clear examples, troubleshooting steps, and security considerations. The migration guidance from
az loginis particularly helpful for existing users. The structure flows well from Quick Start through advanced patterns.website/blog/2025-11-07-azure-authentication-support.mdx (1)
1-376: Great announcement post - hits all the right notes.This blog post effectively communicates the new Azure authentication support with a good mix of quick examples, technical details, and migration guidance. The emphasis on drop-in
az logincompatibility and full token scope support addresses key user concerns upfront. The implementation details section is particularly valuable for understanding the completeness of the solution.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1768 +/- ##
==========================================
- Coverage 70.93% 70.56% -0.38%
==========================================
Files 440 451 +11
Lines 40789 42338 +1549
==========================================
+ Hits 28933 29874 +941
- Misses 9409 9943 +534
- Partials 2447 2521 +74
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
jamengual
force-pushed
the
azure-login-support
branch
from
9a56372 to
23b4508
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
pkg/auth/identities/azure/subscription.go (1)
154-154: Address overflow risk and refine capacity calculation.The security scanner correctly flagged this line for potential overflow when computing allocation size. While
len(environ)reaching max int is unlikely in practice, it's a theoretical risk. Additionally, the capacity calculation allocates space for 3 additional entries, but the method can add up to 5 (2 for subscription, 2 for location, 1 for resource group).Apply this diff to fix both concerns:
- result := make(map[string]string, len(environ)+3) + // Allocate with a reasonable capacity; map will grow if needed + result := make(map[string]string)Alternatively, if you want to preserve the capacity hint for performance, use a safer calculation:
- result := make(map[string]string, len(environ)+3) + const maxAdditionalEntries = 5 // subscription(2) + location(2) + resourceGroup(1) + capacity := len(environ) + if capacity <= (1<<31 - 1 - maxAdditionalEntries) { + capacity += maxAdditionalEntries + } + result := make(map[string]string, capacity)pkg/auth/providers/azure/cli.go (1)
126-130: Fix CLI token expiration parsing.Azure CLI returns
expiresOnin local time format (e.g.,2025-11-07 14:22:19.123456), not RFC3339. Parsing withtime.RFC3339always fails, causing authentication to bail out even whenaz loginsucceeded.Apply this diff to fix the parsing:
- expiresOn, err := time.Parse(time.RFC3339, tokenResp.ExpiresOn) - if err != nil { - return nil, fmt.Errorf("%w: failed to parse Azure CLI token expiration: %w", errUtils.ErrInvalidAuthConfig, err) - } + expiresOn, err := time.ParseInLocation("2006-01-02 15:04:05.999999", tokenResp.ExpiresOn, time.Local) + if err != nil { + expiresOn, err = time.ParseInLocation("2006-01-02 15:04:05", tokenResp.ExpiresOn, time.Local) + } + if err != nil { + return nil, fmt.Errorf("%w: failed to parse Azure CLI token expiration: %w", errUtils.ErrInvalidAuthConfig, err) + } + expiresOn = expiresOn.UTC()pkg/auth/providers/azure/device_code.go (1)
118-149: Cached path drops KeyVault token.When
loadCachedTokensucceeds, you return immediately without acquiring a KeyVault token, and callupdateAzureCLICachewith emptykeyVaultTokenparameters. This means cached flows lose the KeyVault scope, breaking the KeyVault functionality mentioned in the PR description.Persist the KeyVault token and expiry in the cache, validate it before reuse, and rehydrate it here so the CLI cache remains fully populated.
pkg/auth/cloud/azure/setup.go (1)
265-434: Refresh tokens missing from MSAL cache.
updateMSALCacheonly writesAccessTokenandAccountsections. Azure CLI persists refresh tokens alongside access tokens to silently renew tokens and mint new scopes. Without refresh tokens, you're limited to ~1 hour of usability, breaking the PR goal of being a drop-inaz loginreplacement.Extend
updateMSALCacheto persist refresh token entries and plumb the refresh token value through from the credential source. Without it, long Terraform runs and any provider needing a new scope will fail once the initial access token expires.pkg/auth/providers/azure/device_code_cache.go (1)
23-167: Cache schema missing KeyVault fields.
deviceCodeTokenCache(lines 23-33) carries Graph data but omits KeyVault token and expiration fields. Because of this omission,loadCachedToken(lines 90-132) can never restore a KeyVault token, andsaveCachedToken(lines 134-167) doesn't persist it. This forces the CLI cache to run without KeyVault tokens on cached runs.Add
KeyVaultTokenandKeyVaultExpiresAtfields to the cache struct, persist them insaveCachedToken, validate them inloadCachedToken, and thread them throughupdateAzureCLICacheso the CLI cache remains complete on non-interactive runs.
🧹 Nitpick comments (1)
pkg/auth/cloud/azure/setup.go (1)
265-434: Consider extracting shared MSAL cache logic.
updateMSALCachehere andupdateAzureCLICacheindevice_code_cache.go(lines 186-382) have nearly identical logic for building MSAL cache entries. The JWT parsing utilities (extractOIDFromToken,extractUsernameFromToken,extractJWTClaims) are also duplicated across both files.Extract the common MSAL cache building logic and JWT utilities into a shared helper function or file (e.g.,
pkg/auth/cloud/azure/msal.go) to reduce duplication and improve maintainability.
📜 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 (11)
NOTICE(1 hunks)pkg/auth/cloud/azure/console_test.go(1 hunks)pkg/auth/cloud/azure/env_test.go(1 hunks)pkg/auth/cloud/azure/setup.go(1 hunks)pkg/auth/identities/azure/subscription.go(1 hunks)pkg/auth/providers/azure/cli.go(1 hunks)pkg/auth/providers/azure/cli_test.go(1 hunks)pkg/auth/providers/azure/device_code.go(1 hunks)pkg/auth/providers/azure/device_code_cache.go(1 hunks)pkg/auth/types/azure_credentials.go(1 hunks)pkg/auth/types/azure_credentials_test.go(1 hunks)
🧰 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/types/azure_credentials.gopkg/auth/identities/azure/subscription.gopkg/auth/types/azure_credentials_test.gopkg/auth/providers/azure/cli.gopkg/auth/providers/azure/cli_test.gopkg/auth/cloud/azure/console_test.gopkg/auth/providers/azure/device_code.gopkg/auth/cloud/azure/env_test.gopkg/auth/cloud/azure/setup.gopkg/auth/providers/azure/device_code_cache.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/types/azure_credentials.gopkg/auth/identities/azure/subscription.gopkg/auth/types/azure_credentials_test.gopkg/auth/providers/azure/cli.gopkg/auth/providers/azure/cli_test.gopkg/auth/cloud/azure/console_test.gopkg/auth/providers/azure/device_code.gopkg/auth/cloud/azure/env_test.gopkg/auth/cloud/azure/setup.gopkg/auth/providers/azure/device_code_cache.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/types/azure_credentials.gopkg/auth/identities/azure/subscription.gopkg/auth/providers/azure/cli.gopkg/auth/providers/azure/device_code.gopkg/auth/cloud/azure/setup.gopkg/auth/providers/azure/device_code_cache.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/types/azure_credentials_test.gopkg/auth/providers/azure/cli_test.gopkg/auth/cloud/azure/console_test.gopkg/auth/cloud/azure/env_test.go
🧠 Learnings (7)
📚 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/types/azure_credentials.gopkg/auth/types/azure_credentials_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
pkg/auth/types/azure_credentials_test.gopkg/auth/providers/azure/cli_test.gopkg/auth/cloud/azure/console_test.gopkg/auth/cloud/azure/env_test.go
📚 Learning: 2025-09-25T01:02:48.697Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/manager.go:304-312
Timestamp: 2025-09-25T01:02:48.697Z
Learning: The auth manager in pkg/auth/manager.go should remain cloud-agnostic and not contain AWS-specific logic or references to specific cloud providers. Keep the manager generic and extensible.
Applied to files:
pkg/auth/providers/azure/cli.gopkg/auth/cloud/azure/setup.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/providers/azure/cli_test.gopkg/auth/cloud/azure/env_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.
Applied to files:
pkg/auth/cloud/azure/env_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: XDG Base Directory Specification compliance implementation for atmos toolchain is complete: created toolchain/xdg_cache.go with GetXDGCacheDir() and GetXDGTempCacheDir() functions, updated toolchain/installer.go and cmd/toolchain_clean.go to use these XDG helpers, and changed all cache paths from hardcoded ~/.cache/tools-cache to XDG-compliant ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback).
Applied to files:
pkg/auth/providers/azure/device_code_cache.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain XDG compliance implementation is complete with GetXDGCacheDir() and GetXDGTempCacheDir() functions in toolchain/xdg_cache.go, updated installer.go and toolchain_clean.go to use these helpers, and changed cache paths from ~/.cache/tools-cache to ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain when XDG_CACHE_HOME is not set).
Applied to files:
pkg/auth/providers/azure/device_code_cache.go
🧬 Code graph analysis (9)
pkg/auth/types/azure_credentials.go (3)
errors/errors.go (2)
ErrInvalidAuthConfig(383-383)ErrAuthenticationFailed(388-388)pkg/auth/types/whoami.go (1)
WhoamiInfo(6-22)pkg/auth/types/interfaces.go (1)
ValidationInfo(268-283)
pkg/auth/identities/azure/subscription.go (7)
errors/errors.go (3)
ErrInvalidIdentityConfig(385-385)ErrInvalidIdentityKind(384-384)ErrAuthenticationFailed(388-388)pkg/schema/schema.go (3)
Context(426-441)Validate(188-190)AuthContext(515-525)pkg/auth/types/interfaces.go (2)
ICredentials(254-265)PostAuthenticateParams(61-68)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/cloud/azure/env.go (1)
PrepareEnvironment(81-147)pkg/auth/cloud/azure/setup.go (5)
SetupFiles(22-40)UpdateAzureCLIFiles(216-263)SetAuthContextParams(43-50)SetAuthContext(54-106)SetEnvironmentVariables(142-211)pkg/auth/cloud/azure/files.go (1)
NewAzureFileManager(82-99)
pkg/auth/types/azure_credentials_test.go (3)
pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)errors/errors.go (2)
ErrInvalidAuthConfig(383-383)ErrAuthenticationFailed(388-388)pkg/auth/types/whoami.go (1)
WhoamiInfo(6-22)
pkg/auth/providers/azure/cli.go (5)
errors/errors.go (4)
ErrInvalidProviderConfig(387-387)ErrInvalidProviderKind(386-386)ErrAuthenticationFailed(388-388)ErrInvalidAuthConfig(383-383)pkg/perf/perf.go (1)
Track(121-138)pkg/logger/log.go (1)
Debug(24-26)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/cloud/azure/env.go (1)
PrepareEnvironment(81-147)
pkg/auth/providers/azure/cli_test.go (2)
errors/errors.go (2)
ErrInvalidProviderConfig(387-387)ErrInvalidProviderKind(386-386)pkg/auth/providers/azure/cli.go (1)
NewCLIProvider(39-76)
pkg/auth/cloud/azure/console_test.go (5)
pkg/auth/types/interfaces.go (1)
ConsoleURLOptions(297-314)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)errors/errors.go (1)
ErrInvalidAuthConfig(383-383)pkg/auth/types/aws_credentials.go (1)
AWSCredentials(16-24)pkg/auth/cloud/azure/console.go (1)
AzureDefaultSessionDuration(20-20)
pkg/auth/providers/azure/device_code.go (7)
internal/tui/templates/term/term_writer.go (1)
IsTTYSupportForStderr(127-129)pkg/auth/providers/azure/device_code_cache.go (1)
CacheStorage(37-48)errors/errors.go (3)
ErrInvalidProviderConfig(387-387)ErrInvalidProviderKind(386-386)ErrAuthenticationFailed(388-388)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/telemetry/ci.go (1)
IsCI(81-86)pkg/utils/url_utils.go (1)
OpenUrl(13-39)pkg/auth/cloud/azure/env.go (1)
PrepareEnvironment(81-147)
pkg/auth/cloud/azure/setup.go (8)
pkg/auth/types/interfaces.go (1)
ICredentials(254-265)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/cloud/azure/files.go (1)
NewAzureFileManager(82-99)errors/errors.go (2)
ErrAuthenticationFailed(388-388)ErrInvalidAuthConfig(383-383)pkg/schema/schema.go (3)
AuthContext(515-525)ConfigAndStacksInfo(567-650)AzureAuthContext(548-565)pkg/logger/log.go (1)
Debug(24-26)pkg/config/homedir/homedir.go (1)
Dir(36-63)pkg/auth/cloud/azure/env.go (1)
PrepareEnvironment(81-147)
pkg/auth/providers/azure/device_code_cache.go (2)
pkg/xdg/xdg.go (1)
GetXDGCacheDir(32-34)pkg/logger/log.go (1)
Debug(24-26)
⏰ 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). (10)
- GitHub Check: Build (windows)
- GitHub Check: Build (macos)
- GitHub Check: Build (linux)
- GitHub Check: Review Dependency Licenses
- GitHub Check: website-deploy-preview
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Run pre-commit hooks
- GitHub Check: autofix
- GitHub Check: Summary
🔇 Additional comments (5)
NOTICE (1)
947-950: LGTM!The dependency addition is properly formatted, correctly placed in the MIT license section, and maintains alphabetical ordering with other Azure SDK entries. Aligns well with the Azure authentication support being added in this PR.
pkg/auth/identities/azure/subscription.go (4)
66-132: Core identity methods look solid.Kind, GetProviderName, Authenticate, and Validate implement proper validation, error handling, and credential transformation. The fallback logic for location (lines 111-113) ensures the identity uses the provider's location when not explicitly set.
174-222: Post-authentication orchestration is well-structured.The method properly sequences credential file setup, Azure CLI compatibility updates (non-fatal), auth context population, and environment variable injection. Error handling distinguishes between fatal and non-fatal failures appropriately.
224-229: Logout no-op is appropriate.Since credentials are managed by the keyring and files are cleaned up by the provider, having this identity's Logout as a no-op makes sense. The debug log provides observability.
231-274: Credential loading logic is correct.Both methods properly delegate to the file manager and handle errors. The override logic in LoadCredentials (lines 265-271) correctly applies identity-specific subscription ID and location to the loaded credentials, maintaining consistency with the Authenticate method.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
pkg/auth/cloud/azure/setup_test.go (5)
20-78: Consider adding error path coverage.The test currently covers happy paths well, but per coding guidelines, error conditions should also be tested. Consider adding test cases for:
- Invalid/empty provider or identity names
- Nil credentials
- Permission errors (though this may be challenging to test portably)
Apply this diff to add error coverage:
{ name: "handles non-Azure credentials", providerName: "test-provider", identityName: "test-identity", creds: &types.AWSCredentials{}, basePath: tmpDir, expectError: false, // Non-Azure credentials are ignored. }, + { + name: "handles nil credentials", + providerName: "test-provider", + identityName: "test-identity", + creds: nil, + basePath: tmpDir, + expectError: false, + }, }
391-436: Consider consolidating duplicate test code and adding more edge cases.The comment on line 392 indicates this duplicates tests from
device_code_cache_test.go. If the function being tested is the same, consider:
- Moving shared test utilities to a common test helper file
- Adding more edge cases: empty token, malformed base64, invalid JSON in payload
Add more edge cases:
{ name: "invalid JWT format - missing parts", token: "invalid.token", expectError: true, }, + { + name: "empty token", + token: "", + expectError: true, + }, + { + name: "malformed base64 in payload", + token: "valid_header.invalid_base64!.signature", + expectError: true, + }, }
438-480: Consider adding edge case for invalid token format.The test covers the main scenarios but could benefit from an edge case where the token is completely malformed (not just missing the OID claim), similar to the invalid format test in TestExtractJWTClaims_Setup.
597-656: Add error path testing for MSAL cache updates.The test validates the happy path well but lacks error condition coverage. Per coding guidelines, tests should cover both happy and error paths.
Consider adding test cases for:
- Invalid JWT tokens (malformed, missing OID)
- Invalid expiration timestamps
- File system errors (though may be hard to test portably)
tests := []struct { name string + accessToken string + expiration string expectError bool }{ { name: "successfully updates MSAL cache", + accessToken: createTestJWT("user-oid-123", "user@example.com"), + expiration: now.Add(1 * time.Hour).Format(time.RFC3339), expectError: false, }, + { + name: "handles invalid JWT", + accessToken: "invalid-token", + expiration: now.Add(1 * time.Hour).Format(time.RFC3339), + expectError: true, + }, }
658-779: Good test isolation with separate home directories.The test properly isolates each case with separate directories and covers important scenarios including BOM handling. However, consider adding error path coverage such as:
- Malformed JSON in existing profile
- Permission errors during write
- Missing required fields in subscription data
pkg/auth/providers/azure/device_code_test.go (1)
201-216: Consider testing with a valid context as well.The comment states PreAuthenticate "should be a no-op and always return nil," but the test only verifies this with a
nilcontext. To strengthen this assertion, also test withcontext.Background()to confirm it's truly a no-op regardless of context.func TestDeviceCodeProvider_PreAuthenticate(t *testing.T) { provider := &deviceCodeProvider{ name: "test-device-code", config: &schema.Provider{ Kind: "azure/device-code", Spec: map[string]interface{}{ "tenant_id": "tenant-123", }, }, tenantID: "tenant-123", } - // PreAuthenticate should be a no-op and always return nil. - err := provider.PreAuthenticate(nil) - assert.NoError(t, err) + tests := []struct { + name string + ctx context.Context + }{ + {"nil context", nil}, + {"valid context", context.Background()}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // PreAuthenticate should be a no-op and always return nil. + err := provider.PreAuthenticate(tt.ctx) + assert.NoError(t, err) + }) + } }pkg/auth/cloud/azure/files_test.go (2)
17-61: Add test case for homedir.Dir() error condition.The test only covers happy paths but
NewAzureFileManagercan return an error whenhomedir.Dir()fails. Per the coding guidelines, tests should cover both happy paths and error conditions.Consider adding a test case that simulates the home directory lookup failure. While this may require refactoring the code to make
homedir.Dir()injectable or mockable, it would provide more comprehensive coverage of error scenarios.
93-180: Consider adding error condition tests.While the happy path coverage is solid (directory creation, permissions, JSON marshaling), consider testing error scenarios like write failures or permission issues if they can be simulated without excessive complexity.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
pkg/auth/cloud/azure/files_test.go(1 hunks)pkg/auth/cloud/azure/setup_test.go(1 hunks)pkg/auth/identities/azure/subscription_test.go(1 hunks)pkg/auth/providers/azure/cli_test.go(1 hunks)pkg/auth/providers/azure/device_code_cache_test.go(1 hunks)pkg/auth/providers/azure/device_code_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/auth/cloud/azure/files_test.gopkg/auth/cloud/azure/setup_test.gopkg/auth/providers/azure/cli_test.gopkg/auth/identities/azure/subscription_test.gopkg/auth/providers/azure/device_code_cache_test.gopkg/auth/providers/azure/device_code_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
Files:
pkg/auth/cloud/azure/files_test.gopkg/auth/cloud/azure/setup_test.gopkg/auth/providers/azure/cli_test.gopkg/auth/identities/azure/subscription_test.gopkg/auth/providers/azure/device_code_cache_test.gopkg/auth/providers/azure/device_code_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
Files:
pkg/auth/cloud/azure/files_test.gopkg/auth/cloud/azure/setup_test.gopkg/auth/providers/azure/cli_test.gopkg/auth/identities/azure/subscription_test.gopkg/auth/providers/azure/device_code_cache_test.gopkg/auth/providers/azure/device_code_test.go
🧠 Learnings (7)
📚 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/cloud/azure/files_test.gopkg/auth/cloud/azure/setup_test.gopkg/auth/providers/azure/cli_test.gopkg/auth/identities/azure/subscription_test.gopkg/auth/providers/azure/device_code_cache_test.gopkg/auth/providers/azure/device_code_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/cloud/azure/files_test.gopkg/auth/cloud/azure/setup_test.gopkg/auth/identities/azure/subscription_test.gopkg/auth/providers/azure/device_code_test.go
📚 Learning: 2024-12-25T20:28:47.526Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/stack_processor_utils.go:380-380
Timestamp: 2024-12-25T20:28:47.526Z
Learning: Windows path handling often requires `filepath.Join` to ensure correct separators and comparisons. Insufficient tests can break cross-platform compatibility, so migrating from `path.Join` to `filepath.Join` needs thorough testing on Windows before merging.
Applied to files:
pkg/auth/cloud/azure/files_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/cloud/azure/setup_test.gopkg/auth/providers/azure/cli_test.gopkg/auth/identities/azure/subscription_test.gopkg/auth/providers/azure/device_code_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: Final XDG Base Directory Specification implementation for atmos toolchain is complete and verified: toolchain/xdg_cache.go provides GetXDGCacheDir() and GetXDGTempCacheDir() functions, all hardcoded ~/.cache/tools-cache paths have been replaced with XDG-compliant paths using ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback), and tests have been updated to expect the new path structure.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: XDG Base Directory Specification compliance implementation for atmos toolchain is complete: created toolchain/xdg_cache.go with GetXDGCacheDir() and GetXDGTempCacheDir() functions, updated toolchain/installer.go and cmd/toolchain_clean.go to use these XDG helpers, and changed all cache paths from hardcoded ~/.cache/tools-cache to XDG-compliant ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback).
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain XDG compliance implementation is complete with GetXDGCacheDir() and GetXDGTempCacheDir() functions in toolchain/xdg_cache.go, updated installer.go and toolchain_clean.go to use these helpers, and changed cache paths from ~/.cache/tools-cache to ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain when XDG_CACHE_HOME is not set).
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
🧬 Code graph analysis (6)
pkg/auth/cloud/azure/files_test.go (4)
pkg/auth/cloud/azure/files.go (2)
NewAzureFileManager(82-99)AzureFileManager(75-78)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)errors/errors.go (1)
ErrAuthenticationFailed(388-388)pkg/config/homedir/homedir.go (1)
Dir(36-63)
pkg/auth/cloud/azure/setup_test.go (5)
pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/types/aws_credentials.go (1)
AWSCredentials(16-24)pkg/schema/schema.go (3)
AuthContext(515-525)ConfigAndStacksInfo(567-650)AzureAuthContext(548-565)errors/errors.go (1)
ErrInvalidAuthConfig(383-383)pkg/auth/cloud/azure/files.go (1)
NewAzureFileManager(82-99)
pkg/auth/providers/azure/cli_test.go (2)
errors/errors.go (2)
ErrInvalidProviderConfig(387-387)ErrInvalidProviderKind(386-386)pkg/auth/providers/azure/cli.go (1)
NewCLIProvider(39-76)
pkg/auth/identities/azure/subscription_test.go (6)
pkg/schema/schema_auth.go (1)
IdentityVia(60-63)errors/errors.go (3)
ErrInvalidIdentityConfig(385-385)ErrInvalidIdentityKind(384-384)ErrAuthenticationFailed(388-388)pkg/auth/identities/azure/subscription.go (1)
NewSubscriptionIdentity(27-64)pkg/auth/types/interfaces.go (1)
ICredentials(254-265)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/types/aws_credentials.go (1)
AWSCredentials(16-24)
pkg/auth/providers/azure/device_code_cache_test.go (1)
pkg/xdg/xdg.go (1)
GetXDGCacheDir(32-34)
pkg/auth/providers/azure/device_code_test.go (2)
errors/errors.go (3)
ErrInvalidProviderConfig(387-387)ErrInvalidProviderKind(386-386)ErrAuthenticationFailed(388-388)pkg/auth/providers/azure/device_code.go (1)
NewDeviceCodeProvider(54-97)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Build (linux)
- GitHub Check: Build (windows)
- GitHub Check: autofix
- GitHub Check: Run pre-commit hooks
- GitHub Check: Review Dependency Licenses
- GitHub Check: website-deploy-preview
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (35)
pkg/auth/cloud/azure/setup_test.go (6)
80-196: Well-structured test with comprehensive coverage.This test function demonstrates excellent practices:
- Table-driven design with multiple scenarios
- Tests both happy and error paths
- Uses callback pattern for flexible verification
- Includes nil-safety checks
198-291: Excellent edge case coverage.This test thoroughly covers defensive programming scenarios including nil checks, type assertions, and missing keys. The comprehensive test cases ensure the function gracefully handles malformed configuration data.
293-389: Good coverage of environment variable setup.The test properly validates that all required Azure and ARM environment variables are set, including the critical ARM_USE_CLI flag for Terraform provider compatibility.
482-524: Username extraction test looks good.The test covers the primary Azure username claim (upn) and the absence case. If the implementation supports fallback to other username claims (email, preferred_username), consider adding test cases for those scenarios.
526-554: Fallback test is straightforward and adequate.The test validates both successful extraction and fallback behavior. The fallback value "user@unknown" is a reasonable default.
556-595: Comprehensive BOM handling tests.Excellent coverage of all edge cases for UTF-8 BOM detection and stripping, including boundary conditions and partial matches. This is important for robust Azure profile file parsing.
pkg/auth/providers/azure/device_code_test.go (13)
1-14: LGTM! Clean test setup.The imports are well-organized and appropriate for the test scenarios covered.
16-169: Excellent comprehensive constructor testing.This table-driven test covers all critical paths including valid configurations with various field combinations, missing required fields, nil/empty specs, and invalid provider kinds. The use of a
checkProvidercallback for additional assertions is a clean pattern.
171-174: LGTM.Simple and effective test for the Kind method.
176-199: LGTM.Covers both populated and empty name scenarios appropriately.
218-268: LGTM! Thorough validation testing.Covers the validation logic for all required fields (tenant ID and client ID) with appropriate error type checking.
270-330: LGTM! Comprehensive environment variable testing.Effectively tests all combinations of required and optional fields, ensuring the environment map is built correctly.
332-421: LGTM! Critical security behavior well-tested.This test thoroughly verifies that PrepareEnvironment correctly sets Azure/ARM variables, removes conflicting credential environment variables (including Azure service principal creds, AWS, and GCP), and ensures ARM_USE_CLI is always set. The security aspect of clearing conflicting credentials is particularly important.
423-457: LGTM! Clean logout lifecycle test.Properly tests the token cache lifecycle (save → verify → logout → verify deletion) for both the main token and graph token. The use of mock storage is appropriate for unit testing.
459-485: LGTM.Covers the display path generation including the case with hyphens in the provider name.
487-569: LGTM! Thorough UI component testing.Comprehensively tests all spinner model states and transitions including initialization, authentication completion (success/error), user interruption (Ctrl+C), and view rendering for each state.
571-590: LGTM.Straightforward verification that spec fields are correctly extracted into provider fields.
592-609: LGTM.Verifies that optional fields receive appropriate defaults when omitted from the spec.
611-663: LGTM! Type handling correctly verified.This test validates the type coercion behavior: when spec fields are the wrong type, the type assertion silently fails and the field remains at its default value. For required fields like
tenant_id, this ultimately triggers a validation error when the field is empty. For optional fields likesubscription_id, the wrong type is simply ignored. This behavior is consistent and correctly tested.pkg/auth/identities/azure/subscription_test.go (7)
17-155: Excellent test coverage for the constructor.This table-driven test comprehensively covers all scenarios: valid configurations with various field combinations, error cases for invalid inputs, and field extraction verification. The use of
errors.Isfor error type checking follows Go best practices.
162-223: Solid test coverage for provider name extraction.Tests both success and error paths, including nil via and empty provider scenarios.
225-333: Thorough authentication flow testing.The test correctly verifies that identity-specific fields (subscription ID, location) override provider credentials while preserving authentication tokens. Error handling for type mismatches is also covered.
335-408: Good validation logic coverage.Tests verify that validation correctly enforces required fields (subscription ID, via provider) and handles all error conditions properly.
410-468: Comprehensive environment variable generation testing.Tests correctly verify that the identity generates both AZURE_* and ARM_* variants for all applicable fields, with proper handling of optional fields.
470-544: Excellent environment preparation testing with mutation check.This test properly verifies environment merging, overriding behavior, and importantly includes a check (lines 535-541) to ensure the original environment map is not mutated. This is a good defensive testing practice.
157-160: Good coverage of remaining methods.The type handling test (lines 556-576) is particularly valuable as it verifies defensive behavior when principal fields contain unexpected types, ensuring graceful degradation rather than panics.
Also applies to: 546-576
pkg/auth/providers/azure/cli_test.go (6)
15-145: Comprehensive constructor testing.This table-driven test provides excellent coverage of all valid configurations, error scenarios, and field extraction logic. The test properly validates that tenant_id is required while subscription_id and location are optional.
194-282: Solid field extraction and type handling tests.These tests verify that spec fields are correctly extracted, optional fields default appropriately, and invalid types are handled gracefully. The type handling test ensures that wrong types for required fields cause errors while wrong types for optional fields are ignored.
284-323: Good validation test coverage.Tests verify that validation correctly enforces the required tenant_id field.
325-373: Good environment variable generation testing.Tests verify correct generation of AZURE_* environment variables for various field combinations.
375-451: Excellent environment preparation testing.This test properly verifies that PrepareEnvironment adds both AZURE_* and ARM_* variants, sets ARM_USE_CLI=true for CLI authentication, and importantly clears conflicting service principal variables. The test structure checking both expected and missing variables is well done.
147-192: Good coverage of remaining methods.Tests appropriately verify simple getter methods (Kind, Name) and no-op methods (PreAuthenticate, Logout).
Also applies to: 453-463
pkg/auth/cloud/azure/files_test.go (3)
182-278: Solid error handling coverage.This test covers both happy path and multiple error conditions (missing file, invalid JSON) with proper error type assertions using
errors.Is. Well done.
280-337: Good cleanup test coverage.Tests both successful removal and graceful handling of non-existent directories. Clean implementation.
387-427: Excellent JSON marshaling verification.This dedicated test ensures all credential fields survive the marshal/unmarshal round-trip with realistic test data. Strong validation approach.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pkg/auth/providers/azure/device_code_cache_test.go (1)
709-730: Sandbox HOME before exercising the CLI cacheupdateAzureCLICache still follows os.UserHomeDir, so this test will touch the real
~/.azureon whoever runs the suite. That’s risky and violates the hermetic test rules. Please point HOME/USERPROFILE (and AZURE_CONFIG_DIR) at at.TempDir()before invoking the method.now := time.Now().UTC() accessToken := createTestJWT("user-oid-123", "user@example.com") graphToken := createTestJWT("user-oid-123", "user@example.com") keyVaultToken := createTestJWT("user-oid-123", "user@example.com") + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + t.Setenv("USERPROFILE", tmpHome) + azureDir := filepath.Join(tmpHome, ".azure") + require.NoError(t, os.MkdirAll(azureDir, 0o755)) + t.Setenv("AZURE_CONFIG_DIR", azureDir) + provider := &deviceCodeProvider{ name: "test-provider", tenantID: "tenant-123",
🧹 Nitpick comments (1)
pkg/auth/providers/azure/device_code_test.go (1)
201-216: Use context.Background() instead of nil.While PreAuthenticate may not use the context, passing
context.Background()is better practice than passingnil.Apply this diff:
- err := provider.PreAuthenticate(nil) + err := provider.PreAuthenticate(context.Background())
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
pkg/auth/identities/azure/subscription_test.go(1 hunks)pkg/auth/providers/azure/device_code_cache_test.go(1 hunks)pkg/auth/providers/azure/device_code_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/auth/identities/azure/subscription_test.gopkg/auth/providers/azure/device_code_test.gopkg/auth/providers/azure/device_code_cache_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
Files:
pkg/auth/identities/azure/subscription_test.gopkg/auth/providers/azure/device_code_test.gopkg/auth/providers/azure/device_code_cache_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/identities/azure/subscription_test.gopkg/auth/providers/azure/device_code_test.gopkg/auth/providers/azure/device_code_cache_test.go
🧠 Learnings (9)
📚 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/identities/azure/subscription_test.gopkg/auth/providers/azure/device_code_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests
Applied to files:
pkg/auth/identities/azure/subscription_test.gopkg/auth/providers/azure/device_code_test.gopkg/auth/providers/azure/device_code_cache_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/providers/azure/device_code_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: Final XDG Base Directory Specification implementation for atmos toolchain is complete and verified: toolchain/xdg_cache.go provides GetXDGCacheDir() and GetXDGTempCacheDir() functions, all hardcoded ~/.cache/tools-cache paths have been replaced with XDG-compliant paths using ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback), and tests have been updated to expect the new path structure.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: XDG Base Directory Specification compliance implementation for atmos toolchain is complete: created toolchain/xdg_cache.go with GetXDGCacheDir() and GetXDGTempCacheDir() functions, updated toolchain/installer.go and cmd/toolchain_clean.go to use these XDG helpers, and changed all cache paths from hardcoded ~/.cache/tools-cache to XDG-compliant ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback).
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain XDG compliance implementation is complete with GetXDGCacheDir() and GetXDGTempCacheDir() functions in toolchain/xdg_cache.go, updated installer.go and toolchain_clean.go to use these helpers, and changed cache paths from ~/.cache/tools-cache to ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain when XDG_CACHE_HOME is not set).
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2024-12-13T15:33:34.159Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:17-31
Timestamp: 2024-12-13T15:33:34.159Z
Learning: In `pkg/config/cache.go`, when `XDG_CACHE_HOME` is not set, falling back to `.` (current directory) is acceptable and aligns with the requirement to primarily use `XDG_CACHE_HOME` for the cache directory.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
pkg/auth/providers/azure/device_code_cache_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/providers/azure/device_code_cache_test.go
🧬 Code graph analysis (3)
pkg/auth/identities/azure/subscription_test.go (5)
pkg/schema/schema_auth.go (1)
IdentityVia(60-63)errors/errors.go (3)
ErrInvalidIdentityConfig(385-385)ErrInvalidIdentityKind(384-384)ErrAuthenticationFailed(388-388)pkg/auth/identities/azure/subscription.go (1)
NewSubscriptionIdentity(27-64)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/types/aws_credentials.go (1)
AWSCredentials(16-24)
pkg/auth/providers/azure/device_code_test.go (2)
errors/errors.go (3)
ErrInvalidProviderConfig(387-387)ErrInvalidProviderKind(386-386)ErrAuthenticationFailed(388-388)pkg/auth/providers/azure/device_code.go (1)
NewDeviceCodeProvider(54-97)
pkg/auth/providers/azure/device_code_cache_test.go (1)
pkg/xdg/xdg.go (1)
GetXDGCacheDir(32-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build (linux)
- GitHub Check: Build (windows)
- GitHub Check: autofix
- GitHub Check: website-deploy-preview
- GitHub Check: Review Dependency Licenses
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (13)
pkg/auth/identities/azure/subscription_test.go (9)
1-15: LGTM!Clean package declaration and imports. All necessary dependencies are present and properly organized.
17-155: Excellent test coverage for the constructor.Comprehensive table-driven test covering all success and failure scenarios. Proper error type checking with
errors.Isand thorough field verification.
157-160: LGTM!Simple and effective test for the Kind method.
162-223: LGTM!Well-structured table-driven test with appropriate error handling validation.
225-333: Excellent authentication test coverage.Thoroughly tests the override semantics for subscription ID and location, plus token preservation. Good coverage of both success and failure paths.
335-408: LGTM!Comprehensive validation test covering all required fields and error cases.
410-468: LGTM!Thorough testing of environment variable generation with various field combinations. Correctly verifies both AZURE_* and ARM_* prefixes.
470-544: Excellent environment preparation test.Thorough coverage of merge and override behavior. The immutability check (lines 536-541) is particularly good defensive testing, ensuring the original environment map isn't mutated.
546-576: LGTM!Good coverage of logout behavior and type handling edge cases. The PrincipalFieldTypes test ensures robustness against configuration errors.
pkg/auth/providers/azure/device_code_test.go (4)
16-169: Excellent constructor test coverage.The table-driven tests comprehensively cover valid configurations, missing required fields, nil/empty specs, and invalid provider kinds. Good use of custom checkProvider functions and proper error type assertions.
332-421: Solid environment preparation tests.The PrepareEnvironment test thoroughly validates both the presence of expected variables and the removal of conflicting credentials. The dual approach with
expectedContainsandexpectedMissingcatches both false negatives and false positives.
487-569: Thorough spinner model tests.The sub-tests comprehensively cover initialization, message handling (success/error/Ctrl+C), and view rendering in different states. Good coverage of the interactive UI component.
423-457: No issues found.The mock is properly defined in
device_code_cache_test.gowithin the same test package, making it accessible to this test file. The pattern of centralizing shared test utilities in a separate file is a valid approach.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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)
website/blog/2025-11-07-azure-authentication-support.mdx(1 hunks)website/blog/authors.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- website/blog/authors.yml
🧰 Additional context used
📓 Path-based instructions (1)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use cases
Files:
website/blog/2025-11-07-azure-authentication-support.mdx
🧠 Learnings (2)
📓 Common learnings
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
📚 Learning: 2025-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:
website/blog/2025-11-07-azure-authentication-support.mdx
🪛 LanguageTool
website/blog/2025-11-07-azure-authentication-support.mdx
[grammar] ~374-~374: Please add a punctuation mark at the end of paragraph.
Context: ...b.com/cloudposse/atmos/issues) - Slack Community Special...
(PUNCTUATION_PARAGRAPH_END)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build (linux)
- GitHub Check: Build (windows)
- GitHub Check: Analyze (go)
- GitHub Check: website-deploy-preview
- GitHub Check: Lint (golangci)
- GitHub Check: autofix
- GitHub Check: Review Dependency Licenses
- GitHub Check: Summary
🔇 Additional comments (1)
website/blog/2025-11-07-azure-authentication-support.mdx (1)
328-330: Verify documentation links exist.The references to documentation pages should be verified to ensure they exist and are correctly linked:
/cli/commands/auth/tutorials/azure-authentication/cli/commands/auth/login/cli/commands/auth/usageBroken links would degrade user experience. Since this blog post is part of a larger feature rollout that should include updated docs, confirm these pages have been created or updated as part of the PR.
|
@coderabit full review |
1 similar comment
|
@coderabit full review |
|
@coderabbitai full review please |
|
@aknysh: I'll conduct a comprehensive review of the Azure authentication implementation. ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
pkg/auth/providers/azure/device_code_test.go (1)
16-661: Consider adding minimal Authenticate() coverage (follow‑up on earlier note).You’ve got solid coverage around construction, env prep, and helpers, but
Authenticateitself still isn’t exercised; that’s the one place all the flows and cache wiring converge. Even a couple of table‑driven smoke tests using a mockedpublic.Client(or small seam around token acquisition) would help guard regressions. Since this was raised before, treating it as a non‑blocking enhancement rather than a blocker.pkg/auth/cloud/azure/setup.go (1)
218-229: Refresh tokens still missing from MSAL cache.The
updateMSALCachecall passes access tokens for management, Graph, and KeyVault APIs, but no refresh token. A previous review flagged that Azure CLI persists refresh tokens to silently renew credentials and mint new scopes. Without refresh tokens, sessions expire in ~1 hour and new scope requests fail, breaking the "drop-in replacement for az login" goal.Extend the credential flow and this call to include and persist refresh tokens.
🧹 Nitpick comments (5)
pkg/auth/providers/azure/device_code.go (2)
593-605: Fix PrepareEnvironment docstring to match actual behavior.The comment here still mentions setting
ARM_USE_OIDC, butazureCloud.PrepareEnvironmentactually setsARM_USE_CLI(plus the AZURE_/ARM_ subscription, tenant, and location vars). Worth updating the docstring so future readers aren’t misled about which auth mechanism is in play.
27-33: ReusedefaultAzureClientIDinstead of re‑hardcoding the GUID.You already define
defaultAzureClientIDfor the Azure CLI public client ID;updateAzureCLICachere‑introduces the same GUID as a raw string. Using the constant here will keep things in sync if that ever needs to change.Also applies to: 241-243
pkg/auth/providers/azure/device_code_cache.go (2)
183-199: AligndeleteCachedTokenbehavior with its comment.The comment says this returns an error if cache deletion fails, but
Removeerrors (other thanIsNotExist) are logged and then ignored. Either propagate those errors, or relax the comment to make it clear that deletion failures are intentionally treated as non‑fatal.
129-147: Consider validating Graph token expiry when using cached tokens.
loadCachedTokenchecksExpiresAtwith a 5‑minute buffer but doesn’t look atGraphAPIExpiresAtbefore returningGraphAPIToken. If a consumer starts relying on the cached Graph token, you could end up reusing it past its expiry while the management token is still valid. Not urgent, but worth tightening if you ever wire this cache into the main auth flow.pkg/auth/cloud/azure/setup.go (1)
476-531: Consider unexporting if not used externally.
UpdateSubscriptionsInProfileis exported but appears to be used only within this package. If it's not called from tests or other packages, making it unexported would better encapsulate the implementation.The subscription update logic itself is correct—properly handles both update-existing and insert-new cases.
📜 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 (21)
errors/errors.go(1 hunks)go.mod(2 hunks)internal/exec/utils.go(2 hunks)pkg/auth/cloud/azure/console.go(1 hunks)pkg/auth/cloud/azure/constants.go(1 hunks)pkg/auth/cloud/azure/env.go(1 hunks)pkg/auth/cloud/azure/env_test.go(1 hunks)pkg/auth/cloud/azure/msal_cache.go(1 hunks)pkg/auth/cloud/azure/setup.go(1 hunks)pkg/auth/cloud/azure/setup_test.go(1 hunks)pkg/auth/identities/azure/subscription.go(1 hunks)pkg/auth/identities/azure/subscription_test.go(1 hunks)pkg/auth/providers/azure/cli.go(1 hunks)pkg/auth/providers/azure/cli_test.go(1 hunks)pkg/auth/providers/azure/device_code.go(1 hunks)pkg/auth/providers/azure/device_code_cache.go(1 hunks)pkg/auth/providers/azure/device_code_cache_test.go(1 hunks)pkg/auth/providers/azure/device_code_test.go(1 hunks)pkg/auth/types/aws_credentials_test.go(1 hunks)pkg/auth/types/azure_credentials_test.go(1 hunks)tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden
🚧 Files skipped from review as they are similar to previous changes (5)
- pkg/auth/cloud/azure/env_test.go
- internal/exec/utils.go
- pkg/auth/identities/azure/subscription_test.go
- pkg/auth/providers/azure/cli.go
- pkg/auth/types/azure_credentials_test.go
🧰 Additional context used
🧠 Learnings (23)
📚 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/types/aws_credentials_test.gopkg/auth/cloud/azure/console.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/auth/cloud/azure/env.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.
Applied to files:
pkg/auth/cloud/azure/env.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
pkg/auth/cloud/azure/env.gopkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
pkg/auth/cloud/azure/env.gopkg/auth/providers/azure/cli_test.gopkg/auth/providers/azure/device_code_cache_test.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:
pkg/auth/cloud/azure/env.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
pkg/auth/cloud/azure/env.go
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
pkg/auth/cloud/azure/env.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/identities/azure/subscription.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-03-25T12:24:36.177Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:263-264
Timestamp: 2025-03-25T12:24:36.177Z
Learning: Tests for the default Bitbucket username fallback to "x-token-auth" will be added during a future refactoring phase rather than in this PR.
Applied to files:
pkg/auth/providers/azure/device_code_test.go
📚 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:
pkg/auth/providers/azure/device_code_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: Final XDG Base Directory Specification implementation for atmos toolchain is complete and verified: toolchain/xdg_cache.go provides GetXDGCacheDir() and GetXDGTempCacheDir() functions, all hardcoded ~/.cache/tools-cache paths have been replaced with XDG-compliant paths using ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback), and tests have been updated to expect the new path structure.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: XDG Base Directory Specification compliance implementation for atmos toolchain is complete: created toolchain/xdg_cache.go with GetXDGCacheDir() and GetXDGTempCacheDir() functions, updated toolchain/installer.go and cmd/toolchain_clean.go to use these XDG helpers, and changed all cache paths from hardcoded ~/.cache/tools-cache to XDG-compliant ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback).
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/providers/azure/device_code_cache.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain XDG compliance implementation is complete with GetXDGCacheDir() and GetXDGTempCacheDir() functions in toolchain/xdg_cache.go, updated installer.go and toolchain_clean.go to use these helpers, and changed cache paths from ~/.cache/tools-cache to ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain when XDG_CACHE_HOME is not set).
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/providers/azure/device_code_cache.go
📚 Learning: 2024-12-13T15:33:34.159Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:17-31
Timestamp: 2024-12-13T15:33:34.159Z
Learning: In `pkg/config/cache.go`, when `XDG_CACHE_HOME` is not set, falling back to `.` (current directory) is acceptable and aligns with the requirement to primarily use `XDG_CACHE_HOME` for the cache directory.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2024-12-13T16:51:37.868Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:69-97
Timestamp: 2024-12-13T16:51:37.868Z
Learning: In `pkg/config/cache.go`, the function `SaveCache2` is currently unused because it does not work properly on Windows and will be addressed later.
Applied to files:
pkg/auth/cloud/azure/msal_cache.gopkg/auth/providers/azure/device_code.go
📚 Learning: 2024-12-13T16:48:00.294Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:42-42
Timestamp: 2024-12-13T16:48:00.294Z
Learning: The function `withCacheFileLock` in `pkg/config/cache.go` is currently unused and left for future upgrade.
Applied to files:
pkg/auth/cloud/azure/msal_cache.go
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: The project uses Go version 1.23.0 which has been confirmed by the maintainer to be working in production for months. Do not flag this as an invalid Go version.
Applied to files:
go.mod
📚 Learning: 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-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
go.mod
📚 Learning: 2025-11-10T20:03:56.875Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1775
File: pkg/auth/providers/aws/sso_provisioning.go:40-79
Timestamp: 2025-11-10T20:03:56.875Z
Learning: In the Atmos AWS SSO provider (pkg/auth/providers/aws/sso_provisioning.go), the OAuth access token from the AWS SSO device flow is intentionally stored in the `AccessKeyID` field of `AWSCredentials` during authentication. This token is then extracted and used for ListAccounts and ListAccountRoles API calls during identity provisioning. This design reuses the existing `AWSCredentials` type for token transport rather than creating a separate credential type.
Applied to files:
pkg/auth/providers/azure/device_code.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
pkg/auth/providers/azure/device_code.go
🧬 Code graph analysis (12)
pkg/auth/types/aws_credentials_test.go (1)
pkg/auth/types/aws_credentials.go (1)
AWSCredentials(16-24)
pkg/auth/cloud/azure/env.go (2)
pkg/perf/perf.go (1)
Track(121-138)pkg/logger/log.go (1)
Debug(24-26)
pkg/auth/cloud/azure/console.go (5)
pkg/perf/perf.go (1)
Track(121-138)pkg/auth/types/interfaces.go (2)
ICredentials(254-265)ConsoleURLOptions(297-314)pkg/logger/log.go (1)
Debug(24-26)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)errors/errors.go (1)
ErrInvalidAuthConfig(456-456)
pkg/auth/identities/azure/subscription.go (10)
errors/errors.go (3)
ErrInvalidIdentityConfig(458-458)ErrInvalidIdentityKind(457-457)ErrAuthenticationFailed(461-461)pkg/schema/schema.go (3)
Context(451-466)Validate(189-191)AuthContext(540-550)pkg/auth/types/interfaces.go (2)
ICredentials(254-265)PostAuthenticateParams(61-68)pkg/perf/perf.go (1)
Track(121-138)pkg/logger/log.go (1)
Debug(24-26)pkg/auth/cloud/azure/constants.go (2)
LogFieldIdentity(42-42)LogFieldSubscription(43-43)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/cloud/azure/env.go (1)
PrepareEnvironment(68-124)pkg/auth/cloud/azure/setup.go (4)
SetupFiles(22-40)UpdateAzureCLIFiles(184-241)SetAuthContextParams(43-50)SetAuthContext(54-106)pkg/auth/cloud/azure/files.go (1)
NewAzureFileManager(82-99)
pkg/auth/providers/azure/device_code_test.go (3)
errors/errors.go (3)
ErrInvalidProviderConfig(460-460)ErrInvalidProviderKind(459-459)ErrAuthenticationFailed(461-461)pkg/auth/providers/azure/device_code.go (1)
NewDeviceCodeProvider(77-102)pkg/auth/cloud/azure/env.go (1)
PrepareEnvironment(68-124)
pkg/auth/providers/azure/cli_test.go (3)
errors/errors.go (2)
ErrInvalidProviderConfig(460-460)ErrInvalidProviderKind(459-459)pkg/auth/providers/azure/cli.go (1)
NewCLIProvider(40-77)pkg/auth/cloud/azure/env.go (1)
PrepareEnvironment(68-124)
pkg/auth/providers/azure/device_code_cache_test.go (1)
pkg/auth/cloud/azure/constants.go (1)
FieldAccessToken(29-29)
pkg/auth/providers/azure/device_code_cache.go (5)
pkg/xdg/xdg.go (1)
GetXDGCacheDir(32-34)pkg/logger/log.go (1)
Debug(24-26)pkg/auth/cloud/azure/constants.go (8)
FieldHomeAccountID(24-24)FieldEnvironment(25-25)FieldRealm(26-26)LogFieldKey(46-46)DirPermissions(6-6)FilePermissions(8-8)FieldAccessToken(29-29)IntFormat(35-35)errors/errors.go (3)
ErrAzureOIDClaimNotFound(104-104)ErrAzureUsernameClaimNotFound(105-105)ErrAzureInvalidJWTFormat(106-106)pkg/auth/cloud/azure/setup.go (1)
UpdateSubscriptionsInProfile(478-531)
pkg/auth/cloud/azure/setup.go (7)
pkg/auth/types/interfaces.go (1)
ICredentials(254-265)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/cloud/azure/files.go (1)
NewAzureFileManager(82-99)errors/errors.go (5)
ErrAuthenticationFailed(461-461)ErrInvalidAuthConfig(456-456)ErrAzureOIDClaimNotFound(104-104)ErrAzureUsernameClaimNotFound(105-105)ErrAzureInvalidJWTFormat(106-106)pkg/schema/schema.go (3)
AuthContext(540-550)ConfigAndStacksInfo(592-685)AzureAuthContext(573-590)pkg/auth/cloud/azure/env.go (2)
PrepareEnvironment(68-124)PrepareEnvironmentConfig(46-51)pkg/auth/cloud/azure/constants.go (11)
FieldAccessToken(29-29)FieldHomeAccountID(24-24)FieldEnvironment(25-25)FieldRealm(26-26)DirPermissions(6-6)FilePermissions(8-8)IntFormat(35-35)FieldUser(30-30)BomMarker(14-14)BomSecondByte(16-16)BomThirdByte(18-18)
pkg/auth/cloud/azure/msal_cache.go (2)
pkg/auth/cloud/azure/constants.go (2)
DirPermissions(6-6)FilePermissions(8-8)pkg/logger/log.go (1)
Debug(24-26)
pkg/auth/providers/azure/device_code.go (9)
internal/tui/templates/term/term_writer.go (1)
IsTTYSupportForStderr(127-129)pkg/auth/providers/azure/device_code_cache.go (1)
CacheStorage(47-58)errors/errors.go (5)
ErrInvalidProviderConfig(460-460)ErrInvalidProviderKind(459-459)ErrAuthenticationFailed(461-461)ErrAzureNoAccountsInCache(109-109)ErrAzureNoAccountForTenant(110-110)pkg/auth/cloud/azure/msal_cache.go (1)
NewMSALCache(21-39)pkg/telemetry/ci.go (1)
IsCI(81-86)pkg/utils/url_utils.go (1)
OpenUrl(13-39)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/ui/theme/colors.go (4)
ColorCyan(32-32)ColorGray(30-30)ColorGreen(31-31)ColorBlue(34-34)pkg/auth/cloud/azure/env.go (2)
PrepareEnvironment(68-124)PrepareEnvironmentConfig(46-51)
pkg/auth/cloud/azure/setup_test.go (7)
pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/types/interfaces.go (1)
ICredentials(254-265)pkg/auth/types/aws_credentials.go (1)
AWSCredentials(16-24)pkg/auth/cloud/azure/setup.go (4)
SetupFiles(22-40)SetAuthContextParams(43-50)SetAuthContext(54-106)SetEnvironmentVariables(142-179)pkg/schema/schema.go (3)
AuthContext(540-550)ConfigAndStacksInfo(592-685)AzureAuthContext(573-590)errors/errors.go (1)
ErrInvalidAuthConfig(456-456)pkg/auth/cloud/azure/files.go (1)
NewAzureFileManager(82-99)
⏰ 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). (10)
- GitHub Check: Build (macos)
- GitHub Check: Build (windows)
- GitHub Check: Build (linux)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: website-deploy-preview
- GitHub Check: Review Dependency Licenses
- GitHub Check: Run pre-commit hooks
- GitHub Check: autofix
- GitHub Check: Summary
🔇 Additional comments (27)
pkg/auth/types/aws_credentials_test.go (1)
36-40: Refactor verified—helper is properly implemented and provides comprehensive coverage.The
testGetExpirationhelper exists inpkg/auth/types/azure_credentials_test.goand tests all necessary cases: blank expiration, valid RFC3339 format, and invalid input with error handling. Accessible to both AWS and Azure credentials tests within the same package.go.mod (2)
12-12: LGTM: Azure dependencies added.The new Azure SDK armsubscriptions and MSAL libraries align with the PR's Azure authentication feature.
Also applies to: 15-15
3-3: The original review comment is outdated—Go 1.25.0 is the latest stable release.Go 1.25.0 was released on August 12, 2025 and is the current stable version. The go.mod file is correct; no changes needed.
Likely an incorrect or invalid review comment.
pkg/auth/cloud/azure/console.go (1)
60-109: LGTM: Console URL generator implementation is solid.The Azure portal URL generation handles tenant context correctly, validates credentials thoroughly, and includes informative debug logging. Past review concerns about nil checks have been addressed.
errors/errors.go (1)
103-111: LGTM: Azure error sentinels are well-defined.The new error constants cover JWT parsing, cache management, and account lookup scenarios comprehensively.
pkg/auth/providers/azure/cli_test.go (2)
15-145: LGTM: Comprehensive test coverage for CLI provider.The table-driven tests validate constructor behavior across multiple scenarios, including error cases. Good use of testify assertions.
375-451: LGTM: Environment preparation tests are thorough.The tests verify both the basic environment setup and the clearing of conflicting Azure credential variables, which aligns with the provider's design.
pkg/auth/identities/azure/subscription.go (2)
25-63: LGTM: Constructor properly validates subscription identity config.The validation enforces required fields (subscription_id) and extracts optional fields (resource_group, location) cleanly.
78-120: LGTM: Authentication correctly preserves tokens while overriding subscription.The implementation preserves Graph API and KeyVault tokens from the provider while applying identity-specific subscription and location, which is the correct behavior for subscription-scoped identities.
pkg/auth/cloud/azure/constants.go (1)
1-47: LGTM: Constants are well-organized and documented.The file provides clear, shared constants for file permissions, BOM handling, MSAL cache keys, and logging fields used across the Azure auth implementation.
pkg/auth/cloud/azure/msal_cache.go (2)
19-39: LGTM: Cache initialization handles path defaults correctly.The constructor defaults to the Azure CLI location and ensures the directory exists with secure permissions.
41-66: LGTM: Cache operations are context-aware and error-tolerant.Both Replace and Export check for context cancellation and handle file errors gracefully. Treating missing or corrupted cache as non-fatal (starting fresh) is the right approach for a token cache.
Also applies to: 68-88
pkg/auth/cloud/azure/env.go (2)
8-43: The env var clearing strategy is sound for Atmos-managed auth.The list targets Azure-specific credential variables that could conflict with Atmos-managed credentials. The code explicitly preserves AWS/GCP variables for multi-cloud scenarios (noted in line 64-65 and 85).
68-124: LGTM: Environment preparation is clean and well-documented.The function creates a defensive copy, clears conflicting Azure credentials, sets Atmos-managed values, and enables Azure CLI authentication mode. The multi-cloud concern raised in past reviews is addressed by preserving non-Azure credentials.
pkg/auth/cloud/azure/setup_test.go (1)
28-872: Azure setup tests look solid and hermetic.Nice job keeping everything under
t.TempDirand thoroughly exercisingSetupFiles,SetAuthContext, env population, JWT helpers, MSAL cache, andazureProfilehandling (including BOM). This gives good confidence in the Azure setup path.pkg/auth/providers/azure/device_code_cache_test.go (1)
20-880: Good isolation around cache and CLI integration tests.The in‑memory
mockCacheStorageplust.TempDir/t.SetenvaroundupdateAzureCLICachekeep these tests hermetic while still exercising real JSON layouts and edge cases (invalid JSON, missing sections, wrong types). This is exactly the kind of coverage this cache logic needs.pkg/auth/cloud/azure/setup.go (11)
19-40: LGTM!The function correctly handles type assertion, error propagation, and delegates file operations to the file manager. The early return pattern for non-Azure credentials is clean.
42-106: LGTM!The parameter struct is well-designed, nil checks are appropriate, and the location override mechanism properly prioritizes component-level config. Debug logging provides good observability.
108-130: LGTM!Defensive nil checks and safe type assertions throughout. The function gracefully handles missing or malformed configuration by returning an empty string.
154-161: Non-string environment values are silently dropped.Lines 157-160 only copy string values from the existing ComponentEnvSection. If any non-string values exist (e.g., numbers, booleans), they're silently discarded. This is likely correct since environment variables must be strings, but worth confirming that upstream code doesn't rely on those types being preserved.
If this behavior is intentional, consider adding a debug log when skipping non-string values to aid troubleshooting.
256-370: MSAL cache implementation looks solid.The function properly manages the cache structure, handles missing files, adds all required sections (AccessToken, Account), and uses appropriate file permissions. The hardcoded Azure CLI public client ID at line 292 (
04b07795-8ddb-461a-bbee-02f9e1bf7b46) is the official Microsoft identifier for Azure CLI, which is correct for MSAL compatibility.
372-388: LGTM!Clean implementation with appropriate error handling. Returns empty cache when file doesn't exist, which is the correct default behavior.
390-433: LGTM!The function correctly formats MSAL cache entries with all required fields, proper timestamp handling, and appropriate cache key structure. Non-fatal error handling for expiration parsing is reasonable given the fallback behavior.
435-474: LGTM!Proper handling of Azure CLI quirks (UTF-8 BOM), sensible defaults for new profiles, and correct file permissions. The delegation to
UpdateSubscriptionsInProfilekeeps concerns separated.
533-576: LGTM!JWT extraction logic is robust. Trying multiple claim names for username (upn → unique_name → email) handles different token types correctly. The fallback username is a reasonable default.
578-596: LGTM!Standard JWT parsing implementation. Correctly validates the three-part structure, uses
base64.RawURLEncodingfor JWT payload decoding, and provides clear error messages.
598-606: LGTM!Clean BOM handling with proper length check to avoid panic. The comment clearly explains why this is necessary for Azure CLI compatibility.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
NOTICE (1)
963-966: NOTICE file needs regeneration to resolve pipeline failure.The dependency review check is failing because the NOTICE file is out of date. Run the script to regenerate it:
./scripts/generate-notice.shpkg/auth/cloud/azure/msal_cache_test.go (1)
1-1: Formatting issue already flagged.The go-fumpt formatter needs to be run on this file. This was already noted in a previous review.
pkg/auth/cloud/azure/files_test.go (1)
433-474: Concurrency test issue already noted.This test performs sequential writes rather than concurrent operations. The concern was already raised in a previous review.
pkg/auth/cloud/azure/env.go (1)
42-42: Formatting issue already flagged.The go-fumpt formatting error on this line was noted in a previous review.
pkg/auth/types/azure_credentials.go (1)
56-111: AlignWhoamiInfo.Accountwith tenant semantics (useTenantID, not subscription).Right now
BuildWhoamiInfosetsinfo.AccountfromSubscriptionID, whileValidatebuildsValidationInfo{Account: c.TenantID}. That’s inconsistent and ends up surfacing the subscription as the “account” in one code path and the tenant in another.If the shared contract is that “account” for Azure means tenant, this should be wired through
BuildWhoamiInfoas well:func (c *AzureCredentials) BuildWhoamiInfo(info *WhoamiInfo) { info.Region = c.Location - if c.SubscriptionID != "" { - info.Account = c.SubscriptionID - } + if c.TenantID != "" { + info.Account = c.TenantID + } if t, _ := c.GetExpiration(); t != nil { info.Expiration = t } }This keeps Whoami and validation output consistent for Azure identities.
pkg/auth/providers/azure/device_code_cache.go (1)
488-506: Still strip BOM before parsing azureProfile.jsonazureProfile.json that Azure CLI writes on Windows/macOS often starts with the UTF‑8 BOM. Without trimming it, the
json.Unmarshalhere fails and we silently skip the subscription update, undoing the workflows you just fixed for azuread/azapi on cached runs. Please mirror the BOM stripping already used inpkg/auth/cloud/azure/setup.gobefore unmarshalling.@@ - if err := json.Unmarshal(data, &profile); err != nil { + if len(data) >= 3 && data[0] == 0xEF && data[1] == 0xBB && data[2] == 0xBF { + data = data[3:] + } + if err := json.Unmarshal(data, &profile); err != nil {pkg/auth/cloud/azure/setup.go (1)
256-370: Refresh tokens still missing from MSAL cache.This is a continuation of the earlier review feedback. The
updateMSALCachefunction only writesAccessTokenandAccountsections. Azure CLI persists refresh tokens to enable silent token renewal and acquisition of new scopes; without them, Atmos-acquired tokens expire in ~1h and cannot be renewed, breaking the goal of replicatingaz loginbehavior.Extend the cache structure to include a
RefreshTokensection and persist refresh tokens alongside access tokens.
🧹 Nitpick comments (8)
internal/exec/terraform_generate_backends.go (1)
352-356: AuthContext left nil here is fine, but note future Azure needsPassing
nilfor the newAuthContextparameter keeps this JSON backend generation path behaviourally identical to before the signature change. If you later need auth-aware backend tweaks (e.g. Azure-specific behaviour) forterraform generate backends, you’ll want to plumb a real*schema.AuthContextinto this function and pass it through instead ofnil.pkg/auth/providers/azure/device_code_cache_test.go (2)
20-76: Tighten mock Remove semantics to exercise “file not found” pathRight now
mockCacheStorage.Removealways returnsnil(unlessremoveErroris set), even when the file key doesn’t exist. That meansTestDeviceCodeProvider_deleteCachedToken’s “token doesn't exist” case never actually simulatesos.ErrNotExist, so the real error‑handling path indeleteCachedTokenisn’t exercised.You can make the test more realistic by returning
os.ErrNotExistwhen the entry is missing:func (m *mockCacheStorage) Remove(path string) error { if m.removeError != nil { return m.removeError } - delete(m.files, path) - return nil + if _, ok := m.files[path]; !ok { + return os.ErrNotExist + } + delete(m.files, path) + return nil }This keeps the happy path intact while validating that
deleteCachedTokentreats “file not found” as non‑fatal the way the real filesystem does.Also applies to: 319-365
557-690: Azure profile + CLI cache integration tests look good; minor setup nitNice job keeping
updateAzureProfileandupdateAzureCLICachetests fully sandboxed:
updateAzureProfileworks against per‑test directories undert.TempDir().updateAzureCLICache_Integrationnow isolatesHOME,USERPROFILE, andAZURE_CONFIG_DIRwitht.Setenv, so no accidental writes into a real user profile.One small polish you could add is to fail fast on filesystem issues in the profile setup lambdas by checking errors from
os.MkdirAll/os.WriteFilewithrequire.NoError. That would make a broken temp FS or permission problem show up as a clear test failure instead of silently corrupting the fixture.Overall behaviour under test looks correct.
Also applies to: 692-739
pkg/auth/providers/azure/cli_test.go (1)
177-192: Prefer a real context overnilinPreAuthenticatetest.You’re currently calling
PreAuthenticate(nil). If the implementation ever starts using the context (logging, tracing, deadlines), this could mask bugs. Usingcontext.Background()here keeps the test future‑proof without changing behavior today.errors/errors.go (1)
94-112: Azure auth error sentinels look good; consider regroupingErrBackendConfigRequired.The new Azure auth errors are clear and specific, which will help debugging. One tiny clarity nit:
ErrBackendConfigRequiredis generic and now lives inside the “Azure authentication errors” block, which may mislead future readers into assuming it’s Azure‑specific. Consider moving it back alongside the other backend config errors to keep the taxonomy tidy.pkg/auth/types/azure_credentials.go (2)
31-54: Optional: reuseGetExpirationinIsExpiredto avoid duplicate parsing.
IsExpiredandGetExpirationboth parsec.Expirationindependently, with slightly different semantics. Not a bug, but you could consolidate to reduce duplication and keep behavior in one place, e.g. by callingGetExpirationfromIsExpiredand treating a parse error as expired there.
69-90: Validate() always targets the default Azure cloud — confirm that’s intentional.
Validatebuilds anarmsubscriptions.Clientwithniloptions, so it will always talk to the default public Azure ARM endpoint. If you ever support sovereign clouds or custom endpoints elsewhere in the Azure stack, you may want to thread through whatever environment/client options you use there to keep validation consistent with runtime auth behavior.pkg/auth/identities/azure/subscription.go (1)
133-171: Consider reusingEnvironment()insidePrepareEnvironment()to avoid duplication.Both
EnvironmentandPrepareEnvironmentbuild the sameAZURE_SUBSCRIPTION_ID/ARM_SUBSCRIPTION_ID/AZURE_LOCATION/ARM_LOCATION/AZURE_RESOURCE_GROUPset. Not a big deal, but callingEnvironment()and merging its result intoenvironwould DRY this up and keep the two paths in sync if you ever tweak the env contract.
📜 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 (41)
NOTICE(2 hunks)cmd/auth_console.go(2 hunks)cmd/auth_console_test.go(1 hunks)errors/errors.go(1 hunks)go.mod(2 hunks)internal/exec/terraform_generate_backend.go(1 hunks)internal/exec/terraform_generate_backends.go(1 hunks)internal/exec/terraform_generate_backends_test.go(6 hunks)internal/exec/terraform_output_utils.go(2 hunks)internal/exec/terraform_utils.go(2 hunks)internal/exec/utils.go(2 hunks)internal/exec/utils_test.go(2 hunks)pkg/auth/cloud/azure/console.go(1 hunks)pkg/auth/cloud/azure/console_test.go(1 hunks)pkg/auth/cloud/azure/constants.go(1 hunks)pkg/auth/cloud/azure/env.go(1 hunks)pkg/auth/cloud/azure/env_test.go(1 hunks)pkg/auth/cloud/azure/files.go(1 hunks)pkg/auth/cloud/azure/files_test.go(1 hunks)pkg/auth/cloud/azure/msal_cache.go(1 hunks)pkg/auth/cloud/azure/msal_cache_test.go(1 hunks)pkg/auth/cloud/azure/setup.go(1 hunks)pkg/auth/cloud/azure/setup_test.go(1 hunks)pkg/auth/factory/factory.go(3 hunks)pkg/auth/identities/azure/subscription.go(1 hunks)pkg/auth/identities/azure/subscription_test.go(1 hunks)pkg/auth/providers/azure/cli.go(1 hunks)pkg/auth/providers/azure/cli_test.go(1 hunks)pkg/auth/providers/azure/device_code.go(1 hunks)pkg/auth/providers/azure/device_code_cache.go(1 hunks)pkg/auth/providers/azure/device_code_cache_test.go(1 hunks)pkg/auth/providers/azure/device_code_test.go(1 hunks)pkg/auth/types/aws_credentials_test.go(1 hunks)pkg/auth/types/azure_credentials.go(1 hunks)pkg/auth/types/azure_credentials_test.go(1 hunks)pkg/auth/types/constants.go(1 hunks)pkg/schema/schema.go(2 hunks)tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden(1 hunks)website/blog/2025-11-07-azure-authentication-support.mdx(1 hunks)website/docs/cli/commands/auth/auth-login.mdx(1 hunks)website/docs/cli/commands/auth/tutorials/azure-authentication.mdx(1 hunks)
🧰 Additional context used
🧠 Learnings (55)
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
internal/exec/terraform_output_utils.gointernal/exec/utils.gointernal/exec/utils_test.gointernal/exec/terraform_generate_backends_test.gointernal/exec/terraform_generate_backends.gointernal/exec/terraform_generate_backend.gointernal/exec/terraform_utils.go
📚 Learning: 2024-12-03T03:52:02.524Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:144-145
Timestamp: 2024-12-03T03:52:02.524Z
Learning: Avoid adding context timeouts to Terraform commands in `execTerraformOutput` because their execution time can vary from seconds to hours, and making it configurable would require redesigning the command interface.
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
internal/exec/terraform_output_utils.gointernal/exec/utils.gopkg/auth/providers/azure/device_code.gointernal/exec/terraform_utils.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
internal/exec/terraform_output_utils.gointernal/exec/utils.gointernal/exec/utils_test.gointernal/exec/terraform_generate_backends.go
📚 Learning: 2025-03-25T12:24:36.177Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:263-264
Timestamp: 2025-03-25T12:24:36.177Z
Learning: Tests for the default Bitbucket username fallback to "x-token-auth" will be added during a future refactoring phase rather than in this PR.
Applied to files:
pkg/auth/providers/azure/device_code_test.go
📚 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:
pkg/auth/providers/azure/device_code_test.gowebsite/blog/2025-11-07-azure-authentication-support.mdxwebsite/docs/cli/commands/auth/auth-login.mdx
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: Final XDG Base Directory Specification implementation for atmos toolchain is complete and verified: toolchain/xdg_cache.go provides GetXDGCacheDir() and GetXDGTempCacheDir() functions, all hardcoded ~/.cache/tools-cache paths have been replaced with XDG-compliant paths using ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback), and tests have been updated to expect the new path structure.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/msal_cache_test.gopkg/auth/providers/azure/device_code_cache.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: XDG Base Directory Specification compliance implementation for atmos toolchain is complete: created toolchain/xdg_cache.go with GetXDGCacheDir() and GetXDGTempCacheDir() functions, updated toolchain/installer.go and cmd/toolchain_clean.go to use these XDG helpers, and changed all cache paths from hardcoded ~/.cache/tools-cache to XDG-compliant ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback).
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/msal_cache_test.gopkg/auth/cloud/azure/msal_cache.gopkg/auth/providers/azure/device_code_cache.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain XDG compliance implementation is complete with GetXDGCacheDir() and GetXDGTempCacheDir() functions in toolchain/xdg_cache.go, updated installer.go and toolchain_clean.go to use these helpers, and changed cache paths from ~/.cache/tools-cache to ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain when XDG_CACHE_HOME is not set).
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/msal_cache_test.gopkg/auth/cloud/azure/msal_cache.gopkg/auth/providers/azure/device_code_cache.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/env.gopkg/auth/cloud/azure/env_test.gopkg/auth/providers/azure/cli_test.go
📚 Learning: 2024-12-13T15:33:34.159Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:17-31
Timestamp: 2024-12-13T15:33:34.159Z
Learning: In `pkg/config/cache.go`, when `XDG_CACHE_HOME` is not set, falling back to `.` (current directory) is acceptable and aligns with the requirement to primarily use `XDG_CACHE_HOME` for the cache directory.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/env.gopkg/auth/cloud/azure/env_test.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/utils.gointernal/exec/utils_test.gointernal/exec/terraform_generate_backends_test.gointernal/exec/terraform_generate_backends.gointernal/exec/terraform_generate_backend.go
📚 Learning: 2024-10-31T07:09:31.983Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:181-182
Timestamp: 2024-10-31T07:09:31.983Z
Learning: In `internal/exec/vendor_utils.go`, the variables `mergedSources` and `mergedImports` are declared and used later in the code. Do not suggest removing them as unused variables.
Applied to files:
internal/exec/utils.gointernal/exec/utils_test.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/utils.go
📚 Learning: 2024-11-02T15:35:09.958Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Applied to files:
internal/exec/utils.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/types/azure_credentials_test.gopkg/auth/identities/azure/subscription_test.gopkg/auth/types/aws_credentials_test.gopkg/auth/cloud/azure/console.gopkg/auth/cloud/azure/setup.gopkg/auth/types/azure_credentials.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:
internal/exec/utils_test.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
cmd/auth_console.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
cmd/auth_console.gopkg/auth/factory/factory.gogo.mod
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
cmd/auth_console.gopkg/auth/cloud/azure/env.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
cmd/auth_console.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/auth_console.gopkg/auth/factory/factory.gopkg/auth/identities/azure/subscription.gopkg/schema/schema.gopkg/auth/providers/azure/cli.gopkg/auth/cloud/azure/files.gopkg/auth/cloud/azure/setup.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/auth_console.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: ErrWrappingFormat is correctly defined as "%w: %w" in the errors package and is used throughout the codebase to wrap two error types together. The usage fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) is the correct pattern when both arguments are error types.
Applied to files:
cmd/auth_console.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/auth_console.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/auth_console.gopkg/auth/factory/factory.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: The user confirmed that the errors package has an error string wrapping format, contradicting the previous learning about ErrWrappingFormat being invalid. The current usage of fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) appears to be the correct pattern.
Applied to files:
cmd/auth_console.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.
Applied to files:
pkg/auth/cloud/azure/env.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:
pkg/auth/cloud/azure/env.gointernal/exec/terraform_utils.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
pkg/auth/cloud/azure/env.go
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
pkg/auth/cloud/azure/env.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.
Applied to files:
pkg/auth/cloud/azure/env_test.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_backends.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_backends.go
📚 Learning: 2024-11-30T22:07:08.610Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/yaml_func_terraform_output.go:35-40
Timestamp: 2024-11-30T22:07:08.610Z
Learning: In the Go function `processTagTerraformOutput` in `internal/exec/yaml_func_terraform_output.go`, parameters cannot contain spaces. The code splits the input by spaces, and if the parameters contain spaces, `len(parts) != 3` will fail and show an error to the user.
Applied to files:
internal/exec/terraform_generate_backends.go
📚 Learning: 2025-11-01T20:24:29.557Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1714
File: NOTICE:0-0
Timestamp: 2025-11-01T20:24:29.557Z
Learning: In the cloudposse/atmos repository, the NOTICE file is programmatically generated and should not be manually edited. Issues with dependency license URLs in NOTICE will be resolved when upstream package metadata is corrected.
Applied to files:
NOTICE
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Applied to files:
website/blog/2025-11-07-azure-authentication-support.mdxtests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden
📚 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:
website/blog/2025-11-07-azure-authentication-support.mdx
📚 Learning: 2025-01-17T00:18:57.769Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
Applied to files:
website/blog/2025-11-07-azure-authentication-support.mdx
📚 Learning: 2025-02-18T13:13:11.497Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden:59-64
Timestamp: 2025-02-18T13:13:11.497Z
Learning: For Atmos CLI help text, angle brackets in command examples and flag descriptions should be escaped using HTML entities (e.g., `<component>`) rather than converted to backticks or other markdown formatting.
Applied to files:
website/blog/2025-11-07-azure-authentication-support.mdx
📚 Learning: 2025-11-10T20:03:56.875Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1775
File: pkg/auth/providers/aws/sso_provisioning.go:40-79
Timestamp: 2025-11-10T20:03:56.875Z
Learning: In the Atmos AWS SSO provider (pkg/auth/providers/aws/sso_provisioning.go), the OAuth access token from the AWS SSO device flow is intentionally stored in the `AccessKeyID` field of `AWSCredentials` during authentication. This token is then extracted and used for ListAccounts and ListAccountRoles API calls during identity provisioning. This design reuses the existing `AWSCredentials` type for token transport rather than creating a separate credential type.
Applied to files:
pkg/auth/providers/azure/device_code.gowebsite/docs/cli/commands/auth/auth-login.mdx
📚 Learning: 2024-12-13T16:51:37.868Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:69-97
Timestamp: 2024-12-13T16:51:37.868Z
Learning: In `pkg/config/cache.go`, the function `SaveCache2` is currently unused because it does not work properly on Windows and will be addressed later.
Applied to files:
pkg/auth/providers/azure/device_code.gopkg/auth/cloud/azure/msal_cache.go
📚 Learning: 2024-12-25T20:28:47.526Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/stack_processor_utils.go:380-380
Timestamp: 2024-12-25T20:28:47.526Z
Learning: Windows path handling often requires `filepath.Join` to ensure correct separators and comparisons. Insufficient tests can break cross-platform compatibility, so migrating from `path.Join` to `filepath.Join` needs thorough testing on Windows before merging.
Applied to files:
pkg/auth/cloud/azure/files_test.go
📚 Learning: 2024-12-13T16:48:00.294Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:42-42
Timestamp: 2024-12-13T16:48:00.294Z
Learning: The function `withCacheFileLock` in `pkg/config/cache.go` is currently unused and left for future upgrade.
Applied to files:
pkg/auth/cloud/azure/files_test.gopkg/auth/cloud/azure/msal_cache.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_utils.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
internal/exec/terraform_utils.go
📚 Learning: 2025-02-19T05:50:35.853Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden:0-0
Timestamp: 2025-02-19T05:50:35.853Z
Learning: Backtick formatting should only be applied to flag descriptions in Go source files, not in golden test files (test snapshots) as they are meant to capture the raw command output.
Applied to files:
pkg/auth/cloud/azure/msal_cache_test.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
pkg/auth/cloud/azure/msal_cache_test.gopkg/auth/providers/azure/device_code_cache.go
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden
📚 Learning: 2025-02-14T23:12:38.030Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1061
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:8-8
Timestamp: 2025-02-14T23:12:38.030Z
Learning: Test snapshots in the Atmos project, particularly for dry run scenarios, may be updated during the development process, and temporary inconsistencies in their content should not be flagged as issues.
Applied to files:
tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden
📚 Learning: 2025-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-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-09-10T17:34:52.568Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/providers/github/oidc.go:96-100
Timestamp: 2025-09-10T17:34:52.568Z
Learning: The ATMOS_ environment variable binding guideline applies to Atmos configuration variables, not external service-required environment variables like GitHub Actions OIDC variables (GITHUB_ACTIONS, ACTIONS_ID_TOKEN_*) which must use their standard names.
Applied to files:
website/docs/cli/commands/auth/auth-login.mdx
🪛 GitHub Actions: Dependency Review
NOTICE
[error] 1-1: NOTICE file is out of date. Run './scripts/generate-notice.sh' locally and commit the changes. (Triggered by: git diff --exit-code NOTICE)
🪛 LanguageTool
website/docs/cli/commands/auth/tutorials/azure-authentication.mdx
[typographical] ~86-~86: Consider using a typographic opening quote here.
Context: ... 1. Atmos displays a device code (e.g., "WDDD-HRQV") 2. Browser opens to Microsof...
(EN_QUOTES)
[typographical] ~86-~86: Consider using a typographic close quote here.
Context: ...displays a device code (e.g., "WDDD-HRQV") 2. Browser opens to Microsoft's device...
(EN_QUOTES)
[grammar] ~295-~295: Ensure spelling is correct
Context: ... scope } ``` ### azuread Provider The azuread provider automatically uses Graph API t...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~471-~471: You have used the passive voice repeatedly in nearby sentences. To make your writing clearer and easier to read, consider using active voice.
Context: ... provider compatibility - Both storages are secured by your operating system ### Least Pri...
(REP_PASSIVE_VOICE)
[typographical] ~489-~489: Consider using typographic quotation marks here.
Context: ...→ Monitoring → Sign-in logs - Filter by "Application" to see Atmos authentications ## Migrat...
(EN_QUOTES)
⏰ 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). (6)
- GitHub Check: [localstack] demo-localstack
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pkg/auth/cloud/azure/setup.go (1)
257-424: Refresh tokens still missing from MSAL cache.
updateMSALCacheand related functions only writeAccessTokenandAccountsections (lines 282-293). Azure CLI persists refresh tokens alongside access tokens for silent renewal and new scope acquisition. Without refresh tokens, tokens expire in ~1h and cannot be renewed, breaking the PR's goal of replicatingaz loginbehavior.Extend
msalCacheUpdatestruct (line 244),updateMSALCache, and helpers to persist refresh token entries with the same locking pattern used for access tokens.
🧹 Nitpick comments (4)
pkg/auth/cloud/azure/msal_cache.go (1)
41-112: Graceful cache I/O with locking is well done; consider homedir helper for consistencyThe Replace/Export paths handle context cancellation, locking, missing/corrupt cache, and write failures in a reasonable way. As a minor consistency tweak, you could consider using the existing
homedir.Dir()helper (likeAzureFileManagerdoes) instead ofos.UserHomeDir()so all Azure auth paths resolve “home” the same way, but this is purely nice-to-have.pkg/auth/providers/azure/device_code_cache.go (1)
354-386: Consider using the same file lock when reading the CLI cache
loadAndInitializeCLICachereadsmsal_token_cache.jsondirectly without using the.lockfile that writers rely on. In the (rare) case where another process is writing the file at the same time (either our ownwriteCacheFileWithLockingor MSAL viaNewMSALCache), we could briefly see truncated/invalid JSON and reset the in-memory cache map. You’re already handling parse errors gracefully, so this is not a correctness bug, but usingAcquireFileLockhere as well would tighten consistency.pkg/auth/providers/azure/device_code_ui.go (1)
177-210: Spinner model behavior is clear; optionally handleprog.SenderrorsThe spinner model cleanly handles success, error, and Ctrl+C, and renders the “waiting/success/empty-on-error” views as expected. One minor improvement you could consider is checking/logging the error returned by
prog.SendinwaitForAuthWithSpinnerin case the program has already exited when the auth goroutine completes—but ignoring it is also acceptable since the auth result is ultimately surfaced via context cancellation.pkg/auth/providers/azure/device_code.go (1)
441-481: Minor docstring drift in PrepareEnvironmentThe comment above
PrepareEnvironmentstill mentions settingARM_USE_OIDC, but the implementation delegates toazureCloud.PrepareEnvironment, which setsARM_USE_CLIinstead. Worth updating the comment so it matches the actual behavior; the code itself looks correct.
📜 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 (9)
pkg/auth/cloud/azure/files.go(1 hunks)pkg/auth/cloud/azure/msal_cache.go(1 hunks)pkg/auth/cloud/azure/setup.go(1 hunks)pkg/auth/providers/azure/device_code.go(1 hunks)pkg/auth/providers/azure/device_code_cache.go(1 hunks)pkg/auth/providers/azure/device_code_cache_test.go(1 hunks)pkg/auth/providers/azure/device_code_test.go(1 hunks)pkg/auth/providers/azure/device_code_ui.go(1 hunks)pkg/auth/providers/azure/device_code_ui_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (21)
📚 Learning: 2025-11-10T20:03:56.875Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1775
File: pkg/auth/providers/aws/sso_provisioning.go:40-79
Timestamp: 2025-11-10T20:03:56.875Z
Learning: In the Atmos AWS SSO provider (pkg/auth/providers/aws/sso_provisioning.go), the OAuth access token from the AWS SSO device flow is intentionally stored in the `AccessKeyID` field of `AWSCredentials` during authentication. This token is then extracted and used for ListAccounts and ListAccountRoles API calls during identity provisioning. This design reuses the existing `AWSCredentials` type for token transport rather than creating a separate credential type.
Applied to files:
pkg/auth/providers/azure/device_code.go
📚 Learning: 2024-12-13T16:51:37.868Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:69-97
Timestamp: 2024-12-13T16:51:37.868Z
Learning: In `pkg/config/cache.go`, the function `SaveCache2` is currently unused because it does not work properly on Windows and will be addressed later.
Applied to files:
pkg/auth/providers/azure/device_code.gopkg/auth/cloud/azure/msal_cache.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
pkg/auth/providers/azure/device_code.go
📚 Learning: 2024-12-13T16:48:00.294Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:42-42
Timestamp: 2024-12-13T16:48:00.294Z
Learning: The function `withCacheFileLock` in `pkg/config/cache.go` is currently unused and left for future upgrade.
Applied to files:
pkg/auth/cloud/azure/msal_cache.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: Final XDG Base Directory Specification implementation for atmos toolchain is complete and verified: toolchain/xdg_cache.go provides GetXDGCacheDir() and GetXDGTempCacheDir() functions, all hardcoded ~/.cache/tools-cache paths have been replaced with XDG-compliant paths using ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback), and tests have been updated to expect the new path structure.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: XDG Base Directory Specification compliance implementation for atmos toolchain is complete: created toolchain/xdg_cache.go with GetXDGCacheDir() and GetXDGTempCacheDir() functions, updated toolchain/installer.go and cmd/toolchain_clean.go to use these XDG helpers, and changed all cache paths from hardcoded ~/.cache/tools-cache to XDG-compliant ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback).
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain XDG compliance implementation is complete with GetXDGCacheDir() and GetXDGTempCacheDir() functions in toolchain/xdg_cache.go, updated installer.go and toolchain_clean.go to use these helpers, and changed cache paths from ~/.cache/tools-cache to ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain when XDG_CACHE_HOME is not set).
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/providers/azure/device_code_test.go
📚 Learning: 2024-12-13T15:33:34.159Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:17-31
Timestamp: 2024-12-13T15:33:34.159Z
Learning: In `pkg/config/cache.go`, when `XDG_CACHE_HOME` is not set, falling back to `.` (current directory) is acceptable and aligns with the requirement to primarily use `XDG_CACHE_HOME` for the cache directory.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
pkg/auth/providers/azure/device_code_cache.gopkg/auth/cloud/azure/setup.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/cloud/azure/setup.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/cloud/azure/setup.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:
pkg/auth/cloud/azure/setup.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/auth/cloud/azure/setup.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:
pkg/auth/cloud/azure/setup.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:
pkg/auth/cloud/azure/setup.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/cloud/azure/setup.go
📚 Learning: 2025-03-25T12:24:36.177Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:263-264
Timestamp: 2025-03-25T12:24:36.177Z
Learning: Tests for the default Bitbucket username fallback to "x-token-auth" will be added during a future refactoring phase rather than in this PR.
Applied to files:
pkg/auth/providers/azure/device_code_test.go
📚 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:
pkg/auth/providers/azure/device_code_test.go
🧬 Code graph analysis (9)
pkg/auth/providers/azure/device_code.go (7)
internal/tui/templates/term/term_writer.go (1)
IsTTYSupportForStderr(127-129)pkg/auth/providers/azure/device_code_cache.go (1)
CacheStorage(47-58)errors/errors.go (5)
ErrInvalidProviderConfig(460-460)ErrInvalidProviderKind(459-459)ErrAuthenticationFailed(461-461)ErrAzureNoAccountsInCache(109-109)ErrAzureNoAccountForTenant(110-110)pkg/auth/cloud/azure/msal_cache.go (1)
NewMSALCache(21-39)pkg/perf/perf.go (1)
Track(121-138)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/cloud/azure/env.go (2)
PrepareEnvironment(68-124)PrepareEnvironmentConfig(46-51)
pkg/auth/providers/azure/device_code_ui_test.go (1)
errors/errors.go (1)
ErrAuthenticationFailed(461-461)
pkg/auth/providers/azure/device_code_ui.go (4)
errors/errors.go (1)
ErrAuthenticationFailed(461-461)pkg/logger/log.go (1)
Debug(24-26)pkg/utils/url_utils.go (1)
OpenUrl(13-39)pkg/ui/theme/colors.go (4)
ColorCyan(32-32)ColorGray(30-30)ColorGreen(31-31)ColorBlue(34-34)
pkg/auth/cloud/azure/msal_cache.go (4)
pkg/config/homedir/homedir.go (1)
Dir(36-63)pkg/auth/cloud/azure/constants.go (2)
DirPermissions(6-6)FilePermissions(8-8)pkg/auth/cloud/azure/files.go (1)
AcquireFileLock(49-73)pkg/logger/log.go (1)
Debug(24-26)
pkg/auth/providers/azure/device_code_cache_test.go (2)
pkg/xdg/xdg.go (1)
GetXDGCacheDir(32-34)pkg/auth/cloud/azure/constants.go (1)
FieldAccessToken(29-29)
pkg/auth/cloud/azure/files.go (5)
pkg/logger/log.go (1)
Debug(24-26)pkg/perf/perf.go (1)
Track(121-138)pkg/config/homedir/homedir.go (1)
Dir(36-63)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)errors/errors.go (1)
ErrAuthenticationFailed(461-461)
pkg/auth/providers/azure/device_code_cache.go (6)
pkg/xdg/xdg.go (1)
GetXDGCacheDir(32-34)pkg/logger/log.go (1)
Debug(24-26)pkg/auth/cloud/azure/constants.go (11)
LogFieldKey(46-46)FieldHomeAccountID(24-24)FieldEnvironment(25-25)FieldRealm(26-26)FieldAccessToken(29-29)IntFormat(35-35)FilePermissions(8-8)DirPermissions(6-6)BomMarker(14-14)BomSecondByte(16-16)BomThirdByte(18-18)errors/errors.go (3)
ErrAzureOIDClaimNotFound(104-104)ErrAzureUsernameClaimNotFound(105-105)ErrAzureInvalidJWTFormat(106-106)pkg/auth/cloud/azure/setup.go (1)
UpdateSubscriptionsInProfile(526-579)pkg/auth/cloud/azure/files.go (1)
AcquireFileLock(49-73)
pkg/auth/cloud/azure/setup.go (8)
pkg/auth/types/interfaces.go (1)
ICredentials(254-265)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/cloud/azure/files.go (2)
NewAzureFileManager(83-100)AcquireFileLock(49-73)errors/errors.go (5)
ErrAuthenticationFailed(461-461)ErrInvalidAuthConfig(456-456)ErrAzureOIDClaimNotFound(104-104)ErrAzureUsernameClaimNotFound(105-105)ErrAzureInvalidJWTFormat(106-106)pkg/schema/schema.go (3)
AuthContext(540-550)ConfigAndStacksInfo(592-685)AzureAuthContext(573-590)pkg/auth/cloud/azure/env.go (2)
PrepareEnvironment(68-124)PrepareEnvironmentConfig(46-51)pkg/auth/cloud/azure/constants.go (11)
FieldAccessToken(29-29)FieldHomeAccountID(24-24)FieldEnvironment(25-25)FieldRealm(26-26)DirPermissions(6-6)FilePermissions(8-8)IntFormat(35-35)FieldUser(30-30)BomMarker(14-14)BomSecondByte(16-16)BomThirdByte(18-18)pkg/config/homedir/homedir.go (1)
Dir(36-63)
pkg/auth/providers/azure/device_code_test.go (3)
errors/errors.go (2)
ErrInvalidProviderConfig(460-460)ErrInvalidProviderKind(459-459)pkg/auth/providers/azure/device_code.go (1)
NewDeviceCodeProvider(79-104)pkg/auth/cloud/azure/env.go (1)
PrepareEnvironment(68-124)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Build (linux)
- GitHub Check: Build (windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Review Dependency Licenses
- GitHub Check: autofix
- GitHub Check: Run pre-commit hooks
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (23)
pkg/auth/cloud/azure/msal_cache.go (1)
19-38: MSAL cache creation, permissions, and path handling look solidThe constructor and struct wiring are clean: sensible default path, directory creation with 0o700, and use of 0o600 on the cache file plus locking is appropriate for token material. No issues from my side here.
pkg/auth/providers/azure/device_code_cache.go (4)
45-82: CacheStorage abstraction and default implementation are nicely testableThe
CacheStorageinterface plusdefaultCacheStoragegive you a clean seam for unit tests without touching the real filesystem/XDG cache. The methods are minimal but sufficient (read/write/remove/mkdir/XDG), and the integration withxdg.GetXDGCacheDiris straightforward.
100-181: Device-code token cache behavior is sane and non-fatal
loadCachedTokenandsaveCachedTokenare defensive: they gracefully treat missing/invalid/expired or tenant-mismatched entries as cache misses, and only treat JSON marshal/write failures as fatal. The 5‑minute expiry buffer is a good safety margin. No functional issues here.
211-255: Azure CLI MSAL cache update path looks correct and safely non-fatalThe
updateAzureCLICacheflow (extract user info → load/initialize cache → add tokens → write with locking → update azureProfile) matches the goal of emulatingaz login, while logging and swallowing failures so auth doesn’t hard‑fail on cache/profile issues. This is a good balance.
483-577: Azure profile update + BOM handling are robustThe
updateAzureProfilelogic (create-or-update profile JSON, update subscriptions via shared helper, write with lock and 0o600, strip UTF‑8 BOM on read) is solid and matches the Azure CLI behavior. ThestripBOMhelper is small and well-covered by tests.pkg/auth/providers/azure/device_code_ui_test.go (1)
13-95: Spinner UI tests nicely cover the model lifecycleThese tests exercise the key paths (Init, success/error completion, Ctrl+C cancel, and view output states) in a tight, direct way. They’re aligned with the spinner model implementation and should catch regressions in the device-code UX.
pkg/auth/providers/azure/device_code_test.go (2)
15-168: Provider construction tests cover the key config permutations
TestNewDeviceCodeProviderdoes a good job exercising valid configs (with/without subscription/location/custom client ID) and the main error modes (nil/empty spec, missing tenant, wrong kind). That should keep the constructor behavior honest as the schema evolves.
217-420: Validate/Environment/PrepareEnvironment tests look comprehensiveThe tests around
Validate,Environment, andPrepareEnvironmentcover required-field errors, env var projections, clearing conflicting Azure vars, preserving other cloud creds, and enforcingARM_USE_CLI=true. This lines up well with the sharedazureCloud.PrepareEnvironmentbehavior.pkg/auth/providers/azure/device_code_ui.go (2)
21-71: Spinner-based wait flow correctly bridges MSAL and the TUIThe
waitForAuthWithSpinnerpath neatly decouples MSAL’s blockingAuthenticationResultfrom the Bubble Tea spinner loop and returns either the token/expiry or the spinner’s error (including Ctrl+C). That’s a clean, intuitive flow.
73-148: TTY-aware prompt and browser launch behavior are sensibleThe TTY check, split between a styled dialog and plain-text fallback, plus the
?otc=pre-filled browser URL is a nice UX touch. Errors fromOpenUrlare logged and don’t affect auth, which is the right trade-off.pkg/auth/providers/azure/device_code.go (7)
52-76: Config extraction handles defaults and types safely
extractDeviceCodeConfigsensibly defaultsclient_idto the CLI public client, only overrides when a non-empty string is provided, and ignores non-string values. That keeps provider construction resilient to odd specs.
78-104: NewDeviceCodeProvider validation and wiring look goodThe constructor enforces
config != nil, the expected kind, and a non-emptytenant_id, then threads the parsed spec plus adefaultCacheStorageinto the provider. That’s a solid, explicit setup.
121-146: MSAL client creation with shared cache is correctly configured
createMSALClientuses the sharedNewMSALCache("")for persistent tokens and builds a tenant-scoped authority URL. That should give you the expected “az login”-style cache behavior while keeping tenant boundaries clear.
198-243: Authenticate flow (silent first, then device code, then CLI cache update) is well-structuredThe main
Authenticatemethod follows a clean sequence: create MSAL client, try silent acquisition using tenant-filtered account, fall back to device-code flow when needed, then update the Azure CLI MSAL cache and finally return a strongly-typedAzureCredentials. Errors are consistently wrapped withErrAuthenticationFailed, which will make callers’ error handling simpler.
255-317: Silent token acquisition path is tenant-aware and robust
trySilentTokenAcquisitionfilters accounts by tenant ID, then attempts management, Graph, and KeyVault scopes in turn, logging failures but not treating missing Graph/KeyVault as fatal. This matches the “best effort additional scopes” design and avoids cross-tenant surprises.
319-400: Device-code path and additional token acquisition behave sensibly
acquireTokensViaDeviceCodecorrectly rejects non-interactive environments, runs device-code auth for the management scope, then leverages the authenticated account to silently fetch Graph and KeyVault tokens. Skipping those additional tokens when the account lookup fails is a reasonable fallback.
483-494: Logout and file path display behavior are straightforward
Logoutsimply delegates todeleteCachedTokenwith a debug log, andGetFilesDisplayPathreturns a predictable~/.azure/atmos/<provider>path for user-facing display. Both are clear and consistent with the rest of the Azure file-handling story.pkg/auth/providers/azure/device_code_cache_test.go (1)
20-739: Cache, JWT, profile, and CLI cache tests provide solid coverageThe mocked
CacheStorageplus the suite of tests around token cache load/save/delete, JWT claim extraction, azureProfile updates (including BOM), CLI MSAL cache initialization, and the sandboxedupdateAzureCLICacheintegration give this module very good coverage and confidence. Nicely done.pkg/auth/cloud/azure/files.go (2)
22-73: File locking and error sentinels are well thought out
AcquireFileLockwith a dedicated lock file, timeout/retry, and clearErrFileLockTimeoutsentinel gives you a predictable concurrency model for Azure auth files, and the Err* constants will make higher-level error handling cleaner.
81-244: AzureFileManager correctly handles paths, permissions, and concurrencyThe manager resolves
~/.azure/atmosviahomedir.Dir()when needed, writes credentials with 0o600, uses a per-instance mutex plus file locks, and treats missing credentials as anErrAuthenticationFailedcase while logging and swallowing benign cleanup races. This is exactly the kind of cautious handling you want around token files.pkg/auth/cloud/azure/setup.go (3)
142-179: Clean environment variable handling.The complete replacement approach (line 172) prevents stale env vars from lingering, and the helper
PrepareEnvironmentcentralizes the logic. Well done.
581-654: JWT extraction helpers look solid.The fallback chain for username claims (upn → unique_name → email) handles different Azure token formats. BOM stripping at line 648 addresses Azure CLI's file format quirks. Base64 RawURLEncoding is correct for JWT payloads.
524-579: The review comment is incorrect.
UpdateSubscriptionsInProfileis used externally inpkg/auth/providers/azure/device_code_cache.go:510, so it must remain exported. Unexporting it would break this external caller.Likely an incorrect or invalid review comment.
Validates Azure credentials are not expired before populating auth context. This provides early detection and graceful failure with clear error message, rather than failing downstream operations with less obvious errors. Includes test case for expired credentials scenario. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@coderabbitai full review please |
|
@aknysh: I'll conduct a comprehensive review of the Azure authentication implementation, covering all the new authentication methods, provider integrations, and test coverage. ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pkg/auth/cloud/azure/setup.go (1)
261-383: MSAL cache still lacks refresh tokens, limiting “az login” parity.
updateMSALCacheand its helpers populate only theAccessTokenandAccountsections. Azure CLI’s MSAL cache also persists a corresponding refresh token entry so the CLI (and Terraform viaARM_USE_CLI=true) can silently renew tokens and mint new scopes.Without writing a
RefreshTokensection:
- Long-running Terraform plans/applies will fail once the initial access token expires.
- Any provider that needs a new scope after the initial login cannot rely on MSAL to obtain it.
Consider extending:
AzureCredentials/msalCacheUpdateto carry the refresh token and its expiration.initializeCacheSections/addAccountAndTokens(or a new helper) to create/update theRefreshTokensection with entries keyed consistently with Azure CLI’s MSAL schema.Recommend validating the exact JSON shape against a real
az loginmsal_token_cache.jsonbefore implementing.What sections and key structure does Azure CLI use in msal_token_cache.json to store refresh tokens alongside access tokens?Also applies to: 431-474
🧹 Nitpick comments (6)
pkg/auth/cloud/azure/setup_test.go (5)
20-86: SetupFiles tests look solid; consider tightening the non-Azure case.Nice use of a shared
createTestJWThelper and table-driven tests to cover Azure vs non-Azure credentials. For the"handles non-Azure credentials"case, you might optionally assert that the credentials file does not exist to make the “no-op” behavior explicit and guard against future regressions.
225-416: getComponentLocationOverride and env var tests are comprehensive, with minor expansion possible.The matrix of
getComponentLocationOverridecases (nil stack, bad types, missing identity, wrong type forlocation) is thorough.TestSetEnvironmentVariablescorrectly validates the primary environment keys produced byPrepareEnvironment. If you want to go a step further, you could also assert that unrelated pre-existing env keys are preserved and that conflicting Azure env vars are cleared, but that’s optional given the dedicated tests inenv.go.
418-565: JWT helper tests are well-chosen; you could assert error types as a small improvement.The tests around
extractJWTClaims,extractOIDFromToken,extractUsernameFromToken, andextractUsernameOrFallbackcover both valid and failing cases. As a minor enhancement, you might assert specific error sentinels (e.g.ErrAzureInvalidJWTFormat,ErrAzureOIDClaimNotFound,ErrAzureUsernameClaimNotFound) withassert.ErrorIsto lock in error semantics, but the current coverage is already good.
567-606: stripBOM test duplication could be DRY’d up.These
TestStripBOMcases nicely cover BOM, no BOM, short/partial data. Since an equivalent suite already exists inpkg/auth/providers/azure/device_code_cache_test.go, consider extracting a shared test helper or consolidating into one location to avoid duplicated maintenance.
608-891: MSAL/Azure profile/load cache tests are strong; a couple of optional assertions.The tests for
updateMSALCache,updateAzureProfile, andloadMSALCacheexercise creation, merge/update, BOM handling, and error paths (directory in place of file, invalid JSON, empty cache). You might optionally:
- In
TestUpdateMSALCache, assert that multiple token entries (Management, Graph, KeyVault) are present when provided.- In
TestUpdateAzureProfile, assert thatuserandenvironmentNamefields are set as expected, aligning more tightly with Azure CLI semantics.Not required, but would further guard against subtle behavior drift.
pkg/auth/cloud/azure/setup.go (1)
137-184: Env derivation only updates ComponentEnvSection; reconcile with ComponentEnvList usage.
SetEnvironmentVariablesrebuildsComponentEnvSectionfromPrepareEnvironment, but the comment mentions populatingComponentEnvSection/ComponentEnvListwhile only the map is actually touched. If other parts of Atmos rely onComponentEnvListdirectly, they might miss these Azure values.Either:
- Ensure there is a downstream step that regenerates
ComponentEnvListfromComponentEnvSection, or- Update this function (or the comment) to keep them consistent.
📜 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/cloud/azure/setup.go(1 hunks)pkg/auth/cloud/azure/setup_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📚 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/cloud/azure/setup.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/cloud/azure/setup.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:
pkg/auth/cloud/azure/setup.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/cloud/azure/setup.go
📚 Learning: 2024-12-13T16:48:00.294Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:42-42
Timestamp: 2024-12-13T16:48:00.294Z
Learning: The function `withCacheFileLock` in `pkg/config/cache.go` is currently unused and left for future upgrade.
Applied to files:
pkg/auth/cloud/azure/setup.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:
pkg/auth/cloud/azure/setup.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: XDG Base Directory Specification compliance implementation for atmos toolchain is complete: created toolchain/xdg_cache.go with GetXDGCacheDir() and GetXDGTempCacheDir() functions, updated toolchain/installer.go and cmd/toolchain_clean.go to use these XDG helpers, and changed all cache paths from hardcoded ~/.cache/tools-cache to XDG-compliant ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback).
Applied to files:
pkg/auth/cloud/azure/setup.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/auth/cloud/azure/setup.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: Final XDG Base Directory Specification implementation for atmos toolchain is complete and verified: toolchain/xdg_cache.go provides GetXDGCacheDir() and GetXDGTempCacheDir() functions, all hardcoded ~/.cache/tools-cache paths have been replaced with XDG-compliant paths using ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback), and tests have been updated to expect the new path structure.
Applied to files:
pkg/auth/cloud/azure/setup.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain XDG compliance implementation is complete with GetXDGCacheDir() and GetXDGTempCacheDir() functions in toolchain/xdg_cache.go, updated installer.go and toolchain_clean.go to use these helpers, and changed cache paths from ~/.cache/tools-cache to ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain when XDG_CACHE_HOME is not set).
Applied to files:
pkg/auth/cloud/azure/setup.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
pkg/auth/cloud/azure/setup.go
🧬 Code graph analysis (2)
pkg/auth/cloud/azure/setup.go (7)
pkg/auth/types/interfaces.go (1)
ICredentials(254-265)pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/cloud/azure/files.go (2)
NewAzureFileManager(83-100)AcquireFileLock(49-73)errors/errors.go (5)
ErrAuthenticationFailed(461-461)ErrInvalidAuthConfig(456-456)ErrAzureOIDClaimNotFound(104-104)ErrAzureUsernameClaimNotFound(105-105)ErrAzureInvalidJWTFormat(106-106)pkg/schema/schema.go (3)
AuthContext(540-550)ConfigAndStacksInfo(592-685)AzureAuthContext(573-590)pkg/auth/cloud/azure/env.go (2)
PrepareEnvironment(68-124)PrepareEnvironmentConfig(46-51)pkg/auth/cloud/azure/constants.go (11)
FieldAccessToken(29-29)FieldHomeAccountID(24-24)FieldEnvironment(25-25)FieldRealm(26-26)DirPermissions(6-6)FilePermissions(8-8)IntFormat(35-35)FieldUser(30-30)BomMarker(14-14)BomSecondByte(16-16)BomThirdByte(18-18)
pkg/auth/cloud/azure/setup_test.go (8)
pkg/auth/types/azure_credentials.go (1)
AzureCredentials(16-27)pkg/auth/types/interfaces.go (1)
ICredentials(254-265)pkg/auth/types/aws_credentials.go (1)
AWSCredentials(16-24)pkg/auth/cloud/azure/setup.go (4)
SetupFiles(22-40)SetAuthContextParams(43-50)SetAuthContext(54-111)SetEnvironmentVariables(147-184)pkg/schema/schema.go (3)
AuthContext(540-550)ConfigAndStacksInfo(592-685)AzureAuthContext(573-590)errors/errors.go (2)
ErrInvalidAuthConfig(456-456)ErrAuthenticationFailed(461-461)pkg/auth/cloud/azure/files.go (1)
NewAzureFileManager(83-100)pkg/auth/providers/azure/device_code_cache_test.go (1)
TestStripBOM(882-931)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: autofix
- GitHub Check: Build (windows)
- GitHub Check: Build (linux)
- GitHub Check: Build (macos)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: website-deploy-preview
- GitHub Check: Review Dependency Licenses
- GitHub Check: Summary
🔇 Additional comments (3)
pkg/auth/cloud/azure/setup_test.go (1)
88-223: Good coverage for SetAuthContext, including edge cases.These tests exercise the happy path, component-level location overrides, nil params/auth context, non-Azure credentials, and expired credentials. This gives strong confidence in the auth-context setup logic as implemented in
SetAuthContext.pkg/auth/cloud/azure/setup.go (2)
19-111: Core Azure setup and auth-context wiring look good.
SetupFilesandSetAuthContextcorrectly:
- Ignore non-Azure credentials.
- Centralize path resolution through
NewAzureFileManager.- Validate credential expiry before populating
AuthContext.Azure.- Support component-level location overrides via
getComponentLocationOverride.This matches the intended “AuthContext as single source of truth” design.
586-659: JWT/BOM helpers are straightforward and match the tests.
extractOIDFromToken,extractUsernameFromToken,extractUsernameOrFallback,extractJWTClaims, andstripBOMare simple, defensive, and align with the test cases (standard 3-part JWT, UTF‑8 BOM handling). No changes needed here.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
NOTICE (1)
963-966: Regenerate NOTICE file to resolve pipeline failure.The dependency review check is still failing because the NOTICE file is out of date. Run
./scripts/generate-notice.shlocally, review the output, and commit the regenerated file.#!/bin/bash # Verify NOTICE file is current ./scripts/generate-notice.sh # Check if there are any changes git diff NOTICEAlso applies to: 1213-1213
pkg/auth/cloud/azure/files_test.go (1)
433-474: Concurrent test still performs only sequential operations.The test name and comments indicate concurrent access testing, but the implementation performs sequential writes without goroutines. The past review comment suggested adding actual concurrent operations with
sync.WaitGroupand running with-racedetector, but this hasn't been addressed.To properly test concurrent access, spawn multiple goroutines writing simultaneously:
func TestAzureFileManager_ConcurrentAccess(t *testing.T) { tmpDir := t.TempDir() mgr := &AzureFileManager{ baseDir: tmpDir, } - // Test concurrent writes don't corrupt file. - // This tests the mutex locking behavior. providerName := "concurrent-provider" + numGoroutines := 10 + var wg sync.WaitGroup - // Write initial credentials. - creds1 := &types.AzureCredentials{ - AccessToken: "token-1", - TenantID: "tenant-123", - SubscriptionID: "sub-456", + // Spawn multiple goroutines writing concurrently + for i := 0; i < numGoroutines; i++ { + wg.Add(1) + go func(id int) { + defer wg.Done() + creds := &types.AzureCredentials{ + AccessToken: fmt.Sprintf("token-%d", id), + TenantID: fmt.Sprintf("tenant-%d", id), + SubscriptionID: fmt.Sprintf("sub-%d", id), + } + err := mgr.WriteCredentials(providerName, fmt.Sprintf("identity-%d", id), creds) + require.NoError(t, err) + }(i) } - err := mgr.WriteCredentials(providerName, "identity-1", creds1) - require.NoError(t, err) - - // Verify credentials were written. - loaded, err := mgr.LoadCredentials(providerName) - require.NoError(t, err) - assert.Equal(t, "token-1", loaded.AccessToken) - - // Write again with different credentials. - creds2 := &types.AzureCredentials{ - AccessToken: "token-2", - TenantID: "tenant-789", - SubscriptionID: "sub-012", - } - - err = mgr.WriteCredentials(providerName, "identity-2", creds2) - require.NoError(t, err) + wg.Wait() - // Verify new credentials overwrote old ones. + // Verify file is valid JSON (not corrupted by concurrent writes) loaded, err := mgr.LoadCredentials(providerName) require.NoError(t, err) - assert.Equal(t, "token-2", loaded.AccessToken) - assert.Equal(t, "tenant-789", loaded.TenantID) + assert.NotEmpty(t, loaded.AccessToken) }Run tests with:
go test -race ./pkg/auth/cloud/azure/...pkg/auth/providers/azure/device_code_test.go (1)
331-420: Authenticate path remains untested; consider at least minimal coverageThere’s excellent coverage for configuration, env prep, and caching, but
deviceCodeProvider.Authenticateitself is still untested. I know this was discussed earlier and the interactive MSAL flow makes full end‑to‑end tests awkward, but even a very small unit test (or two) that hitsAuthenticatewith a mocked MSAL client and verifies:
- basic success wiring (e.g., returns non‑empty credentials struct when the mock issues tokens), and
- a representative error path (e.g., MSAL error mapped to
ErrAuthenticationFailed),would help protect the most critical method from regressions. If that’s too heavy for this PR, a brief note in the implementation explaining why Authenticate is only covered by higher-level/integration tests would at least document the trade‑off.
Also applies to: 422-454, 484-576
pkg/auth/types/azure_credentials.go (1)
56-64: Align WhoamiInfo.Account with tenant ID, not subscription ID.Right now
BuildWhoamiInfosetsinfo.AccountfromSubscriptionID, but elsewhere (e.g.,Validate) you treat the tenant as the account. The shared whoami contract for Azure expectsAccountto be the tenant ID so UIs and logs don’t show the subscription where a tenant should appear, especially in multi-subscription setups.You can keep Region/Expiration logic as-is and just map Account from
TenantID:func (c *AzureCredentials) BuildWhoamiInfo(info *WhoamiInfo) { info.Region = c.Location - if c.SubscriptionID != "" { - info.Account = c.SubscriptionID - } + if c.TenantID != "" { + info.Account = c.TenantID + } if t, _ := c.GetExpiration(); t != nil { info.Expiration = t } }pkg/auth/providers/azure/device_code_cache.go (1)
25-44: Extend device-code cache schema to persist KeyVault tokens as well.Right now the device-code cache only persists the management and Graph tokens:
deviceCodeTokenCacheholdsGraphAPIToken/GraphAPIExpiresAtbut no KeyVault fields.cachedTokenResultmirrors that (no KeyVault fields).saveCachedTokentakes only management + Graph tokens.loadCachedTokencan therefore never surface a KeyVault token on a cache hit.Meanwhile
tokenCacheUpdateandaddOptionalCLITokensare wired to add a KeyVault MSAL entry whenKeyVaultTokenis present. That means any subsequent non-interactive run that reuses the cached token path will always hit the “No KeyVault API token available, KeyVault operations may not work” branch, even if the initial login had a valid KeyVault token.To keep the CLI/MSAL cache complete on the cached path, you’ll want to carry KeyVault through the same cache:
- Add KeyVault fields to the cache structs:
type deviceCodeTokenCache struct { AccessToken string `json:"accessToken"` TokenType string `json:"tokenType"` ExpiresAt time.Time `json:"expiresAt"` TenantID string `json:"tenantId"` SubscriptionID string `json:"subscriptionId,omitempty"` Location string `json:"location,omitempty"` GraphAPIToken string `json:"graphApiToken,omitempty"` GraphAPIExpiresAt time.Time `json:"graphApiExpiresAt,omitempty"`
KeyVaultToken string `json:"keyVaultToken,omitempty"`KeyVaultExpiresAt time.Time `json:"keyVaultExpiresAt,omitempty"`}
type cachedTokenResult struct {
AccessToken string
ExpiresAt time.Time
GraphAPIToken string
GraphAPIExpiresAt time.TimeKeyVaultToken stringKeyVaultExpiresAt time.Time}
- Thread KeyVault through save/load:
- func (p *deviceCodeProvider) saveCachedToken(accessToken, tokenType string, expiresAt time.Time, graphToken string, graphExpiresAt time.Time) error {
- func (p *deviceCodeProvider) saveCachedToken(accessToken, tokenType string, expiresAt time.Time, graphToken string, graphExpiresAt time.Time, keyVaultToken string, keyVaultExpiresAt time.Time) error {
...
cache := deviceCodeTokenCache{
AccessToken: accessToken,
TokenType: tokenType,
ExpiresAt: expiresAt,
TenantID: p.tenantID,
SubscriptionID: p.subscriptionID,
Location: p.location,
GraphAPIToken: graphToken,
GraphAPIExpiresAt: graphExpiresAt,KeyVaultToken: keyVaultToken,KeyVaultExpiresAt: keyVaultExpiresAt, }```diff return cachedTokenResult{ AccessToken: cache.AccessToken, ExpiresAt: cache.ExpiresAt, GraphAPIToken: cache.GraphAPIToken, GraphAPIExpiresAt: cache.GraphAPIExpiresAt,KeyVaultToken: cache.KeyVaultToken,KeyVaultExpiresAt: cache.KeyVaultExpiresAt,
- Then, when you build the
tokenCacheUpdatefrom a cache hit in the device-code provider, populateKeyVaultToken/KeyVaultExpiresAtfromcachedTokenResultsoaddOptionalCLITokenscan consistently add the KeyVault entry on both fresh and cached flows.That keeps azurerm KeyVault operations working even when device-code auth is satisfied from the cache.
Also applies to: 100-147, 150-181, 201-209, 335-352
pkg/auth/cloud/azure/setup.go (1)
261-383: MSAL cache still omits refresh tokens, limitingaz‑compatibility
updateMSALCacheinitializesAccessTokenandAccountsections and adds management/Graph/KeyVault access tokens, but there’s still noRefreshTokensection being populated. Without refresh tokens in the MSAL cache, Azure CLI semantics (silent token renewal, new scopes) won’t fully matchaz login, and long‑running Terraform runs may hit expirations once the initial access token window closes.Consider extending the cache update to also persist a matching refresh‑token entry (under the standard MSAL
RefreshTokenkey) using the same identifiers (homeAccountID,environment,clientID,realm) so Azure tooling can renew tokens asazdoes today.
🧹 Nitpick comments (13)
pkg/auth/providers/azure/device_code_ui.go (1)
21-71: Consider wiring Ctrl+C into context cancellation and tightening a couple of edgesThe spinner flow is generally solid, but a couple of refinements would make it more robust:
- When the user hits Ctrl+C,
spinnerModel.UpdatesetsauthErrand quits, but the backgrounddeviceCode.AuthenticationResult(authCtx)call is left running. If callers pass a cancelable context, consider threading acontext.CancelFuncintowaitForAuthWithSpinner/spinnerModelso the Ctrl+C path also cancels the auth call instead of letting it run until timeout.finalModel.(*spinnerModel)is safe given current usage, but it will panic if Bubble Tea ever returns a different model type. A defensive assert with anokcheck and a wrappedErrAuthenticationFailedwould fail more gracefully.- When building the verification URL with
?otc=, ifverificationURLever includes its own query string, you’ll end up with a duplicate?. Switching to?vs&based onstrings.Contains(verificationURL, "?")would harden this a bit.None of these are blockers, but they’ll make the UX and failure modes cleaner if you touch this again.
Also applies to: 186-200
cmd/auth_console_test.go (1)
465-505: Expand console provider tests to cover all Azure provider kindsSwitching the Azure OIDC case to expect a non-nil provider matches the new
getConsoleProviderbehavior.You might also want to add analogous cases for
ProviderKindAzureCLIandProviderKindAzureDeviceCodeso regressions in those paths are caught by tests, similar to how AWS IAM Identity Center and SAML are covered.The fabricated
"Azure console access not yet implemented"error in the sentinel test is fine to keep as a genericErrProviderNotSupportedcheck even though Azure now supports console access.Also applies to: 312-318
internal/exec/terraform_generate_backends_test.go (1)
475-509: Tests correctly updated for the new generateComponentBackendConfig signatureUsing
nilfor the newAuthContextparameter in these tests keeps them focused on backend JSON structure and nested maps, while still exercising the updated function signature.If we later add any auth‑context‑dependent behavior in backend generation (e.g., Azure-specific tweaks), it’d be worth adding a dedicated test with a non‑nil
AuthContext, but what you have here is solid for the current logic.Also applies to: 673-781
pkg/auth/cloud/azure/msal_cache_test.go (2)
21-31: Avoid hard‑coded/tmppaths in testsUsing
/tmp/...as a cache path in tests can break on non‑Unix platforms and in constrained environments. Since other tests already uset.TempDir(), it’d be more portable to derive the custom cache paths fromt.TempDir()in both the “custom path” case andTestMSALCache_GetCachePath.You can compute the path inside each subtest, e.g.
cachePath := filepath.Join(t.TempDir(), "msal_cache.json"), instead of hard‑coding/tmp/....Also applies to: 155-162
53-118: Consider adding negative-path coverage for Replace/ExportThe Replace/Export tests cover non‑existent vs existing cache, successful unmarshalling/marshalling, and cancellation, which is solid. What’s missing is behavior when
UnmarshalorMarshalreturns an error (e.g. corrupt file, serialization failure). A couple of extra tests with mocks that return an error would lock in the expected error propagation behavior ofReplaceandExport.pkg/auth/types/azure_credentials_test.go (1)
87-104: Be mindful of external dependencies inValidatetestsThe
Validatetests intentionally expect failures with invalid/no subscription ID, which is fine. IfAzureCredentials.Validateperforms real Azure SDK or HTTP calls, though, these tests will depend on network availability and Azure service behavior, which can introduce flakiness and slowdowns.If feasible, consider structuring
Validateso its external dependencies (HTTP client, Azure SDK client, etc.) can be injected/mocked in tests, and keep these tests focused on error classification and local pre‑validation. Otherwise, a brief comment in the implementation clarifying thatValidateis effectively an integration check would help set expectations.Also applies to: 106-139
pkg/auth/cloud/azure/env.go (1)
14-43: Environment clearing behavior vs user-supplied Azure configThe defensive copy and targeted clearing of Azure/ARM credential variables look good, and the explicit choice to preserve AWS/GCP env vars matches the multi-cloud use case.
One nuance:
problematicAzureEnvVarsalso includesAZURE_CONFIG_DIRandAZURE_REGION. When those are set but thePrepareEnvironmentConfiglacks aLocation, the resulting env will drop those user-supplied settings entirely. That’s probably fine given the “mirror az login + use default~/.azure” goal, but if you ever support custom Azure config directories or regions as part of Atmos-managed auth, this list may need to be revisited so we don’t unintentionally discard valid user intent.Also applies to: 83-109
pkg/auth/providers/azure/device_code.go (1)
52-76: Consider validating spec field types instead of silently ignoring bad types
extractDeviceCodeConfigquietly ignores non‑stringtenant_id,subscription_id,location, andclient_idvalues. That means a typo in YAML (e.g.,tenant_id: 12345) degrades into “tenant_id is required” instead of pointing at the real problem.If you want stricter, clearer feedback for users, you could treat wrong types as config errors:
func extractDeviceCodeConfig(spec map[string]interface{}) deviceCodeConfig { @@ - if tid, ok := spec["tenant_id"].(string); ok { - config.TenantID = tid - } + if tid, ok := spec["tenant_id"]; ok { + s, ok := tid.(string) + if !ok { + // optionally: track an error return instead of silently ignoring + } else { + config.TenantID = s + } + }Same pattern for
subscription_id,location, andclient_id. Not mandatory, but it would tighten UX around misconfigured specs.pkg/auth/cloud/azure/constants.go (1)
3-47: Constants look good; consider scoping if usage stays narrowThe permission, BOM, MSAL, and log-field constants are clear and centralize a lot of magic strings/values. If some of these (
IntFormat,StrconvDecimal,Int64BitSizeespecially) stay internal to a single file, you might later tighten them to unexported to keep the public surface lean, but they’re fine as-is for this PR.pkg/auth/cloud/azure/msal_cache.go (1)
41-78: MSAL cache I/O looks solid; consider tighter context handling as a polish.Replace/Export behavior and locking look good, especially the “start fresh on corruption” path. If you ever care about cancellation under lock contention, you could re-check
ctx.Err()after acquiring the lock and before disk I/O so long waits onAcquireFileLockdon’t ignore caller timeouts. Not urgent, just a small ergonomics win.Also applies to: 80-112
pkg/auth/providers/azure/device_code_cache_test.go (1)
589-606: Tighten test setup by asserting on filesystem operations.In the
setupProfilehelpers you callos.MkdirAllandos.WriteFilewithout checking errors. It’s unlikely to fail undert.TempDir(), but if it ever does you’ll get confusing downstream JSON/IO errors instead of a clear failure at setup time. Wrapping those calls withrequire.NoErrorwould make these tests more self-diagnosing.Example:
-azureDir := filepath.Join(home, ".azure") -os.MkdirAll(azureDir, 0o700) -os.WriteFile(filepath.Join(azureDir, "azureProfile.json"), data, 0o600) +azureDir := filepath.Join(home, ".azure") +require.NoError(t, os.MkdirAll(azureDir, 0o700)) +require.NoError(t, os.WriteFile(filepath.Join(azureDir, "azureProfile.json"), data, 0o600))Also applies to: 622-638
pkg/auth/providers/azure/cli.go (1)
144-171: Honor context cancellation explicitly in executeAzCommand.
exec.CommandContextwill killazon context cancellation, but in the error path you only look atoutputStr. If the context is canceled (timeout, SIGINT), callers will see a generic “failed to get Azure CLI access token” instead of a clearerctx.Err().You could special-case this before inspecting CLI output:
output, err := cmd.CombinedOutput() if err != nil { + if ctx.Err() != nil { + // Surface cancellation to the caller explicitly. + return nil, ctx.Err() + } outputStr := strings.TrimSpace(string(output)) if strings.Contains(outputStr, "az login") { return nil, fmt.Errorf("%w: Azure CLI not logged in. Run 'az login' first: %s", errUtils.ErrAuthenticationFailed, outputStr) } return nil, fmt.Errorf("%w: failed to get Azure CLI access token: %s", errUtils.ErrAuthenticationFailed, outputStr) }pkg/auth/cloud/azure/console.go (1)
170-187: Align passthrough URL behavior with comment (restrict tohttpsor update docs)The comment says “Full URL starting with
https://→ Pass through unchanged”, but the implementation passes through any scheme (parsedURL.Scheme != ""). That’s a small behavioral/documentation mismatch and could allow non-HTTPS schemes if a caller accidentally supplies them.If you want to strictly honor the comment, consider tightening the check:
- // If already a full URL, pass through unchanged. - if parsedURL, err := url.Parse(dest); err == nil && parsedURL.Scheme != "" { - return dest, nil - } + // If already a full HTTPS URL, pass through unchanged. + if parsedURL, err := url.Parse(dest); err == nil && parsedURL.Scheme == "https" { + return dest, nil + }Alternatively, update the comment to say “absolute URL” if the broader behavior is intentional.
📜 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 (43)
NOTICE(2 hunks)cmd/auth_console.go(2 hunks)cmd/auth_console_test.go(1 hunks)errors/errors.go(1 hunks)go.mod(2 hunks)internal/exec/terraform_generate_backend.go(1 hunks)internal/exec/terraform_generate_backends.go(1 hunks)internal/exec/terraform_generate_backends_test.go(6 hunks)internal/exec/terraform_output_utils.go(2 hunks)internal/exec/terraform_utils.go(2 hunks)internal/exec/utils.go(2 hunks)internal/exec/utils_test.go(2 hunks)pkg/auth/cloud/azure/console.go(1 hunks)pkg/auth/cloud/azure/console_test.go(1 hunks)pkg/auth/cloud/azure/constants.go(1 hunks)pkg/auth/cloud/azure/env.go(1 hunks)pkg/auth/cloud/azure/env_test.go(1 hunks)pkg/auth/cloud/azure/files.go(1 hunks)pkg/auth/cloud/azure/files_test.go(1 hunks)pkg/auth/cloud/azure/msal_cache.go(1 hunks)pkg/auth/cloud/azure/msal_cache_test.go(1 hunks)pkg/auth/cloud/azure/setup.go(1 hunks)pkg/auth/cloud/azure/setup_test.go(1 hunks)pkg/auth/factory/factory.go(3 hunks)pkg/auth/identities/azure/subscription.go(1 hunks)pkg/auth/identities/azure/subscription_test.go(1 hunks)pkg/auth/providers/azure/cli.go(1 hunks)pkg/auth/providers/azure/cli_test.go(1 hunks)pkg/auth/providers/azure/device_code.go(1 hunks)pkg/auth/providers/azure/device_code_cache.go(1 hunks)pkg/auth/providers/azure/device_code_cache_test.go(1 hunks)pkg/auth/providers/azure/device_code_test.go(1 hunks)pkg/auth/providers/azure/device_code_ui.go(1 hunks)pkg/auth/providers/azure/device_code_ui_test.go(1 hunks)pkg/auth/types/aws_credentials_test.go(1 hunks)pkg/auth/types/azure_credentials.go(1 hunks)pkg/auth/types/azure_credentials_test.go(1 hunks)pkg/auth/types/constants.go(1 hunks)pkg/schema/schema.go(2 hunks)tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden(1 hunks)website/blog/2025-11-07-azure-authentication-support.mdx(1 hunks)website/docs/cli/commands/auth/auth-login.mdx(1 hunks)website/docs/cli/commands/auth/tutorials/azure-authentication.mdx(1 hunks)
🧰 Additional context used
🧠 Learnings (55)
📚 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:
website/docs/cli/commands/auth/auth-login.mdxpkg/auth/providers/azure/device_code_test.gowebsite/blog/2025-11-07-azure-authentication-support.mdx
📚 Learning: 2025-09-10T17:34:52.568Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/providers/github/oidc.go:96-100
Timestamp: 2025-09-10T17:34:52.568Z
Learning: The ATMOS_ environment variable binding guideline applies to Atmos configuration variables, not external service-required environment variables like GitHub Actions OIDC variables (GITHUB_ACTIONS, ACTIONS_ID_TOKEN_*) which must use their standard names.
Applied to files:
website/docs/cli/commands/auth/auth-login.mdx
📚 Learning: 2025-11-10T20:03:56.875Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1775
File: pkg/auth/providers/aws/sso_provisioning.go:40-79
Timestamp: 2025-11-10T20:03:56.875Z
Learning: In the Atmos AWS SSO provider (pkg/auth/providers/aws/sso_provisioning.go), the OAuth access token from the AWS SSO device flow is intentionally stored in the `AccessKeyID` field of `AWSCredentials` during authentication. This token is then extracted and used for ListAccounts and ListAccountRoles API calls during identity provisioning. This design reuses the existing `AWSCredentials` type for token transport rather than creating a separate credential type.
Applied to files:
website/docs/cli/commands/auth/auth-login.mdxpkg/schema/schema.gopkg/auth/providers/azure/device_code.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
pkg/auth/cloud/azure/env.gocmd/auth_console.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-08-29T20:57:35.423Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1433
File: cmd/theme_list.go:33-36
Timestamp: 2025-08-29T20:57:35.423Z
Learning: In the Atmos codebase, avoid using viper.SetEnvPrefix("ATMOS") with viper.AutomaticEnv() because canonical environment variable names are not exclusive to Atmos and could cause conflicts. Instead, use selective environment variable binding through the setEnv function in pkg/config/load.go with bindEnv(v, "config.key", "ENV_VAR_NAME") for specific environment variables.
Applied to files:
pkg/auth/cloud/azure/env.go
📚 Learning: 2025-11-11T03:47:59.576Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.
Applied to files:
pkg/auth/cloud/azure/env.gopkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/env_test.go
📚 Learning: 2025-11-11T03:47:45.878Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.
Applied to files:
pkg/auth/cloud/azure/env.gopkg/auth/providers/azure/device_code_test.gopkg/auth/providers/azure/device_code_cache_test.gopkg/auth/providers/azure/cli_test.gopkg/auth/cloud/azure/env_test.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:
pkg/auth/cloud/azure/env.gointernal/exec/terraform_utils.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
pkg/auth/cloud/azure/env.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/factory/factory.gopkg/schema/schema.gocmd/auth_console.gopkg/auth/identities/azure/subscription.gopkg/auth/providers/azure/cli.gopkg/auth/cloud/azure/setup.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/factory/factory.gocmd/auth_console.gogo.mod
📚 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:
pkg/auth/factory/factory.gocmd/auth_console.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/types/aws_credentials_test.gopkg/auth/types/azure_credentials_test.gopkg/auth/identities/azure/subscription_test.gopkg/auth/cloud/azure/console.gopkg/auth/types/azure_credentials.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-11-01T20:24:29.557Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1714
File: NOTICE:0-0
Timestamp: 2025-11-01T20:24:29.557Z
Learning: In the cloudposse/atmos repository, the NOTICE file is programmatically generated and should not be manually edited. Issues with dependency license URLs in NOTICE will be resolved when upstream package metadata is corrected.
Applied to files:
NOTICE
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
internal/exec/terraform_generate_backends_test.gointernal/exec/terraform_generate_backend.gointernal/exec/terraform_output_utils.gointernal/exec/terraform_utils.gointernal/exec/terraform_generate_backends.gointernal/exec/utils_test.gointernal/exec/utils.go
📚 Learning: 2024-11-12T03:16:02.910Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
internal/exec/terraform_generate_backends_test.gointernal/exec/terraform_generate_backend.gointernal/exec/utils_test.gointernal/exec/utils.go
📚 Learning: 2025-03-25T12:24:36.177Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:263-264
Timestamp: 2025-03-25T12:24:36.177Z
Learning: Tests for the default Bitbucket username fallback to "x-token-auth" will be added during a future refactoring phase rather than in this PR.
Applied to files:
pkg/auth/providers/azure/device_code_test.go
📚 Learning: 2024-12-03T03:52:02.524Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:144-145
Timestamp: 2024-12-03T03:52:02.524Z
Learning: Avoid adding context timeouts to Terraform commands in `execTerraformOutput` because their execution time can vary from seconds to hours, and making it configurable would require redesigning the command interface.
Applied to files:
internal/exec/terraform_output_utils.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
internal/exec/terraform_output_utils.gointernal/exec/terraform_utils.gopkg/auth/providers/azure/device_code.gointernal/exec/utils_test.go
📚 Learning: 2024-12-13T16:51:37.868Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:69-97
Timestamp: 2024-12-13T16:51:37.868Z
Learning: In `pkg/config/cache.go`, the function `SaveCache2` is currently unused because it does not work properly on Windows and will be addressed later.
Applied to files:
pkg/auth/cloud/azure/msal_cache.gopkg/auth/providers/azure/device_code.go
📚 Learning: 2024-12-13T16:48:00.294Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:42-42
Timestamp: 2024-12-13T16:48:00.294Z
Learning: The function `withCacheFileLock` in `pkg/config/cache.go` is currently unused and left for future upgrade.
Applied to files:
pkg/auth/cloud/azure/msal_cache.gopkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/files_test.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
cmd/auth_console.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
cmd/auth_console.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/auth_console.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: ErrWrappingFormat is correctly defined as "%w: %w" in the errors package and is used throughout the codebase to wrap two error types together. The usage fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) is the correct pattern when both arguments are error types.
Applied to files:
cmd/auth_console.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/auth_console.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: The user confirmed that the errors package has an error string wrapping format, contradicting the previous learning about ErrWrappingFormat being invalid. The current usage of fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) appears to be the correct pattern.
Applied to files:
cmd/auth_console.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: Final XDG Base Directory Specification implementation for atmos toolchain is complete and verified: toolchain/xdg_cache.go provides GetXDGCacheDir() and GetXDGTempCacheDir() functions, all hardcoded ~/.cache/tools-cache paths have been replaced with XDG-compliant paths using ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback), and tests have been updated to expect the new path structure.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/msal_cache_test.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: XDG Base Directory Specification compliance implementation for atmos toolchain is complete: created toolchain/xdg_cache.go with GetXDGCacheDir() and GetXDGTempCacheDir() functions, updated toolchain/installer.go and cmd/toolchain_clean.go to use these XDG helpers, and changed all cache paths from hardcoded ~/.cache/tools-cache to XDG-compliant ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain fallback).
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/msal_cache_test.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain XDG compliance implementation is complete with GetXDGCacheDir() and GetXDGTempCacheDir() functions in toolchain/xdg_cache.go, updated installer.go and toolchain_clean.go to use these helpers, and changed cache paths from ~/.cache/tools-cache to ${XDG_CACHE_HOME}/atmos-toolchain (or ~/.cache/atmos-toolchain when XDG_CACHE_HOME is not set).
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/msal_cache_test.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2024-12-13T15:33:34.159Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: pkg/config/cache.go:17-31
Timestamp: 2024-12-13T15:33:34.159Z
Learning: In `pkg/config/cache.go`, when `XDG_CACHE_HOME` is not set, falling back to `.` (current directory) is acceptable and aligns with the requirement to primarily use `XDG_CACHE_HOME` for the cache directory.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
pkg/auth/providers/azure/device_code_cache_test.gopkg/auth/cloud/azure/setup.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_utils.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
internal/exec/terraform_utils.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_utils.gointernal/exec/utils_test.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_backends.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_backends.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
internal/exec/terraform_generate_backends.gointernal/exec/utils_test.gointernal/exec/utils.go
📚 Learning: 2024-11-30T22:07:08.610Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/yaml_func_terraform_output.go:35-40
Timestamp: 2024-11-30T22:07:08.610Z
Learning: In the Go function `processTagTerraformOutput` in `internal/exec/yaml_func_terraform_output.go`, parameters cannot contain spaces. The code splits the input by spaces, and if the parameters contain spaces, `len(parts) != 3` will fail and show an error to the user.
Applied to files:
internal/exec/terraform_generate_backends.go
📚 Learning: 2025-01-17T00:21:32.987Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:3-3
Timestamp: 2025-01-17T00:21:32.987Z
Learning: The project uses Go version 1.23.0 which has been confirmed by the maintainer to be working in production for months. Do not flag this as an invalid Go version.
Applied to files:
go.mod
📚 Learning: 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-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:
internal/exec/utils_test.go
📚 Learning: 2024-10-31T07:09:31.983Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 737
File: internal/exec/vendor_utils.go:181-182
Timestamp: 2024-10-31T07:09:31.983Z
Learning: In `internal/exec/vendor_utils.go`, the variables `mergedSources` and `mergedImports` are declared and used later in the code. Do not suggest removing them as unused variables.
Applied to files:
internal/exec/utils_test.go
📚 Learning: 2025-02-19T05:50:35.853Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden:0-0
Timestamp: 2025-02-19T05:50:35.853Z
Learning: Backtick formatting should only be applied to flag descriptions in Go source files, not in golden test files (test snapshots) as they are meant to capture the raw command output.
Applied to files:
pkg/auth/cloud/azure/msal_cache_test.go
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
pkg/auth/cloud/azure/msal_cache_test.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/cloud/azure/setup.go
📚 Learning: 2024-12-25T20:28:47.526Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 887
File: internal/exec/stack_processor_utils.go:380-380
Timestamp: 2024-12-25T20:28:47.526Z
Learning: Windows path handling often requires `filepath.Join` to ensure correct separators and comparisons. Insufficient tests can break cross-platform compatibility, so migrating from `path.Join` to `filepath.Join` needs thorough testing on Windows before merging.
Applied to files:
pkg/auth/cloud/azure/files_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.
Applied to files:
pkg/auth/cloud/azure/env_test.go
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Applied to files:
tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.goldenwebsite/blog/2025-11-07-azure-authentication-support.mdx
📚 Learning: 2025-02-14T23:12:38.030Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1061
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:8-8
Timestamp: 2025-02-14T23:12:38.030Z
Learning: Test snapshots in the Atmos project, particularly for dry run scenarios, may be updated during the development process, and temporary inconsistencies in their content should not be flagged as issues.
Applied to files:
tests/snapshots/TestCLICommands_atmos_auth_whoami_without_authentication.stderr.golden
📚 Learning: 2025-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:
website/blog/2025-11-07-azure-authentication-support.mdx
📚 Learning: 2025-01-17T00:18:57.769Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
Applied to files:
website/blog/2025-11-07-azure-authentication-support.mdx
📚 Learning: 2025-02-18T13:13:11.497Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden:59-64
Timestamp: 2025-02-18T13:13:11.497Z
Learning: For Atmos CLI help text, angle brackets in command examples and flag descriptions should be escaped using HTML entities (e.g., `<component>`) rather than converted to backticks or other markdown formatting.
Applied to files:
website/blog/2025-11-07-azure-authentication-support.mdx
📚 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:
pkg/auth/cloud/azure/setup.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:
pkg/auth/cloud/azure/setup.go
🪛 GitHub Actions: Dependency Review
NOTICE
[error] 1-1: NOTICE file is out of date. Run './scripts/generate-notice.sh' locally and commit the changes.
🪛 LanguageTool
website/docs/cli/commands/auth/tutorials/azure-authentication.mdx
[typographical] ~86-~86: Consider using a typographic opening quote here.
Context: ... 1. Atmos displays a device code (e.g., "WDDD-HRQV") 2. Browser opens to Microsof...
(EN_QUOTES)
[typographical] ~86-~86: Consider using a typographic close quote here.
Context: ...displays a device code (e.g., "WDDD-HRQV") 2. Browser opens to Microsoft's device...
(EN_QUOTES)
[grammar] ~293-~293: Ensure spelling is correct
Context: ...rovides KeyVault token scope } ``` ### azuread Provider The azuread provider automati...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~471-~471: You have used the passive voice repeatedly in nearby sentences. To make your writing clearer and easier to read, consider using active voice.
Context: ... provider compatibility - Both storages are secured by your operating system ### Least Pri...
(REP_PASSIVE_VOICE)
[typographical] ~489-~489: Consider using typographic quotation marks here.
Context: ...→ Monitoring → Sign-in logs - Filter by "Application" to see Atmos authentications ## Migrat...
(EN_QUOTES)
⏰ 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: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Summary
Add os.MkdirAll to updateAzureProfile to prevent failures on first-run systems where Azure CLI hasn't run yet. Both os.WriteFile and AcquireFileLock require the parent directory to exist. This mirrors the existing pattern in writeMSALCacheToFile (lines 405-409) which already creates the .azure directory before writing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…vent panic Add nil check to WriteCredentials before dereferencing the creds parameter. This prevents panics when a typed-nil *types.AzureCredentials is passed in, which could occur in edge cases. The nil check is placed after perf.Track and before acquiring the mutex lock to fail fast without holding resources. Includes test case to verify the nil credentials error handling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ode flow Fixed compilation error where isTTY() was called but not defined. The function isInteractive() already exists and correctly uses term.IsTTYSupportForStderr() to check if we're in an interactive terminal. Using isInteractive() is appropriate here since: - We're checking if we can display the spinner on stderr - The function already encapsulates the TTY detection logic - Consistent with the rest of the device code flow 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…event panic Add nil check after type assertion to handle typed-nil *AzureCredentials. Without this check, the type assertion could succeed (ok=true) but azureCreds could still be nil, causing a panic when calling IsExpired() or accessing fields. The fix treats typed-nil as "no Azure credentials" and returns early, preserving the existing no-op behavior while preventing crashes. Includes test case to verify typed-nil credentials are handled correctly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…te_WithExpiration Updated misleading test comments to accurately describe what the test verifies. The test checks that Validate handles credentials with an Expiration field without panicking, not that "expiration is returned" (which isn't asserted). Changes: - Function comment: clarifies this tests handling of Expiration field - Inline comment: explains this ensures code path doesn't panic This improves test documentation clarity for future maintainers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Changelog Entry RequiredThis PR is labeled Action needed: Add a new blog post in Example filename: Alternatively: If this change doesn't require a changelog entry, remove the |
|
These changes were released in v1.199.0-rc.1. |
Summary
This PR adds comprehensive native Azure authentication support to Atmos, enabling seamless authentication to Azure with full Terraform provider compatibility.
Features
Three Authentication Methods
Full Terraform Provider Support
Works seamlessly with all Azure Terraform providers out of the box:
Identical to
az loginBehavior~/.azure/msal_token_cache.json(Azure CLI MSAL cache)~/.azure/azureProfile.jsonwith subscription configurationARM_USE_CLI=truefor Terraform providershttps://management.azure.com/.default- Azure Resource Managerhttps://graph.microsoft.com/.default- Azure AD operationshttps://vault.azure.net/.default- Azure KeyVault operationsMulti-Subscription & Multi-Region Support
Easy switching between different Azure subscriptions, regions, and environments:
Quick Start
Configuration
Usage
Implementation Details
New Packages
pkg/auth/providers/azure/
device_code.go- Device code flow authentication with interactive browser flowoidc.go- OIDC workload identity federation for CI/CDservice_principal.go- Client credentials authenticationcli.go- Azure CLI compatibility utilitiesdevice_code_cache.go- Token caching and MSAL cache managementpkg/auth/identities/azure/
subscription.go- Azure subscription identity with location supportpkg/auth/cloud/azure/
setup.go- MSAL cache and Azure profile file managementenv.go- Environment variable configuration for Terraformfiles.go- Credential file operations with proper lockingconsole.go- Azure Portal URL generationpkg/auth/types/
azure_credentials.go- Azure credential type implementationArchitecture
Follows Atmos architectural patterns:
Complete Token Support
Atmos provides all three Azure token scopes (matching
az loginexactly):Management Token (
https://management.azure.com/.default)Graph API Token (
https://graph.microsoft.com/.default)KeyVault Token (
https://vault.azure.net/.default)This comprehensive token support ensures all Terraform resources work correctly, including KeyVault certificate contacts, secret management, and AD group operations.
CI/CD Integration Examples
GitHub Actions with OIDC
Service Principal Authentication
Testing
Unit Test Coverage
Added comprehensive test coverage (3,401 lines of test code):
pkg/auth/cloud/azure/
files_test.go- File manager, locking, permissions (112 lines)setup_test.go- MSAL cache updates, JWT extraction, profile management (336 lines)pkg/auth/providers/azure/
device_code_test.go- Device code provider, validation, spinner UI (302 lines)device_code_cache_test.go- Token caching, expiration, MSAL updates (286 lines)cli_test.go- CLI provider validation and environment prep (179 lines)oidc_test.go- OIDC provider with workload identity federationservice_principal_test.go- Service principal authenticationpkg/auth/identities/azure/
subscription_test.go- Subscription identity, location overrides (141 lines)pkg/auth/types/
azure_credentials_test.go- Credential type, expiration, validationCoverage Results
Coverage gap is primarily in Azure SDK integration code (device code authentication flow, token acquisition) which follows the same pattern as AWS implementation (no SDK mocking).
Manual Testing
Verified with:
Documentation
Comprehensive Tutorial
Created detailed Azure authentication guide:
website/docs/cli/commands/auth/tutorials/azure-authentication.mdx(689 lines)Updated Command Documentation
website/docs/cli/commands/auth/auth-login.mdxwith Azure examplesFeature Announcement Blog Post
website/blog/2025-11-07-azure-authentication-support.mdx(447 lines)az loginMigration from az login
Atmos is a drop-in replacement for
az login:Before:
After:
Both write to the same Azure CLI files (
~/.azure/msal_token_cache.jsonand~/.azure/azureProfile.json), so existing Terraform code works without any changes.Security Features
Files Changed
New Implementation Files (27 files)
Core Azure Auth
pkg/auth/types/azure_credentials.go- Azure credential typepkg/auth/cloud/azure/*.go- Azure cloud utilities (5 files)pkg/auth/providers/azure/*.go- Azure providers (5 files)pkg/auth/identities/azure/*.go- Azure identities (1 file)Tests
pkg/auth/types/azure_credentials_test.gopkg/auth/cloud/azure/*_test.go(2 files)pkg/auth/providers/azure/*_test.go(3 files)pkg/auth/identities/azure/*_test.go(1 file)Integration
pkg/auth/factory/factory.go- Register Azure providers/identitiespkg/auth/types/constants.go- Azure provider kind constantspkg/schema/schema.go- Azure auth context schemaerrors/errors.go- Azure error definitionsDocumentation Files (4 files)
website/docs/cli/commands/auth/tutorials/azure-authentication.mdxwebsite/docs/cli/commands/auth/auth-login.mdx(updated)website/blog/2025-11-07-azure-authentication-support.mdxwebsite/blog/authors.yml(updated)Modified Core Files (6 files)
internal/exec/terraform_generate_backend.go- Azure backend authinternal/exec/terraform_utils.go- Azure provider authinternal/exec/utils.go- Azure auth context handlingcmd/auth_console.go- Azure console URL supportgo.mod/go.sum- Dependencies already presentBreaking Changes
None. This is a new feature that doesn't affect existing functionality.
Checklist
Future Enhancements
Potential additions:
References
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests