feat(sso): separate SSO portal region from resource region - #232
Conversation
SSO contexts conflated the IAM Identity Center portal region with the resource query region into a single `region` field, so an SSO login in one region (e.g. us-east-1) forced all resource browsing into that same region even when resources live elsewhere (e.g. ap-northeast-2). Add an optional `sso_region` context field. SSO login, GetRoleCredentials, and account/role listing use `sso_region`; all resource clients use `region`. `sso_region` falls back to `region` when unset, so existing single-region configs are unchanged. Closes DevopsArtFactory#231 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015fYsfxB3atyD47s3JqKYPT
WalkthroughChangesSSO region separation
Sequence Diagram(s)sequenceDiagram
participant ContextConfig
participant SSOService
participant AWSIdentityCenter
participant ResourceClients
ContextConfig->>SSOService: provide Region and EffectiveSSORegion()
SSOService->>AWSIdentityCenter: use effective SSO region
AWSIdentityCenter-->>SSOService: return SSO credentials
SSOService->>ResourceClients: return config with resource region
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 174-181: Add the required profile field to the SSO example
configuration near the existing auth_type and SSO settings, using a concrete
profile value consistent with the example so it matches the mandatory fields
table and implemented configuration behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 408efb5d-25ec-463f-9b0a-a00478b588d8
📒 Files selected for processing (9)
README.mddocs/architecture.en.mddocs/architecture.ko.mdinternal/app/context_add.gointernal/auth/setup.gointernal/config/config.gointernal/config/config_test.gointernal/services/aws/sso.gointernal/services/aws/sso_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
docs/**
⚙️ CodeRabbit configuration file
docs/**: Documentation must match implemented behavior. When both English and
Korean docs are updated, verify that they preserve the same meaning.
Files:
docs/architecture.en.mddocs/architecture.ko.md
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use lipgloss for styled TUI output — column-aligned tables with dimmed labels in Go implementation files
Implement scroll windowing with formula:visibleLines := max(m.height-N, 5)in Go TUI implementation
Files:
internal/app/context_add.gointernal/config/config_test.gointernal/services/aws/sso.gointernal/auth/setup.gointernal/services/aws/sso_test.gointernal/config/config.go
⚙️ CodeRabbit configuration file
**/*.go: For Go reviews, look beyond compilation and prioritize nil pointer risks,
context propagation, AWS SDK pagination, error wrapping, deterministic
sorting, and stable table/detail rendering. For new AWS service work,
verify that repository interfaces, model mapping, app integration, and
tests are updated together.
Files:
internal/app/context_add.gointernal/config/config_test.gointernal/services/aws/sso.gointernal/auth/setup.gointernal/services/aws/sso_test.gointernal/config/config.go
internal/app/**
⚙️ CodeRabbit configuration file
internal/app/**: For Bubble Tea screen changes, verify message routing, key handling,
filter target resets, height-based windowing, help text, and back/home
navigation against the existing screen patterns.
Files:
internal/app/context_add.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Tests use mock client interfaces (see
rds_test.gopattern) in Go test files
Files:
internal/config/config_test.gointernal/services/aws/sso_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Check that tests cover API errors, mapping edge cases, and navigation
state transitions, not only happy paths. Prefer mock-based tests that do
not depend on external AWS calls.
Files:
internal/config/config_test.gointernal/services/aws/sso_test.go
README.md
📄 CodeRabbit inference engine (CLAUDE.md)
README.md: When adding, modifying, or deleting features, always updateREADME.mdin parallel with code changes
UpdateCurrently Implemented Featurestable in README.md: add new services/features, update status changes (🚧→✅), remove deleted items
UpdateTUI Key Bindingstable in README.md when key bindings are added, changed, or deleted
UpdateUsagesection in README.md when new CLI commands or flags are added
UpdateConfigurationsection in README.md when configuration format changes
Files:
README.md
⚙️ CodeRabbit configuration file
README.md: Verify that README changes match actual CLI/TUI behavior and that
Currently Implemented Features, TUI Key Bindings, Usage, and
Configuration content stay aligned with code changes.
Files:
README.md
internal/services/aws/**
⚙️ CodeRabbit configuration file
internal/services/aws/**: For AWS integration code, focus on SDK client interface mockability,
paginator usage, nil/empty response handling, AWS pointer conversion,
stable list ordering, and user-facing error messages.
Files:
internal/services/aws/sso.gointernal/services/aws/sso_test.go
🔇 Additional comments (9)
README.md (1)
210-219: LGTM!docs/architecture.en.md (1)
196-197: LGTM!docs/architecture.ko.md (1)
196-197: LGTM!internal/config/config.go (1)
72-72: LGTM!Also applies to: 88-106, 135-135, 175-223, 271-271, 343-343
internal/app/context_add.go (1)
24-25: LGTM!Also applies to: 139-139
internal/auth/setup.go (1)
321-331: LGTM!Also applies to: 349-349
internal/config/config_test.go (1)
243-276: LGTM!internal/services/aws/sso.go (1)
77-101: LGTM!Also applies to: 199-199, 240-240, 327-327
internal/services/aws/sso_test.go (1)
10-10: LGTM!Also applies to: 147-198
| - name: seoul-sso-123456789012-admin | ||
| region: ap-northeast-2 # where resources are queried | ||
| sso_region: us-east-1 # where the SSO/IAM Identity Center portal lives | ||
| auth_type: sso | ||
| sso_start_url: https://example.awsapps.com/start | ||
| sso_account_id: "123456789012" | ||
| sso_role_name: Admin | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add the required profile field to the SSO example.
The example omits profile, but the sso required-fields table at Line [208] lists it as mandatory. Add the profile field so users can copy a valid concrete SSO context.
As per path instructions, README content must match implemented behavior and configuration content must stay aligned with code changes.
Proposed fix
- name: seoul-sso-123456789012-admin
+ profile: my-sso-profile
region: ap-northeast-2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: seoul-sso-123456789012-admin | |
| region: ap-northeast-2 # where resources are queried | |
| sso_region: us-east-1 # where the SSO/IAM Identity Center portal lives | |
| auth_type: sso | |
| sso_start_url: https://example.awsapps.com/start | |
| sso_account_id: "123456789012" | |
| sso_role_name: Admin | |
| - name: seoul-sso-123456789012-admin | |
| profile: my-sso-profile | |
| region: ap-northeast-2 # where resources are queried | |
| sso_region: us-east-1 # where the SSO/IAM Identity Center portal lives | |
| auth_type: sso | |
| sso_start_url: https://example.awsapps.com/start | |
| sso_account_id: "123456789012" | |
| sso_role_name: Admin |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 174 - 181, Add the required profile field to the SSO
example configuration near the existing auth_type and SSO settings, using a
concrete profile value consistent with the example so it matches the mandatory
fields table and implemented configuration behavior.
Source: Path instructions
2 participants
Summary
SSO contexts conflated the IAM Identity Center portal region with the resource query region into a single
regionfield. If SSO login lived in one region (e.g.us-east-1) but resources lived in another (e.g.ap-northeast-2), every resource browse was forced into the login region — there was no way to split them.This adds an optional
sso_regioncontext field. SSO login,GetRoleCredentials, and account/role listing usesso_region; all resource clients useregion. Whensso_regionis unset it falls back toregion, so existing single-region configs are unchanged.Closes #231
Changes
internal/config/config.go: addSSORegiontoContextEntry/Config/ContextInfo, populate in all three load paths, and addEffectiveSSORegion()(falls back toRegion).internal/services/aws/sso.go:resolveSSOCredentialsbuilds the SSO client withEffectiveSSORegion()and returns a config whose region is the resourceRegion;ListSSOAccounts/ListSSOAccountRolesuseEffectiveSSORegion();BuildSSOLoginCmdwritessso_regionandregionseparately.internal/auth/setup.go: carrySSORegionthrough SSO context setup and dedup matching.internal/app/context_add.go: add optionalsso_regionto the interactive "Add Context" (SSO) flow; relabelregionas "Region (resources)".config_test.go), login-command region split + fallback (sso_test.go).docs/architecture.{en,ko}.md.Resulting config
Testing
Local Go toolchain was unavailable in the authoring environment, so build/tests were not run locally — CI (
go test ./...) covers build + tests. Backward compatibility is preserved via thesso_region→regionfallback.🤖 Generated with Claude Code
https://claude.ai/code/session_015fYsfxB3atyD47s3JqKYPT
Summary by CodeRabbit
New Features
sso_regionsetting for SSO login, account and role discovery, and credential retrieval.sso_regionis not specified, the resource region is used automatically.Documentation