Fix: Enable auto-import of provisioned identities from profiles
what
- Add
injectProvisionedIdentitiesPostLoad()function that runs after profiles are loaded, ensuringauth.providers(which may be defined in profiles) is available before checking for provisioned identity cache files - Add
processFileImportsIfPresent()helper to enableimport:directives in profile files (previously silently ignored) - Comprehensive test coverage for both provisioned identity injection and profile imports
why
Provisioned identities were not auto-importing when auth.providers was defined in a profile because the provider check happened during early config loading, before profiles were merged. Additionally, import: directives inside profile files were completely ignored. These changes fix both issues, allowing proper auto-discovery of provisioned identities from cache files.
references
Fixes provisioned identity auto-import for profiles with auto_provision_identities: true configured.
Summary by CodeRabbit
-
New Features
- UI/CLI now preserves and displays original-case identity names (display names) in lists and outputs.
- Imports are merged before their main files; main file still takes precedence.
-
Bug Fixes
- User-defined identities reliably override auto-provisioned ones by reapplying user config precedence.
- Provision/import errors are non-fatal to avoid load failures.
-
Tests
- Extensive new tests for provisioning, imports, case-preservation, precedence, and edge cases.
-
Chores
- Increased trace-level logging for import/provision actions.
βοΈ Tip: You can customize this high-level summary in your review settings.
Dependency Review
β No vulnerabilities or license issues found.Scanned Files
None
π Walkthrough
Walkthrough
This PR implements provisioned identity injection and case-preservation in the Atmos config loading pipeline. It adds logic to load provisioned identities from cache files after profiles are processed, re-applies user config for precedence, handles per-file imports during directory loading, and preserves original-case identity names via a new IdentityCaseMap. A GetIdentityDisplayName method is added to the AuthManager interface and threaded through auth formatters.
Changes
| Cohort / File(s) | Summary |
|---|---|
Config Loading Core pkg/config/load.go |
Modified execution flow to inject provisioned identities post-profile load, re-apply user config for precedence, and process per-file imports before merging main config. Increased trace-level logging and extended case-preservation to include provisioned identity data. |
Provisioned Identity Case Preservation pkg/config/provisioned_identity_case.go |
New file implementing discovery and preservation of original-case provisioned identity names from per-provider XDG cache files. Provides helper functions to read YAML, extract identities, and populate IdentityCaseMap. |
Config Loading Tests pkg/config/load_test.go |
Added extensive test suite covering provisioned identity injection scenarios, user config precedence override, case preservation across varied edge cases (no providers, multiple providers, cache issues), and config import/merge logic. |
Profile Tests pkg/config/profiles_test.go |
Added tests for import directives within profile files and the processFileImportsIfPresent helper. |
AuthManager Interface & Core pkg/auth/types/interfaces.go, pkg/auth/types/mock_interfaces.go, pkg/auth/manager.go |
Added public GetIdentityDisplayName method to AuthManager interface and manager implementation. Returns display name preserving original case from IdentityCaseMap or falls back to input key. Updated mock. |
Auth Manager Tests pkg/auth/manager_case_test.go, pkg/auth/providers/aws/saml_test.go |
Added test cases for provisioned identity case sensitivity and identity display name retrieval. Extended test stubs. |
Auth Formatter Updates pkg/auth/list/formatter_table.go, pkg/auth/list/formatter_tree.go |
Updated to use GetIdentityDisplayName for displaying identity names in table and tree formats, preserving original case. |
Auth Test Stubs cmd/auth_console_test.go, pkg/auth/hooks_test.go |
Added GetIdentityDisplayName method to mock AuthManager implementations. |
Terraform Output Utils internal/exec/terraform_output_utils.go |
Added GetIdentityDisplayName method to authContextWrapper. |
Golden Test Snapshot tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden |
Updated diagnostic logging output to reflect new provisioned identity injection flow, config precedence re-application, and expanded config discovery checks. |
Sequence Diagram
sequenceDiagram
participant User as User
participant CLI as CLI
participant ConfigLoader as Config Loader
participant ProfileMgr as Profile Manager
participant IdentityCache as Identity Cache
participant AuthMgr as Auth Manager
participant Formatter as Identity Formatter
User->>CLI: Load config
CLI->>ConfigLoader: LoadConfig()
ConfigLoader->>ProfileMgr: Load profiles
ProfileMgr-->>ConfigLoader: Merged profiles + user config
ConfigLoader->>IdentityCache: injectProvisionedIdentitiesPostLoad()
Note over IdentityCache: Read per-provider cache files<br/>from XDG directory
IdentityCache->>ConfigLoader: Populate IdentityCaseMap
ConfigLoader->>ConfigLoader: reapplyUserConfigForPrecedence()
Note over ConfigLoader: Re-merge to ensure<br/>user config takes precedence
ConfigLoader->>ConfigLoader: preserveProvisionedIdentityCase()
Note over ConfigLoader: Extract and preserve<br/>original-case identity names
ConfigLoader-->>CLI: Final AtmosConfiguration
CLI->>AuthMgr: ResolveIdentity(lowercase_name)
CLI->>AuthMgr: GetIdentityDisplayName(lowercase_name)
AuthMgr->>AuthMgr: Lookup in IdentityCaseMap
AuthMgr-->>CLI: Display name (original case)
CLI->>Formatter: Format identity list
Formatter->>AuthMgr: GetIdentityDisplayName()
AuthMgr-->>Formatter: Original-case names
Formatter-->>User: Display with preserved case
Estimated code review effort
π― 4 (Complex) | β±οΈ ~45 minutes
- Config loading flow logic: Review new provisioned identity injection, precedence re-application, and import processing order changes in load.go to ensure proper merge semantics and side-effect handling.
- Identity case preservation: Verify XDG cache file discovery, YAML parsing, and error handling in provisioned_identity_case.go; ensure user config takes precedence over provisioned mappings.
- Interface changes: Confirm GetIdentityDisplayName implementation across all AuthManager implementations (manager, mocks, stubs) and verify IdentityCaseMap threading through config/auth lifecycle.
- Test coverage: Scan test additions for correctness of edge cases (nil maps, missing files, multiple providers) and integration points between config loading and identity display.
- Golden snapshot changes: Verify new diagnostic logging reflects actual flow and is appropriate for trace-level verbosity.
Possibly related PRs
- #1684: Adds GetIdentityDisplayName and IdentityCaseMap handling to auth identity case/display-name surface.
- #1805: Touches config/auth loading surface with profile and identity loading concerns.
- #1742: Adds and propagates GetIdentityDisplayName behavior across auth types and formatters.
Suggested reviewers
- aknysh
- milldr
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 title directly and specifically describes the main change: enabling auto-import of provisioned identities from profiles, which aligns with the PR objectives. |
| Docstring Coverage | β Passed | Docstring coverage is 94.44% 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
osterman/fix-identity-import
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 help to get the list of available commands and usage tips.
π Walkthrough
Walkthrough
The changes extend the config loading pipeline with post-load provisioned identity injection, user config precedence reapplication, and import processing hooks. Functions enable provisioned identities to load after profiles, ensure user config overrides provisioned data, and process import directives within individual config files with non-fatal error handling.
Changes
| Cohort / File(s) | Summary |
|---|---|
Post-load provisioning and precedence logic pkg/config/load.go |
Introduces injectProvisionedIdentitiesPostLoad to load provisioned identity files after profile application; reapplyUserConfigForPrecedence to re-merge user config after provisioned identities; reapplyProfilesToEnforceConfigPrecedence to re-apply profiles; processFileImportsIfPresent to parse and apply import directives within individual files; updates loadAtmosConfigsFromDirectory and mergeDefaultImports to invoke import processing for each loaded file. All new steps use non-fatal, debug-level logging for failures. |
Load.go test coverage pkg/config/load_test.go |
Adds TestInjectProvisionedIdentitiesPostLoad (provider skipping, cache presence, identity loading from provisioned-identities.yaml, multiple provider handling) and TestInjectProvisionedIdentitiesPostLoad_ErrorHandling (invalid cache handling); adds TestUserConfigOverridesProvisionedIdentities to verify user config merging and precedence re-application override semantics. |
Profile import tests pkg/config/profiles_test.go |
Introduces TestProfileWithImports (import directives in profile files, value merging, override semantics) and TestProcessFileImportsIfPresent (import processing helper validation, missing import handling). Both construct temporary directories, write test files, and assert configuration outcomes. |
Sequence Diagram
sequenceDiagram
participant Client
participant LoadConfig
participant ProfileLoader
participant ProvisionedLoader
participant UserConfig
participant ImportProcessor
Client->>LoadConfig: LoadConfig()
LoadConfig->>ProfileLoader: Load & apply profiles
ProfileLoader-->>LoadConfig: Profiles applied
LoadConfig->>ProvisionedLoader: injectProvisionedIdentitiesPostLoad()
ProvisionedLoader->>ProvisionedLoader: Read provisioned-identities.yaml
ProvisionedLoader-->>LoadConfig: Identities injected (non-fatal)
LoadConfig->>ProfileLoader: reapplyProfilesToEnforceConfigPrecedence()
ProfileLoader-->>LoadConfig: Profiles re-applied
LoadConfig->>UserConfig: reapplyUserConfigForPrecedence()
UserConfig->>UserConfig: Re-merge main config file
UserConfig-->>LoadConfig: User config precedence restored
LoadConfig->>ImportProcessor: processFileImportsIfPresent()
ImportProcessor->>ImportProcessor: Parse & apply imports
ImportProcessor-->>LoadConfig: Imports processed (non-fatal)
LoadConfig-->>Client: Final merged config
Estimated code review effort
π― 4 (Complex) | β±οΈ ~45 minutes
- Precedence semantics: Verify that the order of operations (profiles β provisioned identities β user config re-application β imports) maintains correct override hierarchy across all scenarios
-
load.go integration points: Review how new functions hook into
loadAtmosConfigsFromDirectory,mergeDefaultImports, and the mainLoadConfigflow to ensure no race conditions or missing cases -
Error handling coverage: Confirm all non-fatal error paths (debug logging) are appropriate and don't mask critical failures; validate test coverage for error scenarios in
load_test.go -
Import processing logic: Assess
processFileImportsIfPresentcorrectness when called at multiple points in the loading pipeline
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 title accurately captures the main change: enabling auto-import of provisioned identities from profiles, which is central to the PR's objectives. |
| Docstring Coverage | β Passed | Docstring coverage is 93.33% 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
osterman/fix-identity-import
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 help to get the list of available commands and usage tips.
Codecov Report
:x: Patch coverage is 70.50360% with 41 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 73.48%. Comparing base (638d1b8) to head (42fbc93).
:warning: Report is 1 commits behind head on main.
:x: Your patch check has failed because the patch coverage (70.50%) 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 #1865 +/- ##
==========================================
+ Coverage 73.33% 73.48% +0.15%
==========================================
Files 564 566 +2
Lines 54336 54732 +396
==========================================
+ Hits 39847 40221 +374
Misses 11625 11625
- Partials 2864 2886 +22
| Flag | Coverage Ξ | |
|---|---|---|
| unittests | 73.48% <70.50%> (+0.15%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Files with missing lines | Coverage Ξ | |
|---|---|---|
| pkg/auth/list/formatter_tree.go | 86.61% <100.00%> (+0.14%) |
:arrow_up: |
| pkg/auth/manager.go | 85.76% <100.00%> (+0.14%) |
:arrow_up: |
| pkg/auth/list/formatter_table.go | 93.10% <50.00%> (-1.27%) |
:arrow_down: |
| internal/exec/terraform_output_utils.go | 68.04% <0.00%> (-0.54%) |
:arrow_down: |
| pkg/config/provisioned_identity_case.go | 89.79% <89.79%> (ΓΈ) |
|
| pkg/config/load.go | 75.72% <56.94%> (-2.55%) |
:arrow_down: |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
[!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.
π₯ This pull request now has conflicts. Could you fix it @osterman? π
π₯ This pull request now has conflicts. Could you fix it @osterman? π