-
-
Notifications
You must be signed in to change notification settings - Fork 135
feat(auth): add aws/assume-root identity for centralized root access #1828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
aws/assume-root Identityaws/assume-root Identity Kind
aws/assume-root Identity Kindaws/assume-root Identity Kind
aws/assume-root Identity Kindaws/assume-root Identity Kind
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (78.62%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1828 +/- ##
==========================================
+ Coverage 72.13% 72.20% +0.07%
==========================================
Files 475 477 +2
Lines 45713 46003 +290
==========================================
+ Hits 32974 33217 +243
- Misses 10118 10144 +26
- Partials 2621 2642 +21
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughAdds a new aws/assume-root identity: documentation, factory wiring, a provider constant, STS client helper, full identity implementation (auth, env/files, creds handling, logout), and comprehensive unit tests. No public API changes beyond the new provider-kind constant. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor CLI
participant Factory
participant Identity as AssumeRootIdentity
participant STS as AWS STS
participant FileMgr as AWS File Manager
participant EnvPrep as PrepareEnvironment
CLI->>Factory: Request identity of kind "aws/assume-root"
Factory->>Identity: NewAssumeRootIdentity(config)
CLI->>Identity: Authenticate(base AWS creds)
Identity->>STS: NewSTSClientWithCredentials(base creds, region)
STS-->>Identity: STS client + resolved region
Identity->>STS: AssumeRoot(AssumeRootInput: targetPrincipal, taskPolicy, duration)
STS-->>Identity: Temporary AWS credentials
Identity->>FileMgr: Write credentials/profile (shared creds file)
FileMgr-->>Identity: Confirm file paths
Identity->>EnvPrep: PrepareEnvironment(profile, creds, region)
EnvPrep-->>CLI: Inject env vars (AWS_SHARED_CREDENTIALS_FILE, AWS_PROFILE, AWS_REGION, ...)
CLI->>Identity: Run tasks using injected credentials
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (2)website/blog/**/*.mdx📄 CodeRabbit inference engine (CLAUDE.md)
Files:
website/**📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-04-25T20:54:19.701ZApplied to files:
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (1)
docs/prd/aws-assume-root-identity.md (1)
1-714: Comprehensive and well-structured PRD.The document provides clear requirements across functional, non-functional, and technical dimensions. The implementation plan with four phased deliverables is actionable, and the testing strategy covers unit, mock, and integration scenarios. Configuration examples and error handling patterns are practical.
Minor suggestion: Consider adding a "Related Identities" section that explicitly cross-references
aws/assume-roleandaws/permission-setimplementations for developers implementing this feature. This would reduce context-switching when working across related identity types.
📜 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)
docs/prd/aws-assume-root-identity.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/prd/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Create PRD documentation in docs/prd/ using kebab-case filenames (e.g., command-registry-pattern.md, error-handling-strategy.md).
Files:
docs/prd/aws-assume-root-identity.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1775
File: pkg/auth/providers/aws/sso_provisioning.go:40-79
Timestamp: 2025-11-10T20:03:56.875Z
Learning: In the Atmos AWS SSO provider (pkg/auth/providers/aws/sso_provisioning.go), the OAuth access token from the AWS SSO device flow is intentionally stored in the `AccessKeyID` field of `AWSCredentials` during authentication. This token is then extracted and used for ListAccounts and ListAccountRoles API calls during identity provisioning. This design reuses the existing `AWSCredentials` type for token transport rather than creating a separate credential type.
📚 Learning: 2025-04-25T20:54:19.701Z
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
Applied to files:
docs/prd/aws-assume-root-identity.md
🪛 LanguageTool
docs/prd/aws-assume-root-identity.md
[style] ~15-~15: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ...dentials across all member accounts. Use Case: Organizations implementing AWS best practices for centralized root access can use atmos auth exec --identity core-audit/iam-audit-root-access to assume root-level privileges on member accounts for specific task policies, enabling operations like credential auditing, root password management, and S3/SQS bucket policy unlocking. ## Problem Statement ### Current Stat...
(TOO_LONG_SENTENCE)
[typographical] ~146-~146: Consider using a typographic close quote here.
Context: ...dentials ``` Implementation: - Add "aws/assume-root" case to `factory.NewId...
(EN_QUOTES)
[grammar] ~148-~148: Please add a punctuation mark at the end of paragraph.
Context: ...yKindAWSAssumeRoot = "aws/assume-root"totypes/constants.go` ### FR-2: Princip...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~192-~192: Please add a punctuation mark at the end of paragraph.
Context: ...eturn AWSCredentials with scoped root access Error Handling: - AccessDenied: ...
(PUNCTUATION_PARAGRAPH_END)
[grammar] ~552-~552: Please add a punctuation mark at the end of paragraph.
Context: ...sts.AssumeRoot(); if not, use raw API call ### Risk 2: Centralized Root Access Pr...
(PUNCTUATION_PARAGRAPH_END)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/aws-assume-root-identity.md
171-171: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Summary
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 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)
docs/prd/aws-assume-root-identity.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/prd/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Create PRD documentation in docs/prd/ using kebab-case filenames (e.g., command-registry-pattern.md, error-handling-strategy.md).
Files:
docs/prd/aws-assume-root-identity.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
📚 Learning: 2025-04-25T20:54:19.701Z
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
Applied to files:
docs/prd/aws-assume-root-identity.md
🪛 LanguageTool
docs/prd/aws-assume-root-identity.md
[typographical] ~147-~147: Consider using a typographic close quote here.
Context: ...ementation**: Implementation: - Add "aws/assume-root" case to `factory.NewId...
(EN_QUOTES)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
…cess
Implement the `aws/assume-root` identity kind that enables Atmos users
to leverage AWS's centralized root access feature via the `sts:AssumeRoot`
API. This allows organizations to perform privileged root-level operations
through a single permission set in the management account.
Key features:
- New identity kind `aws/assume-root` registered in factory
- Validation for 12-digit AWS account IDs
- Support for all 5 AWS-managed root task policies:
- IAMAuditRootUserCredentials
- IAMCreateRootUserPassword
- IAMDeleteRootUserCredentials
- S3UnlockBucketPolicy
- SQSUnlockQueuePolicy
- Duration capping at 900 seconds (AWS limit)
- Full credential lifecycle management
- Identity chaining from permission sets
- Shared STS client helper to reduce code duplication
Example configuration:
```yaml
auth:
identities:
core-audit/iam-audit-root-access:
kind: aws/assume-root
via:
identity: organizational-root-access
principal:
target_principal: "123456789012"
task_policy_arn: arn:aws:iam::aws:policy/root-task/IAMAuditRootUserCredentials
```
Ref: docs/prd/aws-assume-root-identity.md
🤖 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
pkg/auth/factory/factory.go(1 hunks)pkg/auth/factory/factory_test.go(1 hunks)pkg/auth/identities/aws/assume_root.go(1 hunks)pkg/auth/identities/aws/assume_root_test.go(1 hunks)pkg/auth/identities/aws/sts_client.go(1 hunks)pkg/auth/types/constants.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use functional options pattern to avoid functions with many parameters. Define Option functions that modify a Config struct and pass variadic options to New functions.
Use context.Context for cancellation signals, deadlines/timeouts, and request-scoped values (trace IDs). Context should be first parameter in functions that accept it. DO NOT use context for configuration, dependencies, or avoiding proper function parameters.
All comments must end with periods. This is enforced by the godot linter.
NEVER delete existing comments without a very strong reason. Preserve helpful comments, update them to match code changes, refactor for clarity, and add context when modifying code. Only remove factually incorrect, duplicated, or outdated comments.
Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
All errors MUST be wrapped using static errors defined in errors/errors.go. Use errors.Join for combining multiple errors, fmt.Errorf with %w for adding string context, error builder for complex errors, and errors.Is() for error checking. NEVER use dynamic errors directly.
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation. Never create manual mocks.
Use viper.BindEnv with ATMOS_ prefix for environment variables.
Use colors from pkg/ui/theme/colors.go for theme consistency.
Ensure Linux/macOS/Windows compatibility. Use SDKs over binaries. Use filepath.Join(), not hardcoded path separators.
Small focused files (<600 lines). One cmd/impl per file. Co-locate tests. Never use //revive:disable:file-length-limit.
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider us...
Files:
pkg/auth/factory/factory_test.gopkg/auth/identities/aws/sts_client.gopkg/auth/factory/factory.gopkg/auth/types/constants.gopkg/auth/identities/aws/assume_root.gopkg/auth/identities/aws/assume_root_test.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Prefer unit tests with mocks over integration tests. Use interfaces + dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen. Use table-driven tests for comprehensive coverage. Target >80% code coverage.
Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Files:
pkg/auth/factory/factory_test.gopkg/auth/identities/aws/assume_root_test.go
🧠 Learnings (3)
📓 Common learnings
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
pkg/auth/identities/aws/assume_root_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
pkg/auth/identities/aws/assume_root_test.go
🧬 Code graph analysis (3)
pkg/auth/identities/aws/sts_client.go (3)
pkg/auth/types/aws_credentials.go (1)
AWSCredentials(16-24)pkg/auth/cloud/aws/resolver.go (1)
GetResolverConfigOption(25-49)pkg/auth/cloud/aws/env.go (1)
LoadIsolatedAWSConfig(100-124)
pkg/auth/factory/factory.go (1)
pkg/auth/identities/aws/assume_root.go (1)
NewAssumeRootIdentity(55-72)
pkg/auth/identities/aws/assume_root_test.go (6)
pkg/auth/identities/aws/assume_root.go (3)
NewAssumeRootIdentity(55-72)IsSupportedTaskPolicy(514-521)GetSupportedTaskPolicies(524-528)pkg/schema/schema_auth.go (2)
Principal(69-72)IdentityVia(62-65)pkg/auth/types/aws_credentials.go (1)
AWSCredentials(16-24)pkg/auth/types/interfaces.go (1)
PostAuthenticateParams(71-78)pkg/auth/types/github_oidc_credentials.go (1)
OIDCCredentials(15-19)tests/preconditions.go (1)
RequireAWSProfile(67-85)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (18)
pkg/auth/factory/factory_test.go (1)
139-144: LGTM!The new test case for
aws/assume-rootfollows the established pattern and appropriately validates factory-level instantiation.pkg/auth/factory/factory.go (1)
56-57: LGTM!The factory integration for
aws/assume-rootis consistent with existing identity cases.pkg/auth/types/constants.go (1)
11-11: LGTM!The new constant follows established naming conventions and is appropriately placed within the AWS provider kinds group.
pkg/auth/identities/aws/assume_root.go (9)
54-72: LGTM!The constructor properly validates inputs and follows coding guidelines with appropriate error wrapping.
74-101: LGTM!The
Kind()andValidate()methods are well-structured with appropriate validation delegation and error handling.
103-162: LGTM!The validation helpers provide excellent error messages with actionable hints, following error handling best practices.
164-213: LGTM!The
Authenticatemethod follows a clear flow with comprehensive error handling and helpful diagnostic hints.
215-284: LGTM!The STS client creation, input building, and credential conversion logic is well-structured with appropriate error handling and AWS API compliance.
286-341: LGTM!The environment methods properly integrate AWS file-based configuration with identity-specific settings, following the established pattern.
343-387: LGTM!The provider resolution logic follows a clear hierarchy and provides appropriate fallbacks with helpful error messages.
389-511: LGTM!The lifecycle methods (
PostAuthenticate,CredentialsExist,LoadCredentials,Logout) are well-implemented with thorough validation and appropriate delegation to shared AWS helpers.
513-528: LGTM!The policy helper functions are straightforward and thread-safe, with
GetSupportedTaskPoliciesappropriately returning a defensive copy.pkg/auth/identities/aws/assume_root_test.go (5)
21-72: LGTM!The constructor tests provide solid coverage of validation paths with appropriate error checking.
74-253: LGTM!The validation tests are thorough, covering edge cases and ensuring proper field population and provider resolution.
255-420: LGTM!The input building and credential conversion tests thoroughly validate duration handling, nil guards, and region defaults.
422-711: LGTM!The environment, post-authentication, and authentication tests provide comprehensive coverage including error paths and region resolution precedence.
713-1038: LGTM!The credentials lifecycle, policy helper, and environment preparation tests provide excellent coverage with appropriate use of temporary directories and comprehensive validation of all supported task policies.
pkg/auth/identities/aws/sts_client.go (1)
44-51: Fix comment formatting.The comment on line 44 should end with a period per the godot linter requirement.
- // Load config with isolated environment to avoid conflicts with external AWS env vars + // Load config with isolated environment to avoid conflicts with external AWS env vars. cfg, err := awsCloud.LoadIsolatedAWSConfig(ctx, configOpts...)Likely an incorrect or invalid review comment.
aws/assume-root Identity Kind|
Warning Changelog Entry RequiredThis PR is labeled Action needed: Add a new blog post in Example filename: Alternatively: If this change doesn't require a changelog entry, remove the |
1 similar comment
|
Warning Changelog Entry RequiredThis PR is labeled Action needed: Add a new blog post in Example filename: Alternatively: If this change doesn't require a changelog entry, remove the |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
what
aws/assume-rootidentity kind for centralized root access to AWS member accountswhy
references
docs/prd/aws-assume-root-identity.mdSummary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.