Conversation
…ssions Implement `atmos auth shell` command to launch an interactive shell with authentication environment variables pre-configured. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds an interactive Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as atmos CLI
participant Conf as Config/AuthManager
participant Auth as Provider/Identity
participant Store as CredentialStore
participant Shell as Shell Process
User->>CLI: run `atmos auth shell [--identity ID] [--shell S] -- args...`
CLI->>Conf: InitCliConfig / build AuthManager
CLI->>Conf: resolve identity (flag/env/config or default)
CLI->>Auth: Authenticate(ctx, identity)
Auth->>Store: persist credentials (backend)
Auth-->>CLI: return env vars (AWS_*, ATMOS_IDENTITY, expiration)
CLI->>Shell: ExecAuthShellCommand(env, shell, args)
Shell->>Shell: set ATMOS_SHLVL, merge env (incl. TF_CLI_ARGS_* handling)
Shell->>Shell: spawn interactive shell (forward IO)
User->>Shell: interactive session
User->>Shell: exit
Shell->>Shell: decrement ATMOS_SHLVL / cleanup
Shell-->>CLI: propagate exit code (ExitCodeError)
CLI-->>User: exit code returned
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
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 (7)
cmd/auth_shell.go(1 hunks)cmd/auth_shell_test.go(1 hunks)cmd/markdown/atmos_auth_shell_usage.md(1 hunks)errors/errors.go(1 hunks)internal/exec/shell_utils.go(4 hunks)internal/exec/shell_utils_test.go(2 hunks)website/docs/cli/commands/auth/auth-shell.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
cmd/markdown/*_usage.md
📄 CodeRabbit inference engine (CLAUDE.md)
Store command examples in embedded markdown files named atmos___usage.md.
Files:
cmd/markdown/atmos_auth_shell_usage.md
**/*.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
**/*.go: All comments must end with periods across all Go code (godot linter enforced).
Organize imports into three groups (stdlib, 3rd-party, Atmos) separated by blank lines and sorted alphabetically within groups; preserve existing aliases.
Add defer perf.Track(<atmosConfig|nil>, ".")() at the start of all public and critical private functions, followed by a blank line.
All errors must be wrapped using static errors from errors/errors.go; prefer errors.Join for multiple errors, fmt.Errorf with %w for context, and check with errors.Is; never use dynamic errors directly.
Use utils.PrintfMarkdown() to render embedded markdown examples for CLI help output.
Co-locate unit tests with implementation files; integration tests reside under tests/.
Distinguish UI output from logging: UI prompts/status/errors to stderr; data/results to stdout; logging for system/debug info only with structured fields.
Most text UI must go to stderr; only data/results to stdout. Prefer utils.PrintfMessageToTUI for UI messages.
Bind all environment variables with viper.BindEnv(); every env var must have an ATMOS_ alternative binding.
Favor cross-platform code: prefer SDKs over external binaries, use filepath/os/runtime, and handle OS-specific differences or use build tags.
For non-standard execution paths, capture telemetry using telemetry.CaptureCmd or telemetry.CaptureCmdString without collecting user data.
Search for existing methods and utilities (internal/exec, pkg/) before implementing new functionality; prefer reuse/refactoring over duplication.
Files:
errors/errors.gocmd/auth_shell_test.gointernal/exec/shell_utils_test.gointernal/exec/shell_utils.gocmd/auth_shell.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
errors/errors.gointernal/exec/shell_utils.gocmd/auth_shell.go
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in website/ when adding features
Ensure consistency between CLI help text and website documentation
Follow the website's documentation structure and style
Keep website code in website/ and follow its architecture/style; test changes locally
Keep CLI and website documentation in sync; document new features with examples and use cases
Files:
website/docs/cli/commands/auth/auth-shell.mdx
website/docs/cli/commands/**/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
website/docs/cli/commands/**/**/*.mdx: All new commands/flags/parameters must have Docusaurus documentation at website/docs/cli/commands//.mdx using prescribed structure and definition lists.
Create Docusaurus docs for new commands with required frontmatter, purpose note, usage, examples, arguments, and flags sections in consistent order.
Files:
website/docs/cli/commands/auth/auth-shell.mdx
website/docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always build the website after any docs changes to website/docs (*.mdx or *.md).
Files:
website/docs/cli/commands/auth/auth-shell.mdx
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands
Implement each CLI command in a separate file under cmd/
Use Viper for managing configuration, environment variables, and flags in the CLI
Keep separation of concerns between CLI interface (cmd) and business logic
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in Cobra command help
Use Viper for configuration management; support files, env vars, and flags with precedence flags > env > config > defaults
Follow single responsibility; separate command interface from business logic
Provide meaningful user feedback and include progress indicators for long-running operations
Provide clear error messages to users and troubleshooting hints where appropriate
cmd/**/*.go: Define one Cobra command per file under cmd/ and follow example pattern with embedded markdown examples.
New commands should rely on automatic telemetry capture; add explicit CaptureCmd/CaptureCmdString only for non-standard paths.
Files:
cmd/auth_shell_test.gocmd/auth_shell.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
**/*_test.go: Write unit tests as table-driven tests focused on behavior; prefer high coverage for pkg/ and internal/exec/; comments must end with periods.
Use t.Skipf with a reason instead of t.Skip; never skip without a reason.
Test files must mirror implementation file names (e.g., aws_ssm_store_test.go pairs with aws_ssm_store.go).
Files:
cmd/auth_shell_test.gointernal/exec/shell_utils_test.go
cmd/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Place each CLI command in its own file (one command per file) under cmd/.
Files:
cmd/auth_shell_test.gocmd/auth_shell.go
cmd/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Place command tests under cmd/ as *_test.go files.
Files:
cmd/auth_shell_test.go
🧠 Learnings (1)
📚 Learning: 2025-10-11T23:25:09.224Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-11T23:25:09.224Z
Learning: Applies to internal/exec/template_funcs.go : Add new template functions in internal/exec/template_funcs.go and add comprehensive tests and documentation if user-facing.
Applied to files:
internal/exec/shell_utils_test.go
🧬 Code graph analysis (3)
internal/exec/shell_utils_test.go (2)
internal/exec/shell_utils.go (1)
ExecAuthShellCommand(264-306)errors/errors.go (1)
ExitCodeError(377-379)
internal/exec/shell_utils.go (4)
pkg/schema/schema.go (1)
AtmosConfiguration(27-65)pkg/perf/perf.go (1)
Track(121-138)pkg/logger/log.go (3)
Debug(24-26)Errorf(59-61)Warn(44-46)errors/errors.go (2)
ErrNoSuitableShell(112-112)ExitCodeError(377-379)
cmd/auth_shell.go (5)
pkg/logger/log.go (2)
Errorf(59-61)Trace(14-16)errors/errors.go (5)
ErrInvalidSubcommand(359-359)ErrFailedToInitializeAtmosConfig(118-118)ErrFailedToInitializeAuthManager(343-343)ErrNoDefaultIdentity(352-352)ErrAuthenticationFailed(333-333)pkg/config/config.go (1)
InitCliConfig(25-62)pkg/schema/schema.go (1)
ConfigAndStacksInfo(460-539)internal/exec/shell_utils.go (1)
ExecAuthShellCommand(264-306)
🪛 LanguageTool
website/docs/cli/commands/auth/auth-shell.mdx
[grammar] ~8-~8: There might be a mistake here.
Context: ...b from '@site/src/components/Screengrab' import Intro from '@site/src/components/...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...vel (increments for nested Atmos shells) - Cloud provider credentials (e.g., `AWS_A...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.18.1)
cmd/markdown/atmos_auth_shell_usage.md
3-3: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
4-4: Dollar signs used before commands without showing output
(MD014, commands-show-output)
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
10-10: Dollar signs used before commands without showing output
(MD014, commands-show-output)
15-15: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
16-16: Dollar signs used before commands without showing output
(MD014, commands-show-output)
21-21: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
22-22: Dollar signs used before commands without showing output
(MD014, commands-show-output)
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
28-28: Dollar signs used before commands without showing output
(MD014, commands-show-output)
33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
34-34: Dollar signs used before commands without showing output
(MD014, commands-show-output)
⏰ 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: Build (windows-latest, windows)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Analyze (go)
- GitHub Check: website-deploy-preview
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (1)
cmd/markdown/atmos_auth_shell_usage.md (1)
3-34: Fix markdownlint violations in fenced blocks.Friendly heads-up: markdownlint is tripping over these snippets because the fences lack a language and each command starts with
$without accompanying output. Please tag every block asshell(or an appropriate language) and drop the$prompt indicator so the linter passes. Example fix:-``` -$ atmos auth shell -``` +```shell +atmos auth shell +```Apply the same pattern to the remaining command examples.
⛔ Skipped due to learnings
Learnt from: samtholiya PR: cloudposse/atmos#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.
Fix flaky TestExecutePacker_Validate test by running `packer init` to install required plugins before validation. The test now: - Runs packer init to install the amazon plugin - Skips gracefully if init fails (network issues) - Proceeds with validation only if init succeeds This prevents the "Missing plugins" error that was causing test failures. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
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. |
The snapshot test infrastructure pads output to terminal width. Regenerated using --regenerate-snapshots flag to match CI expectations.
Addresses CodeRabbit review feedback: 1. Use exec.LookPath to resolve shell command before os.StartProcess - os.StartProcess doesn't search PATH, so bare commands like 'bash' fail - Now resolves to absolute path for non-absolute commands 2. Honor --shell flag on Windows - Previously always used cmd.exe even when --shell was specified - Now respects shellOverride and viper config before falling back to cmd.exe - Only applies -l flag on Unix systems (not Windows) 3. Add osWindows constant to satisfy linting
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1640 +/- ##
==========================================
+ Coverage 66.63% 66.73% +0.09%
==========================================
Files 359 364 +5
Lines 41893 42499 +606
==========================================
+ Hits 27915 28361 +446
- Misses 11919 12050 +131
- Partials 2059 2088 +29
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Comprehensive blog post covering: - Problem statement and motivation - Key features (auto auth, custom shell, nesting, cross-platform) - Real-world use cases (Terraform workflows, debugging, long-running ops) - How it works under the hood - Getting started guide - Environment variables reference - Future enhancements
|
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. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
website/blog/2025-10-15-atmos-auth-shell.mdx(1 hunks)
🧰 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-10-15-atmos-auth-shell.mdx
🪛 LanguageTool
website/blog/2025-10-15-atmos-auth-shell.mdx
[grammar] ~19-~19: Please add a punctuation mark at the end of paragraph.
Context: ...als 4. Re-authenticate when credentials expire This workflow becomes tedious when swi...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~46-~46: There might be a mistake here.
Context: ...tures ### 🔐 Automatic Authentication Your credentials are automatically confi...
(QB_NEW_EN)
[style] ~50-~50: Since ownership is already implied, this phrasing may be redundant.
Context: ...itional setup commands. ### 🐚 Bring Your Own Shell Use your preferred shell by sp...
(PRP_OWN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...commands. ### 🐚 Bring Your Own Shell Use your preferred shell by specifying t...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...shell. ### 🪆 Shell Nesting Awareness Atmos tracks shell nesting with the `ATM...
(QB_NEW_EN)
[style] ~69-~69: Consider using a synonym here to strengthen your wording.
Context: ... you nest an auth shell. This helps you keep track of how many layers deep you are and preven...
(KEEP_TRACK_OF)
[grammar] ~71-~71: There might be a mistake here.
Context: ...ssions. ### 🎯 Custom Shell Arguments Pass custom arguments to your shell usin...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ... -l ``` ### 🌍 Cross-Platform Support Works seamlessly on Linux, macOS, and Wi...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...ell. ### 🔑 Multiple Identity Support Easily switch between different identiti...
(QB_NEW_EN)
[grammar] ~141-~141: There might be a mistake here.
Context: ...NTITY` to track which identity is active 5. Launches your preferred shell with all...
(QB_NEW_EN)
[grammar] ~142-~142: There might be a mistake here.
Context: ...ith all environment variables configured 6. Propagates the shell's exit code back ...
(QB_NEW_EN)
[grammar] ~143-~143: Please add a punctuation mark at the end of paragraph.
Context: ...hell's exit code back to Atmos when you exit ## Environment Variables When you're ...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~149-~149: There might be a mistake here.
Context: ...TITY**: The name of the active identity - **ATMOS_SHLVL`**: Shell nesting level (increments for ne...
(QB_NEW_EN)
[grammar] ~150-~150: There might be a mistake here.
Context: ...ing level (increments for nested shells) - Cloud provider credentials: Platform-s...
(QB_NEW_EN)
[grammar] ~192-~192: There might be a mistake here.
Context: ... ## Learn More - Command Documentation - [Authentication Guide](/core-concepts/aut...
(QB_NEW_EN)
[grammar] ~193-~193: There might be a mistake here.
Context: ...auth/auth-shell) - Authentication Guide - [PR #1640](https://github.com/cloudposse/...
(QB_NEW_EN)
[grammar] ~200-~200: There might be a mistake here.
Context: ...clude: - Session duration customization - Automatic credential refresh - Multi-clo...
(QB_NEW_EN)
[grammar] ~201-~201: There might be a mistake here.
Context: ...omization - Automatic credential refresh - Multi-cloud authentication in a single s...
(QB_NEW_EN)
[grammar] ~202-~202: There might be a mistake here.
Context: ...i-cloud authentication in a single shell - Integration with external secret manager...
(QB_NEW_EN)
[grammar] ~209-~209: There might be a mistake here.
Context: ...ommunity! --- Happy authenticating! 🚀
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: website-deploy-preview
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
Condensed key features section to emphasize the core value: identity-based authentication and seamless identity switching. Other features (custom shell, args, nesting, cross-platform) mentioned succinctly in one sentence.
Changed messaging to emphasize the core purpose: launch a shell with active cloud credentials to work naturally without juggling environment variables or re-authenticating.
The /core-concepts/authentication page doesn't exist. Removed the link to fix website build.
Replaced "credentials removed from environment" language with more accurate description: when you exit, the shell is destroyed and you return to your parent shell where those credentials were never present. Changes: - "credentials are removed from your environment" → "you return to your parent shell where those credentials were never present" - "those credentials are removed from the environment" → "the shell is destroyed and you return to your parent shell where those credentials don't exist" - "those production credentials are removed from your environment" → "you're back in your parent shell where those production credentials don't exist" - Table: "Credentials removed from environment on exit" → "Return to parent shell on exit" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Removed "Atmos Auth previously..." language since the feature just launched.
Emphasized the Docker/Dev Container use case as the key benefit of file-based
keyrings—solving the cross-OS boundary problem where system keyrings aren't
accessible inside containers.
Key changes:
- Replaced "The Problem" section with "Why Different Keyring Types Matter"
- Removed historical references ("previously relied exclusively", "previously these tests were skipped")
- Added Docker/Dev Container as primary example for file-based keyrings
- Updated introduction to highlight cross-boundary credential sharing
- Simplified "What This Means for You" to "What This Gives You"
- Removed "backward compatibility" language from system keyring description
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
website/blog/2025-10-15-atmos-auth-shell.mdx (1)
191-191: Add period to complete list item.Line 191 ends without terminal punctuation, breaking list formatting. This is a flagged duplicate from prior reviews.
- - Automating workflows where a subshell isn't appropriate + - Automating workflows where a subshell isn't appropriate.
🧹 Nitpick comments (3)
website/blog/2025-10-15-atmos-auth-shell.mdx (1)
1-249: Build website after documentation changes.Per coding guidelines, run
npm run buildlocally to verify no broken links, missing assets, or build errors after publishing documentation changes.website/blog/2025-10-18-keyring-backends.mdx (2)
45-82: Minor word choice: Reduce repetition of "where you need".Line 59 repeats a word pattern from nearby text. Consider rewording to improve readability:
-**Best for**: Shared environments, servers, or when you need portability across different machines. +**Best for**: Shared environments, servers, or scenarios requiring portability across different machines.
84-118: Minor word choice: Reduce repetition in "where you need".Line 95 has similar repetition from nearby context. A small rephrase improves flow:
-**Best for**: Unit tests, integration tests, and CI/CD pipelines where you need fast, isolated credential storage without external dependencies. +**Best for**: Unit tests, integration tests, and CI/CD pipelines needing fast, isolated credential storage without external dependencies.
📜 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-10-15-atmos-auth-shell.mdx(1 hunks)website/blog/2025-10-18-keyring-backends.mdx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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 casesAlways build the website (npm run build) after documentation changes to ensure no errors, broken links, or missing assets.
Files:
website/blog/2025-10-15-atmos-auth-shell.mdxwebsite/blog/2025-10-18-keyring-backends.mdx
website/**/{*.md,*.mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use definition lists
for arguments and flags in docs instead of tables.
Files:
website/blog/2025-10-15-atmos-auth-shell.mdxwebsite/blog/2025-10-18-keyring-backends.mdx
🪛 LanguageTool
website/blog/2025-10-15-atmos-auth-shell.mdx
[grammar] ~191-~191: Please add a punctuation mark at the end of paragraph.
Context: ...mating workflows where a subshell isn't appropriate Think of it this way: `atmos auth shel...
(PUNCTUATION_PARAGRAPH_END)
website/blog/2025-10-18-keyring-backends.mdx
[style] ~59-~59: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...ared environments, servers, or when you need portability across different machines. ...
(EN_REPEATEDWORDS_NEED)
[style] ~95-~95: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...on tests, and CI/CD pipelines where you need fast, isolated credential storage witho...
(EN_REPEATEDWORDS_NEED)
[grammar] ~168-~168: Ensure spelling is correct
Context: ...: ATMOS_KEYRING_PASSWORD Mount the keyring into your container: yaml # docker-...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ 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: Review Dependency Licenses
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (go)
- GitHub Check: website-deploy-preview
- GitHub Check: autofix
- GitHub Check: Summary
🔇 Additional comments (2)
website/blog/2025-10-18-keyring-backends.mdx (2)
1-270: Blog post is clear, comprehensive, and well-structured.The post effectively introduces the three keyring backends with practical use cases, security considerations, and configuration examples. The progression from concept explanation through real-world scenarios to hands-on examples is well-paced. Consider the minor word choice suggestions above and verify the YAML examples against the implementation before merge.
30-35: YAML schema examples verified correct—run website build locally to finalize.All YAML keyring configuration examples in the blog post are accurate and match the actual implementation:
auth.keyring.typevalues (system, file, memory) align with the schemaspec.pathandspec.password_envfor file keyrings matchkeyring_file.goimplementation- Examples at lines 30–35, 49–57, 88–93, 142–150, 158–166, 183–186, and 247–253 are consistent
The sandbox environment lacks build dependencies (docusaurus), so per the coding guidelines, run
npm run buildlocally in thewebsite/directory to verify no broken links, missing assets, or formatting errors before merging.
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
Resolved conflicts and interface compatibility: - pkg/auth/credentials/store.go: Kept flexible keyring backend system - tests/snapshots/TestCLICommands_atmos_auth_invalid-command.stderr.golden: Included both logout and shell commands - Added Logout() method to mock Provider and Identity - Added GetFilesDisplayPath() to mock Provider - Removed obsolete GetAny/SetAny tests (methods removed in main) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
pkg/config/cache.go (1)
38-39: Solid refactor to use centralized XDG resolution.The change correctly delegates cache directory resolution to
xdg.GetXDGCacheDir, which handles bothATMOS_XDG_CACHE_HOMEandXDG_CACHE_HOME. Error wrapping and path handling follow the coding guidelines. Based on learnings.Optional: Consider adding
defer perf.Track(...)to this and other public functions (LoadCache,SaveCache,UpdateCache) for consistency with the coding guideline:func GetCacheFilePath() (string, error) { defer perf.Track("config.GetCacheFilePath", nil)() cacheDir, err := xdg.GetXDGCacheDir("", CacheDirPermissions) // ... rest of implementation }
📜 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 (8)
.gitignore(1 hunks)NOTICE(5 hunks)errors/errors.go(1 hunks)pkg/auth/credentials/store_test.go(1 hunks)pkg/auth/providers/mock/identity.go(1 hunks)pkg/auth/providers/mock/provider.go(1 hunks)pkg/config/cache.go(2 hunks)tests/snapshots/TestCLICommands_atmos_auth_invalid-command.stderr.golden(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .gitignore
- errors/errors.go
- pkg/auth/providers/mock/provider.go
🧰 Additional context used
📓 Path-based instructions (9)
tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Integration tests and shared test utilities live under tests/ with fixtures in tests/test-cases/; keep new integration tests in this tree.
Files:
tests/snapshots/TestCLICommands_atmos_auth_invalid-command.stderr.golden
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Place business logic in pkg rather than in cmd
Files:
pkg/config/cache.gopkg/auth/credentials/store_test.gopkg/auth/providers/mock/identity.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
**/*.go: All comments must end with periods. Applies to single-line, multi-line, inline, and documentation comments.
Organize imports into three groups (stdlib, 3rd-party, Atmos) separated by blank lines and sort alphabetically within each group; preserve existing aliases.
Add defer perf.Track(...) in all public and critical private functions; include a blank line after the perf.Track call; use "packagename.FunctionName" as the name; pass atmosConfig when available, otherwise nil.
Wrap all returned errors using static errors (from errors/errors.go); use errors.Join to combine errors; use fmt.Errorf with %w to add context; use errors.Is for checks; never rely on err.Error() string matching.
Always use viper.BindEnv with ATMOS_ alternatives; prefer ATMOS_* first with fallback to external env vars when needed.
Prefer many small, focused files; never use //revive:disable:file-length-limit to bypass file length limits.
Use filepath.Join, os.PathSeparator, and runtime.GOOS for portable path/OS handling; use build constraints when necessary.
Files:
pkg/config/cache.gopkg/auth/credentials/store_test.gopkg/auth/providers/mock/identity.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Document all exported functions, types, and methods with Go doc comments
Files:
pkg/config/cache.gopkg/auth/providers/mock/identity.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Use Viper for configuration management and bind environment variables consistently.
Distinguish structured logging from UI output: UI prompts/status/errors to stderr; data/results to stdout; do not use logging for UI elements.
Prefer SDKs over invoking external binaries for cross-platform compatibility (e.g., terraform-exec instead of exec.Command("terraform")).
Files:
pkg/config/cache.gopkg/auth/credentials/store_test.gopkg/auth/providers/mock/identity.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
**/*_test.go: Unit tests should focus on pure functions and favor table-driven tests.
Test behavior (inputs/outputs), not implementation detail; avoid tautologies and stub-only tests; remove always-skipped tests.
Always call production code within tests rather than duplicating logic under test.
Use t.Skipf with a clear reason when skipping tests; never call t.Skipf without a reason.
Use go.uber.org/mock/mockgen for interface mocks; never hand-write large mock implementations.
Add //go:generate mockgen directives for mocks in test files.
Files:
pkg/auth/credentials/store_test.go
{pkg,internal,cmd}/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Test files should mirror implementation naming (e.g., foo.go has foo_test.go) and be co-located with implementations.
Files:
pkg/auth/credentials/store_test.go
pkg/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests for packages under pkg/ should reside alongside implementations with _test.go suffix.
Files:
pkg/auth/credentials/store_test.go
**/mock/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Exclude files under any mock/ directory from coverage metrics.
Files:
pkg/auth/providers/mock/identity.go
🧠 Learnings (5)
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
PR: cloudposse/atmos#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/config/cache.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
PR: cloudposse/atmos#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/config/cache.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
PR: cloudposse/atmos#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/config/cache.go
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
PR: cloudposse/atmos#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/config/cache.go
📚 Learning: 2024-12-13T15:33:34.159Z
Learnt from: Listener430
PR: cloudposse/atmos#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/config/cache.go
🧬 Code graph analysis (3)
pkg/config/cache.go (1)
pkg/xdg/xdg.go (1)
GetXDGCacheDir(15-17)
pkg/auth/credentials/store_test.go (3)
pkg/auth/types/interfaces.go (1)
CredentialStore(129-144)pkg/auth/credentials/store.go (1)
NewCredentialStore(41-45)pkg/auth/credentials/suite_test.go (1)
RunCredentialStoreTests(17-29)
pkg/auth/providers/mock/identity.go (5)
pkg/auth/factory/factory.go (1)
NewIdentity(37-54)pkg/auth/providers/mock/provider.go (7)
Provider(30-33)MockExpirationYear(15-15)MockExpirationMonth(17-17)MockExpirationDay(19-19)MockExpirationHour(21-21)MockExpirationMinute(23-23)MockExpirationSecond(25-25)pkg/schema/schema.go (2)
Context(409-424)ConfigAndStacksInfo(488-567)pkg/auth/types/interfaces.go (1)
ICredentials(161-167)pkg/auth/providers/mock/credentials.go (1)
Credentials(10-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Lint (golangci)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- 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: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (6)
tests/snapshots/TestCLICommands_atmos_auth_invalid-command.stderr.golden (1)
11-13: LGTM! Snapshot correctly updated with new shell subcommand.The addition of "shell" to the valid subcommands list is properly formatted and maintains alphabetical ordering.
NOTICE (1)
597-599: LGTM! Proper license attribution following best practices.The new dependency entries are correctly formatted and placed in their respective license sections. The attributions follow ASF best practices for documenting third-party dependencies with non-Apache-2.0 licenses, and the BSD-2-Clause license for
godbus/dbusis properly noted as Category A compatible.Also applies to: 923-925, 1151-1153, 1223-1225, 1415-1417
pkg/auth/credentials/store_test.go (1)
95-102: Nice use of the shared suite pattern.The factory-based approach cleanly delegates coverage to the shared test suite, which is a solid way to validate the default store against the
CredentialStoreinterface. The existing tests for AWS and OIDC credentials provide complementary coverage for specific credential types.pkg/auth/providers/mock/identity.go (2)
1-10: Import organization looks good.Imports are correctly grouped with stdlib separated from Atmos packages.
12-16: LGTM on the struct definition.Clean encapsulation with unexported fields.
pkg/config/cache.go (1)
22-25: Nice addition of the CacheDirPermissions constant.The constant is well-documented and the permission value (0o755) is appropriate for cache directories. Exporting it allows for consistent reuse across the codebase.
Fixed TestDelete_Flow test failure by treating keyring.ErrNotFound as success (idempotent delete). Updated snapshots to include proper trailing whitespace formatting for command output alignment. Changes: - pkg/auth/credentials/keyring_system.go: Check for keyring.ErrNotFound in Delete() - tests/snapshots: Updated trailing whitespace to match actual CLI output formatting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Added perf.Track instrumentation to all public methods in mock Identity
to comply with performance tracking guidelines. Updated cache.go doc
comment to accurately describe current behavior.
Changes:
- pkg/auth/providers/mock/identity.go: Added perf.Track to all public methods
- NewIdentity, Kind, GetProviderName, Authenticate, Validate, Environment,
PostAuthenticate, Logout
- pkg/config/cache.go: Updated GetCacheFilePath doc comment to remove
outdated reference to environment variable binding
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Replaced incorrect snapshot update commands with the documented approach from tests/README.md. The correct method is to use the -regenerate-snapshots flag, not environment variables. Changes: - Replaced "Golden Snapshots (MANDATORY)" section with accurate information - Added snapshot types explanation (.stdout.golden, .stderr.golden, .tty.golden) - Documented proper regeneration commands using -regenerate-snapshots flag - Added workflow for reviewing and committing regenerated snapshots - Added reference to tests/README.md for complete documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
Resolved conflict in CLAUDE.md by using main's concise testing section format while preserving the golden snapshots regeneration instructions. Fixed errorlint issue in auth_shell.go to use %w instead of %v for error wrapping. Changes from main: - Interactive identity selection for atmos auth login - Enhanced SAML driver detection with browser version checks - New lintroller rule for os.Args validation (reveals pre-existing issues to fix) - GitHub workflow improvements and CLAUDE.md size checking Note: The new os.Args lintroller rule is flagging pre-existing usage of os.Args in test files. These should be migrated to cmd.SetArgs() but are not part of this branch's changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Replaced bulleted list with semantic <dl> for environment variables in atmos auth shell blog post, following documentation guidelines. Added "(AWS identities only)" qualifier to AWS-specific variables. Changes: - Converted bullet points to <dl> with <dt><code>...</code></dt> and <dd> - ATMOS_IDENTITY, ATMOS_SHLVL: General variables - AWS_SHARED_CREDENTIALS_FILE, AWS_CONFIG_FILE, AWS_PROFILE: AWS-only 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Changed suite_test.go to expect NoError when deleting a non-existent credential, matching the idempotent behavior implemented in the Delete method and consistent with main branch expectations. Changes: - suite_test.go: assert.Error → assert.NoError for non-existent delete - Updated comment to clarify idempotent behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/auth/credentials/suite_test.go (2)
55-57: Consider asserting all stored fields.SessionToken and Expiration are stored but not verified in the assertions. Adding these checks would strengthen the test coverage.
assert.Equal(t, creds.AccessKeyID, awsCreds.AccessKeyID) assert.Equal(t, creds.SecretAccessKey, awsCreds.SecretAccessKey) + assert.Equal(t, creds.SessionToken, awsCreds.SessionToken) assert.Equal(t, creds.Region, awsCreds.Region) + assert.Equal(t, creds.Expiration, awsCreds.Expiration)
31-58: Add error case for non-existent retrieval.The test validates the happy path but doesn't verify that Retrieve returns an error when the alias doesn't exist. This error behavior is critical for the store contract.
assert.Equal(t, creds.AccessKeyID, awsCreds.AccessKeyID) assert.Equal(t, creds.SecretAccessKey, awsCreds.SecretAccessKey) assert.Equal(t, creds.Region, awsCreds.Region) + + // Retrieve non-existent alias should error. + _, err = store.Retrieve("non-existent") + assert.Error(t, err) }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
pkg/auth/credentials/suite_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/credentials/suite_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
**/*_test.go: Use table-driven tests for coverage and clarity
Test behavior, not implementation; avoid tautological/stub tests; use DI for testability
Tests must call actual production code paths; never duplicate logic in tests
Use t.Skipf("reason") with clear context when skipping tests; CLI tests auto-build temp binaries
Files:
pkg/auth/credentials/suite_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
**/*.go: Define interfaces for major functionality (interface-driven design) to enable testability and decoupling
Generate mocks with go.uber.org/mock/mockgen using //go:generate directives; do not write manual mocks
Prefer the functional Options pattern to avoid functions with many parameters
Use context.Context only for cancellation, deadlines/timeouts, and sparing request-scoped values; never for config or dependencies
When used, context.Context must be the first parameter of functions
All comments must end with periods (godot linter)
Organize imports in three groups (stdlib, third-party, atmos) separated by blank lines and sorted alphabetically; maintain aliases cfg, log, u, errUtils
Add defer perf.Track(atmosConfig, "pkg.FuncName")() with a blank line in all public functions (use nil if no atmosConfig)
Use Viper for configuration loading with precedence: CLI flags → ENV vars → config files → defaults
Use static errors from errors/errors.go; wrap with fmt.Errorf and errors.Join; check with errors.Is; never compare error strings or create dynamic errors
Add //go:generate mockgen directives near interfaces and generate mocks; never hand-write mocks
Keep files focused and under 600 lines; one cmd/impl per file; co-locate tests; never disable revive file-length limit
Bind environment variables with viper.BindEnv and use ATMOS_ prefix
UI (prompts/status) must be written to stderr; data to stdout; logging only for system events, never for UI
Ensure cross-platform compatibility (Linux/macOS/Windows); use SDKs over binaries and filepath.Join() for paths
Files:
pkg/auth/credentials/suite_test.go
🧬 Code graph analysis (1)
pkg/auth/credentials/suite_test.go (2)
pkg/auth/types/aws_credentials.go (1)
AWSCredentials(11-18)pkg/auth/types/github_oidc_credentials.go (1)
OIDCCredentials(14-18)
⏰ 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: Build (macos)
- GitHub Check: website-deploy-preview
- GitHub Check: Review Dependency Licenses
- GitHub Check: autofix
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (3)
pkg/auth/credentials/suite_test.go (3)
13-29: Nice factory pattern for multi-backend testing.The reusable suite structure is clean and enables consistent validation across different credential store implementations.
60-88: Solid expiration testing.Covers expired, fresh, and missing credentials. The UTC time handling ensures cross-platform consistency.
90-108: Well-rounded delete tests.Tests both successful deletion and idempotent behavior. Using OIDC credentials here provides good type coverage across the suite.
Updated memory and file keyring stores to treat "not found" as success when deleting credentials, matching the idempotent behavior of the system keyring store. Changes: - keyring_memory.go: Remove error check, just delete (idempotent) - keyring_file.go: Check for keyring.ErrKeyNotFound and return nil This ensures consistent idempotent delete behavior across all three credential store backends (system, file, memory). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
These changes were released in v1.196.0-rc.0. |
what
atmos auth shellcommand to launch an interactive shell with authentication environment variables pre-configured$SHELLenvironment variable with fallbacks to bash/sh--shellflag with viper binding toATMOS_SHELLandSHELLenvironment variables--separator for passing custom shell arguments to the launched shellATMOS_SHLVLenvironment variableATMOS_IDENTITYenvironment variable in the shell sessionwhy
atmos terraform shell, this provides a consistent experience for authenticated sessionsreferences
atmos terraform shellcommand implementationatmos auth execandatmos auth envtesting
documentation
website/docs/cli/commands/auth/auth-shell.mdxwith full command documentationcmd/markdown/atmos_auth_shell_usage.mdwith usage examplesSummary by CodeRabbit
New Features
Documentation
Tests
Chores