Skip to content

feat: Add atmos auth shell command#1640

Merged
aknysh merged 88 commits intomainfrom
atmos-auth-shell
Oct 22, 2025
Merged

feat: Add atmos auth shell command#1640
aknysh merged 88 commits intomainfrom
atmos-auth-shell

Conversation

@osterman
Copy link
Member

@osterman osterman commented Oct 16, 2025

what

  • Add atmos auth shell command to launch an interactive shell with authentication environment variables pre-configured
  • Implement shell detection that respects $SHELL environment variable with fallbacks to bash/sh
  • Add --shell flag with viper binding to ATMOS_SHELL and SHELL environment variables
  • Support -- separator for passing custom shell arguments to the launched shell
  • Track shell nesting level with ATMOS_SHLVL environment variable
  • Propagate shell exit codes back to Atmos process
  • Set ATMOS_IDENTITY environment variable in the shell session

why

  • Users need an easy way to work interactively with cloud credentials without manually managing environment variables
  • Similar to atmos terraform shell, this provides a consistent experience for authenticated sessions
  • Allows running multiple commands in a single authenticated session without re-authenticating
  • Supports custom shell configurations and arguments for flexibility

references

  • Similar to existing atmos terraform shell command implementation
  • Follows authentication patterns from atmos auth exec and atmos auth env

testing

  • Comprehensive unit tests with 80-100% coverage on testable functions
  • 25 passing tests covering:
    • Shell detection and fallback logic (100% coverage)
    • Environment variable management (100% coverage)
    • Shell nesting level tracking (83-100% coverage)
    • Exit code propagation (tested with codes 0, 1, 42)
    • Flag parsing and viper integration
    • Cross-platform support (Unix and Windows)
  • All linting checks passing (0 issues)
  • Pre-commit hooks passing

documentation

  • Added website/docs/cli/commands/auth/auth-shell.mdx with full command documentation
  • Created cmd/markdown/atmos_auth_shell_usage.md with usage examples
  • Includes purpose note, usage patterns, examples, and environment variable reference

Summary by CodeRabbit

  • New Features

    • Interactive authenticated shell with shell selection, argument passthrough, nested-shell tracking, and identity selection.
    • Pluggable credential storage: system, file (path/password) and memory backends selectable via config/env.
    • Deterministic mock auth provider for testing.
  • Documentation

    • New auth-shell docs, usage examples, blog posts, keyring-backends guide, XDG docs, and PRD.
  • Tests

    • Expanded unit/integration coverage for shell flows, keyring backends, XDG, and credential stores.
  • Chores

    • Added keyring-related dependencies, CI/workflow and tooling adjustments.

…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>
@osterman osterman requested a review from a team as a code owner October 16, 2025 01:05
@github-actions github-actions bot added the size/l Large size PR label Oct 16, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

📝 Walkthrough

Walkthrough

Adds an interactive atmos auth shell command and supporting shell-launch utilities (ATMOS_SHLVL, env merging, exit-code preservation), pluggable credential keyring backends (system/file/memory) selectable via config/env, XDG helpers, a deterministic mock auth provider for tests, extensive tests, and documentation updates.

Changes

Cohort / File(s) Summary
Auth Shell Command
cmd/auth_shell.go, cmd/auth_shell_test.go, cmd/markdown/atmos_auth_shell_usage.md
New Cobra auth shell command (DisableFlagParsing + manual parsing), identity resolution and binding, authentication producing env vars, invocation of ExecAuthShellCommand, completions, tests, and usage docs.
Shell Utilities
internal/exec/shell_utils.go, internal/exec/shell_utils_test.go
Adds ExecAuthShellCommand and helpers to spawn shells, ATMOS_SHLVL management, env merging and TF_CLI_ARGS_* handling, convert/map helpers, shell detection/resolution, ExitCodeError propagation, and updated ExecuteShellCommand signature; tests added/expanded.
Mock Auth Provider
pkg/auth/providers/mock/*
pkg/auth/providers/mock/provider.go, .../identity.go, .../credentials.go, .../credentials_test.go
Deterministic mock provider/identity/credentials for testing with fixed expiration and environment data; constructors and test coverage added.
Keyring Backends (impl + tests)
pkg/auth/credentials/keyring_system.go, pkg/auth/credentials/keyring_file.go, pkg/auth/credentials/keyring_memory.go, pkg/auth/credentials/*_test.go
Implements system (OS), file (99designs/keyring), and memory keyring stores with typed credential envelopes, CRUD APIs, error wrapping, perf instrumentation, and comprehensive tests.
Credential Store API / Factory
pkg/auth/credentials/store.go, pkg/auth/credentials/store_test.go, pkg/auth/credentials/suite_test.go
Introduces NewCredentialStoreWithConfig(authConfig) and runtime backend selection via auth.keyring / ATMOS_KEYRING_TYPE, plus a credential-store test suite and refactored tests.
Schema & Errors
pkg/schema/schema_auth.go, errors/errors.go
Adds AuthConfig.Keyring (KeyringConfig) and sentinel error ErrNoSuitableShell.
Auth Factory & Integration
pkg/auth/factory/factory.go, cmd/auth_integration_test.go
Factory supports mock provider/identity; CI integration test now sets ATMOS_KEYRING_TYPE=memory in CI instead of skipping.
XDG Support & Cache
pkg/xdg/xdg.go, pkg/xdg/xdg_test.go, pkg/config/cache.go
New XDG helper functions with ATMOS_ overrides and directory creation; cache resolution now delegates to XDG helper; tests added.
Fixtures, Tests & Snapshots
tests/fixtures/scenarios/atmos-auth-mock/*, tests/test-cases/auth-cli.yaml, tests/snapshots/*
Adds mock scenario fixtures, test-cases, and updates golden snapshots for new auth/keyring fields and auth-shell output.
Packer Test
internal/exec/packer_test.go
Adds preliminary Packer init step before validate and skips the test if init fails due to network.
Build / Tooling / Workflows
Makefile, .github/workflows/pre-commit.yml, .gitignore, go.mod, NOTICE
Adds deps/pre-download step to pre-commit, moves targets to depend on deps, updates .gitignore scoping, adds keyring-related dependencies and NOTICE entries.
Test Helpers / Output
tests/cli_test.go
Normalizes expiration durations in sanitized test output for deterministic snapshots.
Docs & Website
website/docs/cli/commands/auth/*, website/docs/cli/global-flags.mdx, website/blog/*, pkg/auth/docs/keyring-backends-design.md, docs/prd/*
Adds docs for auth-shell, exec-vs-shell tip, keyring backends design/PRDs, XDG env vars, blog posts, and examples.
Examples / Tooling
examples/quick-start-advanced/.tool-versions
Removed explicit Atmos version entry from example tool versions.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • aknysh

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "feat: Add atmos auth shell command" directly and accurately summarizes the primary change in this changeset. The title clearly identifies the new feature being introduced—an interactive authentication shell command—and uses the conventional commit format appropriately. The title is specific and unambiguous; a teammate reviewing the git history would immediately understand that this PR introduces the auth shell capability. While the changeset includes supporting infrastructure (keyring backends, XDG directory resolution, mock providers), the core feature and the developer's intent is clearly captured by the title.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch atmos-auth-shell

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 032025e and bece832.

📒 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.go
  • cmd/auth_shell_test.go
  • internal/exec/shell_utils_test.go
  • internal/exec/shell_utils.go
  • cmd/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.go
  • internal/exec/shell_utils.go
  • cmd/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.go
  • cmd/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.go
  • internal/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.go
  • cmd/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 as shell (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.

@osterman osterman added the minor New features that do not break anything label Oct 16, 2025
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>
@github-actions github-actions bot added size/xl Extra large size PR and removed size/l Large size PR labels Oct 16, 2025
@mergify
Copy link

mergify bot commented Oct 16, 2025

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.
Consider refactoring it into smaller, more focused PRs to facilitate a smoother review process.

The snapshot test infrastructure pads output to terminal width.
Regenerated using --regenerate-snapshots flag to match CI expectations.
@github-actions github-actions bot added size/l Large size PR and removed size/xl Extra large size PR labels Oct 16, 2025
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
Copy link

codecov bot commented Oct 16, 2025

Codecov Report

❌ Patch coverage is 71.46853% with 204 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.73%. Comparing base (ecbf1cf) to head (f9d9651).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/auth/credentials/keyring_file.go 74.01% 41 Missing and 12 partials ⚠️
pkg/auth/credentials/keyring_system.go 59.04% 37 Missing and 6 partials ⚠️
internal/exec/shell_utils.go 80.00% 21 Missing and 8 partials ⚠️
cmd/auth_shell.go 56.92% 21 Missing and 7 partials ⚠️
pkg/auth/credentials/keyring_memory.go 78.81% 18 Missing and 7 partials ⚠️
pkg/auth/credentials/store.go 48.71% 19 Missing and 1 partial ⚠️
pkg/xdg/xdg.go 81.81% 4 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            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     
Flag Coverage Δ
unittests 66.73% <71.46%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
errors/errors.go 100.00% <ø> (ø)
pkg/auth/factory/factory.go 81.25% <100.00%> (+2.67%) ⬆️
pkg/config/cache.go 74.26% <100.00%> (-0.24%) ⬇️
pkg/xdg/xdg.go 81.81% <81.81%> (ø)
pkg/auth/credentials/store.go 52.17% <48.71%> (-27.83%) ⬇️
pkg/auth/credentials/keyring_memory.go 78.81% <78.81%> (ø)
cmd/auth_shell.go 56.92% <56.92%> (ø)
internal/exec/shell_utils.go 54.29% <80.00%> (+16.00%) ⬆️
pkg/auth/credentials/keyring_system.go 59.04% <59.04%> (ø)
pkg/auth/credentials/keyring_file.go 74.01% <74.01%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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
@github-actions github-actions bot added size/xl Extra large size PR and removed size/l Large size PR labels Oct 16, 2025
@mergify
Copy link

mergify bot commented Oct 16, 2025

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.
Consider refactoring it into smaller, more focused PRs to facilitate a smoother review process.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 74f4314 and d3b5c92.

📒 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>
coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 21, 2025
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>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ 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 build locally 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 5ab4a81 and 0617036.

📒 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 cases

Always 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.mdx
  • website/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.mdx
  • website/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.type values (system, file, memory) align with the schema
  • spec.path and spec.password_env for file keyrings match keyring_file.go implementation
  • 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 build locally in the website/ directory to verify no broken links, missing assets, or formatting errors before merging.

@mergify
Copy link

mergify bot commented Oct 22, 2025

💥 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>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 both ATMOS_XDG_CACHE_HOME and XDG_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.

📥 Commits

Reviewing files that changed from the base of the PR and between 0617036 and ed5e217.

📒 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.go
  • pkg/auth/credentials/store_test.go
  • pkg/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.go
  • pkg/auth/credentials/store_test.go
  • pkg/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.go
  • pkg/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.go
  • pkg/auth/credentials/store_test.go
  • pkg/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/dbus is 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 CredentialStore interface. 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.

osterman and others added 3 commits October 21, 2025 22:01
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>
@mergify
Copy link

mergify bot commented Oct 22, 2025

💥 This pull request now has conflicts. Could you fix it @osterman? 🙏

osterman and others added 2 commits October 21, 2025 22:59
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>
coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 22, 2025
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>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
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.

📥 Commits

Reviewing files that changed from the base of the PR and between 0ebaf7d and b0e19e6.

📒 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.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 22, 2025
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>
@github-actions
Copy link

These changes were released in v1.196.0-rc.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor New features that do not break anything size/xl Extra large size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants