Repository navigation
feat(stores): add identity-based authentication for stores - #2099
Merged
Merged
Conversation
Stores can now reference an Atmos auth identity via a new `identity` field in the store configuration. When set, the store authenticates using that identity's credentials instead of the default credential chain. This works with AWS SSM Parameter Store, Azure Key Vault, and Google Secret Manager. Stores use lazy client initialization (sync.Once) so the cloud client is created on first access rather than at construction time, allowing auth to complete before store usage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
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. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Contributor
📝 WalkthroughWalkthroughAdds optional store-level Changes
Sequence DiagramsequenceDiagram
participant Client as Terraform/Client
participant Store as Identity-Aware Store
participant Resolver as AuthContextResolver
participant AuthMgr as AuthManager
participant Cloud as Cloud Provider
Client->>Store: Get/Set(key)
activate Store
Store->>Store: ensureClient()
alt Identity configured
Store->>Resolver: Resolve<Provider>AuthContext(identity)
activate Resolver
Resolver->>AuthMgr: Authenticate(identity)
activate AuthMgr
AuthMgr->>Cloud: Request credentials for identity
Cloud-->>AuthMgr: Temporary credentials
AuthMgr-->>Resolver: Auth context with credentials
deactivate AuthMgr
Resolver-->>Store: ProviderAuthConfig (creds, region, profile, etc.)
deactivate Resolver
Store->>Store: Create client with resolved credentials
else No identity
Store->>Store: Create client with default credential chain
end
Store->>Cloud: Access secret store
Cloud-->>Store: Secret/value
Store-->>Client: Return result
deactivate Store
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
pkg/store/authbridge/resolver_test.go (1)
248-265:TestResolver_NilStackInfoonly exercises the AWS path despite the comment.The comment on line 261 says "All resolve methods should return error when stackInfo is nil" but only
ResolveAWSAuthContextis called. Consider adding calls for Azure and GCP to match the stated intent — or adjust the comment.Also, the mock currently expects exactly one
Authenticatecall. If you add Azure/GCP checks, you'll needgomock.AnyTimes()or additional expectations.Proposed expansion
// All resolve methods should return error when stackInfo is nil. _, err := resolver.ResolveAWSAuthContext(context.Background(), "test-identity") assert.Error(t, err) assert.Contains(t, err.Error(), "AWS auth context not available") + + mockManager.EXPECT(). + Authenticate(gomock.Any(), "test-identity"). + Return(&types.WhoamiInfo{}, nil) + + _, err = resolver.ResolveAzureAuthContext(context.Background(), "test-identity") + assert.Error(t, err) + assert.Contains(t, err.Error(), "Azure auth context not available") + + mockManager.EXPECT(). + Authenticate(gomock.Any(), "test-identity"). + Return(&types.WhoamiInfo{}, nil) + + _, err = resolver.ResolveGCPAuthContext(context.Background(), "test-identity") + assert.Error(t, err) + assert.Contains(t, err.Error(), "GCP auth context not available")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/store/authbridge/resolver_test.go` around lines 248 - 265, TestResolver_NilStackInfo only calls ResolveAWSAuthContext but its comment claims "All resolve methods should return error when stackInfo is nil"; update the test to either call ResolveAzureAuthContext and ResolveGCPAuthContext as well (and assert errors containing the corresponding "auth context not available" messages) or change the comment to match the single-path test; if you add Azure/GCP calls also change the mock expectation on mockManager.Authenticate in TestResolver_NilStackInfo (or use gomock.AnyTimes()) so multiple Authenticate invocations are allowed; locate these symbols: TestResolver_NilStackInfo, ResolveAWSAuthContext, ResolveAzureAuthContext, ResolveGCPAuthContext, mockManager, and Authenticate to make the changes.pkg/store/identity_test.go (1)
13-40: Manual mock — consider usingmockgeninstead.The
mockAuthContextResolveris hand-rolled withtestify/mock. The coding guidelines state to usego.uber.org/mock/mockgenwith//go:generatedirectives and avoid manual mocks. SinceAuthContextResolveris an exported interface,mockgencan generate this cleanly.Not a blocker, but worth aligning with the rest of the codebase (e.g.,
resolver_test.goalready usesgomock).As per coding guidelines: "Use
go.uber.org/mock/mockgenwith//go:generatedirectives. Never manual mocks."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/store/identity_test.go` around lines 13 - 40, The tests currently use a hand-rolled mock type mockAuthContextResolver implementing AuthContextResolver; replace this manual mock with a mock generated by go.uber.org/mock/mockgen: add a //go:generate mockgen ... directive near the AuthContextResolver interface (or in pkg/store) to emit a generated mock (e.g., into pkg/store/mocks), run mockgen to produce the mock, update tests to import and use the generated mock type instead of mockAuthContextResolver, and remove the manual mockAuthContextResolver implementation and its ResolveAWSAuthContext/ResolveAzureAuthContext/ResolveGCPAuthContext methods from identity_test.go. Ensure the generated mock package is referenced in tests and the go:generate comment is committed so future regenerations are straightforward.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/store/aws_ssm_param_store.go`:
- Around line 182-199: The early-return in SSMStore.ensureClient (the if
s.client != nil) causes a data race with the write to s.client inside
s.initOnce.Do; remove that pre-check and always call s.initOnce.Do (using
s.initOnce.Do to perform s.initDefaultClient or s.initIdentityClient) so the
write happens under the Once synchronization and callers get the proper
happens-before guarantee; apply the same change to
AzureKeyVaultStore.ensureClient and GSMStore.ensureClient to eliminate the race
on their respective client fields and initOnce usage.
In `@pkg/store/azure_keyvault_store.go`:
- Around line 152-167: The early check reading s.client in
AzureKeyVaultStore.ensureClient introduces a data race; remove the top-level if
s.client != nil return nil and instead perform the s.client nil check inside the
initOnce.Do closure (alongside the existing logic that sets s.initErr via
initDefaultClient or initIdentityClient), so that initialization and the client
presence check are both performed under the sync.Once protection; keep returning
s.initErr after initOnce.Do as before.
In `@pkg/store/google_secret_manager_store.go`:
- Around line 133-182: ensureClient has an unsynchronized read of s.client (data
race) and inlines identity initialization; fix by making the nil check
thread-safe and moving identity setup into a new initIdentityClient method:
replace the top-level if s.client != nil return nil with a synchronized check
using s.initOnce (or re-check inside initOnce.Do) consistent with SSM/Azure
pattern, extract the block that handles s.identityName != "" (including
authResolver.ResolveGCPAuthContext, credential selection, clientOpts creation
and secretmanager.NewClient) into a new GSMStore.initIdentityClient() helper
that sets s.client and s.initErr, and have ensureClient call
s.initOnce.Do(func(){ if s.identityName=="" { s.initErr = s.initDefaultClient();
return } s.initErr = s.initIdentityClient() }) so identity path is no longer
inlined; keep existing cleanup/close logic and preserve initErr semantics.
In `@website/src/data/roadmap.js`:
- Line 167: Add the missing pr field to the milestone object with label
'Identity selection for stores' in the roadmap data: include pr: 2099 alongside
the existing keys (label, status, quarter, changelog, description, benefits) so
the milestone follows the same schema as other shipped entries (e.g., those with
pr: 2043, pr: 2051).
- Line 141: The roadmap entry's progress regressed from 85 to 83; update the
progress property (the object with "progress: 83") to a value ≥85 (suggest 88)
to reflect the newly shipped milestone so the initiative's progress only moves
forward.
---
Nitpick comments:
In `@pkg/store/authbridge/resolver_test.go`:
- Around line 248-265: TestResolver_NilStackInfo only calls
ResolveAWSAuthContext but its comment claims "All resolve methods should return
error when stackInfo is nil"; update the test to either call
ResolveAzureAuthContext and ResolveGCPAuthContext as well (and assert errors
containing the corresponding "auth context not available" messages) or change
the comment to match the single-path test; if you add Azure/GCP calls also
change the mock expectation on mockManager.Authenticate in
TestResolver_NilStackInfo (or use gomock.AnyTimes()) so multiple Authenticate
invocations are allowed; locate these symbols: TestResolver_NilStackInfo,
ResolveAWSAuthContext, ResolveAzureAuthContext, ResolveGCPAuthContext,
mockManager, and Authenticate to make the changes.
In `@pkg/store/identity_test.go`:
- Around line 13-40: The tests currently use a hand-rolled mock type
mockAuthContextResolver implementing AuthContextResolver; replace this manual
mock with a mock generated by go.uber.org/mock/mockgen: add a //go:generate
mockgen ... directive near the AuthContextResolver interface (or in pkg/store)
to emit a generated mock (e.g., into pkg/store/mocks), run mockgen to produce
the mock, update tests to import and use the generated mock type instead of
mockAuthContextResolver, and remove the manual mockAuthContextResolver
implementation and its
ResolveAWSAuthContext/ResolveAzureAuthContext/ResolveGCPAuthContext methods from
identity_test.go. Ensure the generated mock package is referenced in tests and
the go:generate comment is committed so future regenerations are
straightforward.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2099 +/- ##
==========================================
+ Coverage 76.19% 76.32% +0.12%
==========================================
Files 830 831 +1
Lines 78597 78802 +205
==========================================
+ Hits 59888 60143 +255
+ Misses 14964 14899 -65
- Partials 3745 3760 +15
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Replace hand-rolled testify/mock with gomock-generated MockAuthContextResolver per coding guidelines. Expand TestResolver_NilStackInfo to test all three cloud providers (AWS, Azure, GCP) instead of only the AWS path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d roadmap Move client-nil check inside initOnce.Do in all three store implementations (SSM, Azure, GSM) to eliminate potential data race between unsynchronized read and write from another goroutine. Bump auth initiative progress to 88% and add pr: 2099 to store identity milestone. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/store/identity_test.go (1)
104-129: Minor:ensureClienterror is silently discarded.Intentional per the comment (credential files don't exist in test), and the gomock expectation validates the resolver was called. Just noting this pattern — if future refactors make
ensureClientsucceed unexpectedly, the test won't catch regressions. Same applies to lines 400 and 425.Consider at minimum asserting
err != nilto confirm the expected failure path is exercised.Example
- _ = store.ensureClient() + err := store.ensureClient() + // Expected to fail because credential files don't exist in test environment. + assert.Error(t, err)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/store/identity_test.go` around lines 104 - 129, The test TestSSMStore_LazyInit_WithResolver currently discards the result of store.ensureClient(), which hides the expected failure when AWS credential files are missing; update the test to capture the returned error from SSMStore.ensureClient() and add an assertion that err != nil (or use require.Error/AssertError) so the test explicitly verifies the failure path and that the resolver was invoked; apply the same pattern to the other tests that call ensureClient() (the ones referenced around lines ~400 and ~425) to ensure they also assert the expected error outcome.pkg/store/aws_ssm_param_store_test.go (1)
627-692:TestSSMStore_BuildAuthConfigOptscovers the matrix well but only asserts slice length.The table-driven approach with 6 scenarios is solid. Consider also asserting specific option types or values in at least the "all fields populated" case to catch regressions where the right number of options is produced but with wrong content. Not critical though — length checks already catch most config wiring bugs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/store/aws_ssm_param_store_test.go` around lines 627 - 692, Test only checks the length of the options slice in TestSSMStore_BuildAuthConfigOpts which can miss incorrect option contents; update the test to also assert the actual option types/values for at least the "all fields populated" case by calling store.buildAuthConfigOpts with the AWSAuthConfig fixture and verifying that opts contains the expected option entries (credentials file, config file, profile and region) in whatever ordering buildAuthConfigOpts produces — use the AWSAuthConfig struct, the SSMStore.buildAuthConfigOpts call and inspect elements of opts to compare against expected values/types so regressions in option contents are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/store/google_secret_manager_store.go`:
- Around line 170-185: The check of s.client in GSMStore.ensureClient is done
unsafely outside the sync.Once, causing a data race; move the nil-check into the
initOnce.Do closure so the initialization (calling initDefaultClient or
initIdentityClient) and any writes to s.client happen only inside the Do, and
ensure ensureClient returns s.initErr after Do; specifically, update
GSMStore.ensureClient to call s.initOnce.Do(func(){ if s.client == nil { if
s.identityName == "" { s.initErr = s.initDefaultClient() } else { s.initErr =
s.initIdentityClient() } } }) and then return s.initErr, referencing
ensureClient, s.client, s.initOnce.Do, initDefaultClient and initIdentityClient.
---
Nitpick comments:
In `@pkg/store/aws_ssm_param_store_test.go`:
- Around line 627-692: Test only checks the length of the options slice in
TestSSMStore_BuildAuthConfigOpts which can miss incorrect option contents;
update the test to also assert the actual option types/values for at least the
"all fields populated" case by calling store.buildAuthConfigOpts with the
AWSAuthConfig fixture and verifying that opts contains the expected option
entries (credentials file, config file, profile and region) in whatever ordering
buildAuthConfigOpts produces — use the AWSAuthConfig struct, the
SSMStore.buildAuthConfigOpts call and inspect elements of opts to compare
against expected values/types so regressions in option contents are caught.
In `@pkg/store/identity_test.go`:
- Around line 104-129: The test TestSSMStore_LazyInit_WithResolver currently
discards the result of store.ensureClient(), which hides the expected failure
when AWS credential files are missing; update the test to capture the returned
error from SSMStore.ensureClient() and add an assertion that err != nil (or use
require.Error/AssertError) so the test explicitly verifies the failure path and
that the resolver was invoked; apply the same pattern to the other tests that
call ensureClient() (the ones referenced around lines ~400 and ~425) to ensure
they also assert the expected error outcome.
Add tests for eager-init constructors (no identity), ensureClient default-client path, identity client success with temp credential files, and Get/Set/GetKey ensureClient error guards across all three cloud stores. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Minimal example showing AWS SSM, Azure Key Vault, and GCP Secret Manager stores configured with Atmos auth identities. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Andriy Knysh (aknysh)
dismissed
coderabbitai[bot]’s stale review
via
February 22, 2026 18:02
cb55c22
Andriy Knysh (aknysh)
requested a review
from Erik Osterman (Cloud Posse) (osterman)
February 22, 2026 18:02
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (5)
examples/auth-stores/atmos.yaml (2)
32-35: Consider clarifying thegcp/adcidentity/provider symmetry.Both the
gcp-adcprovider (Line 17) and thegcp-prodidentity (Line 33) carrykind: gcp/adc. A brief comment here (e.g.,# ADC resolves credentials from the environment; via links the identity to the registered provider) would clarify to readers that this isn't redundant boilerplate but a deliberate schema requirement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/auth-stores/atmos.yaml` around lines 32 - 35, Add a short clarifying comment near the "gcp-prod" identity that explains why both the provider ("gcp-adc") and the identity ("gcp-prod") use kind: gcp/adc and what "via" does; specifically mention that ADC resolves credentials from the environment and that the "via" field links the identity to the registered provider (so this duplication is intentional schema wiring, not redundant boilerplate). Reference the provider name "gcp-adc", the identity "gcp-prod", the kind value "gcp/adc", and the "via" field so reviewers can locate where to add the comment.
1-57: Flag this example as intentionally minimal.The file has no
base_path,stacks, orcomponentsblock — required for a runnable Atmos project. A short comment up top noting that this is a config snippet (not a complete project) would save readers from chasing a "why doesn't this work" rabbit hole.💡 Suggested header clarification
# Demonstrates using Atmos auth identities with stores. # Each store references an identity for credential resolution. +# +# NOTE: This is an intentionally minimal snippet focused on auth + stores. +# A complete atmos.yaml also requires base_path, stacks, and components sections.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/auth-stores/atmos.yaml` around lines 1 - 57, Add a short top-line comment to clarify this file is an intentionally minimal config snippet (not a complete Atmos project) because the file contains auth:, providers:, identities:, and stores: entries but lacks required project-level blocks like base_path, stacks, or components; update the header to state it’s an example snippet for demonstrating identity-based stores so readers won’t expect a runnable project.pkg/store/aws_ssm_param_store.go (1)
170-177: Dead code:s.regionis guaranteed non-empty by the constructor.
NewSSMStorereturnsErrRegionRequiredwhenoptions.Region == ""(line 65-67), sos.regionis always non-empty. That meansregionat line 171 is never"", making the fallback at lines 172-174 unreachable.Not harmful — just defensive code that can't trigger. Up to you whether to trim it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/store/aws_ssm_param_store.go` around lines 170 - 177, Remove the dead fallback that checks s.region for emptiness since NewSSMStore enforces a non-empty region (ErrRegionRequired) so s.region is always set; in the AWS SSM store initialization replace the three-line branch that assigns region := s.region and then overrides it with authContext.Region only-if-empty with a single use of s.region (and still apply config.WithRegion(s.region) to cfgOpts) and delete the unreachable authContext.Region fallback logic to simplify code around cfgOpts and config.WithRegion.pkg/store/identity_test.go (1)
678-785: Consider consolidating these 9 repetitive guard tests into a table-driven test.Each SSM/Azure/GSM × Get/Set/GetKey test follows the exact same pattern: create a store with
identityNamebut no resolver, call the operation, assertErrIdentityNotConfigured. A single table-driven test would reduce ~100 lines to ~30 and make it trivial to add new store types later.Not urgent — the current form is clear and correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/store/identity_test.go` around lines 678 - 785, Replace the nine repetitive tests for SSMStore/AzureKeyVaultStore/GSMStore (TestSSMStore_Get_EnsureClientError, TestSSMStore_Set_EnsureClientError, TestSSMStore_GetKey_EnsureClientError, TestAzureKeyVaultStore_..., TestGSMStore_...) with a single table-driven test (e.g., TestStore_EnsureClientError_TableDriven) that iterates cases describing store constructor (returning initialized SSMStore/AzureKeyVaultStore/GSMStore with identityName:"broken"), the operation to call (Get, Set, GetKey) as a function/closure, and the expected error ErrIdentityNotConfigured; for each case construct the store via the factory, invoke the operation closure, and assert the returned error is non-nil and errors.Is(err, ErrIdentityNotConfigured) so adding new stores/ops only requires another table entry.pkg/store/azure_keyvault_store.go (1)
119-150: Consider adding a comment explaining whyinitIdentityClientuses onlyTenantID.The struct carries
CredentialsFile,SubscriptionID,UseOIDC,ClientID, andTokenFilePathto mirror schema.AzureAuthContext (per design pattern), but the store's Azure auth flow works through MSAL cache and environment variables set duringAuthenticate(), not through loading credential files like SSM and GSM do. A comment here would clarify thatDefaultAzureCredentialwith the tenant hint is sufficient because it checks the cached MSAL tokens and env vars already configured upstream.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/store/azure_keyvault_store.go` around lines 119 - 150, Add a short clarifying comment inside initIdentityClient explaining that only authContext.TenantID is applied to azidentity.DefaultAzureCredentialOptions because this store relies on the MSAL cache and environment variables (configured during Authenticate and exposed via authResolver.ResolveAzureAuthContext) rather than loading credential files or using SubscriptionID/ClientID/TokenFilePath directly; mention that DefaultAzureCredential will pick up cached tokens and env credentials so a tenant hint is sufficient before creating azidentity.NewDefaultAzureCredential and azsecrets.NewClient.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/store/aws_ssm_param_store.go`:
- Around line 186-199: The race condition has already been fixed by moving the
s.client != nil check inside ensureClient's s.initOnce.Do; no further changes
required — keep the current implementation of ensureClient (with s.initOnce.Do
wrapping the s.client check) and retain usage of s.initErr, initDefaultClient
and initIdentityClient as-is.
In `@pkg/store/azure_keyvault_store.go`:
- Around line 152-168: ensureClient already moves the client-nil check inside
initOnce.Do to avoid the data race; no code changes required. Keep the current
implementation of AzureKeyVaultStore.ensureClient which uses s.initOnce.Do,
checks s.client, and sets s.initErr via s.initDefaultClient or
s.initIdentityClient as appropriate.
In `@pkg/store/google_secret_manager_store.go`:
- Around line 170-186: Previous data-race fix looks applied: ensureClient moved
the client-nil check inside initOnce.Do and identity init was extracted; now
confirm and enforce that initDefaultClient and initIdentityClient both set
s.client and s.initErr (no other goroutine reads s.client unsafely), add a unit
test exercising the eager-init path where s.client is pre-set and a test for the
identity init path to verify initErr propagation, and ensure all accesses to
s.client outside ensureClient are synchronized or only read after ensureClient
returns; reference functions: ensureClient, initDefaultClient,
initIdentityClient.
In `@website/src/data/roadmap.js`:
- Line 141: The change to the roadmap entry (the progress property set to 88 and
the milestone including `pr: 2099` in website/src/data/roadmap.js) has addressed
prior review feedback; no code changes required—approve and merge the PR as-is,
ensuring the `progress: 88` value and the `pr: 2099` milestone remain in the
`roadmap` entry.
---
Nitpick comments:
In `@examples/auth-stores/atmos.yaml`:
- Around line 32-35: Add a short clarifying comment near the "gcp-prod" identity
that explains why both the provider ("gcp-adc") and the identity ("gcp-prod")
use kind: gcp/adc and what "via" does; specifically mention that ADC resolves
credentials from the environment and that the "via" field links the identity to
the registered provider (so this duplication is intentional schema wiring, not
redundant boilerplate). Reference the provider name "gcp-adc", the identity
"gcp-prod", the kind value "gcp/adc", and the "via" field so reviewers can
locate where to add the comment.
- Around line 1-57: Add a short top-line comment to clarify this file is an
intentionally minimal config snippet (not a complete Atmos project) because the
file contains auth:, providers:, identities:, and stores: entries but lacks
required project-level blocks like base_path, stacks, or components; update the
header to state it’s an example snippet for demonstrating identity-based stores
so readers won’t expect a runnable project.
In `@pkg/store/aws_ssm_param_store.go`:
- Around line 170-177: Remove the dead fallback that checks s.region for
emptiness since NewSSMStore enforces a non-empty region (ErrRegionRequired) so
s.region is always set; in the AWS SSM store initialization replace the
three-line branch that assigns region := s.region and then overrides it with
authContext.Region only-if-empty with a single use of s.region (and still apply
config.WithRegion(s.region) to cfgOpts) and delete the unreachable
authContext.Region fallback logic to simplify code around cfgOpts and
config.WithRegion.
In `@pkg/store/azure_keyvault_store.go`:
- Around line 119-150: Add a short clarifying comment inside initIdentityClient
explaining that only authContext.TenantID is applied to
azidentity.DefaultAzureCredentialOptions because this store relies on the MSAL
cache and environment variables (configured during Authenticate and exposed via
authResolver.ResolveAzureAuthContext) rather than loading credential files or
using SubscriptionID/ClientID/TokenFilePath directly; mention that
DefaultAzureCredential will pick up cached tokens and env credentials so a
tenant hint is sufficient before creating azidentity.NewDefaultAzureCredential
and azsecrets.NewClient.
In `@pkg/store/identity_test.go`:
- Around line 678-785: Replace the nine repetitive tests for
SSMStore/AzureKeyVaultStore/GSMStore (TestSSMStore_Get_EnsureClientError,
TestSSMStore_Set_EnsureClientError, TestSSMStore_GetKey_EnsureClientError,
TestAzureKeyVaultStore_..., TestGSMStore_...) with a single table-driven test
(e.g., TestStore_EnsureClientError_TableDriven) that iterates cases describing
store constructor (returning initialized SSMStore/AzureKeyVaultStore/GSMStore
with identityName:"broken"), the operation to call (Get, Set, GetKey) as a
function/closure, and the expected error ErrIdentityNotConfigured; for each case
construct the store via the factory, invoke the operation closure, and assert
the returned error is non-nil and errors.Is(err, ErrIdentityNotConfigured) so
adding new stores/ops only requires another table entry.
Erik Osterman (Cloud Posse) (osterman)
approved these changes
Feb 22, 2026
|
These changes were released in v1.208.0-rc.0. |
|
These changes were released in v1.208.0-test.15. |
This was referenced Apr 21, 2026
This was referenced Jun 6, 2026
This was referenced Jun 14, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
!storeYAML function) can now reference an Atmos auth identity via a newidentityfield in the store configurationidentityis set, the store authenticates using that identity's credentials instead of the default credential chain (environment variables, default AWS profiles, etc.)identityis set (unsupported since they don't map to cloud provider identity types)sync.Once) — the cloud client is created on firstGet/Setaccess rather than at construction timeidentityfield work exactly as beforewhy
atmos authidentity system for both, simplifying credential management and enabling more granular access controlreferences
docs/prd/store-identity-support.mdwebsite/blog/2026-02-22-store-identity-support.mdxwebsite/src/data/roadmap.jsexamples/auth-stores/Configuration example
Example
See
examples/auth-stores/for a complete multi-cloud configuration showing AWS SSM, Azure Key Vault, and GCP Secret Manager stores with identity-based authentication.Files changed
pkg/store/config.goIdentityfield toStoreConfigpkg/store/identity.goAuthContextResolver,IdentityAwareStore, local auth config typespkg/store/errors.goErrIdentityNotConfigured,ErrAuthContextNotAvailablepkg/store/aws_ssm_param_store.goSetAuthContext, identity-based AWS config loadingpkg/store/azure_keyvault_store.goSetAuthContext, identity-based credential creationpkg/store/google_secret_manager_store.goSetAuthContext, identity-based client creationpkg/store/registry.goSetAuthContextResolver()pkg/store/authbridge/resolver.gointernal/exec/terraform.go,terraform_shell.gopkg/store/identity_test.go,authbridge/resolver_test.go,registry_test.goexamples/auth-stores/Summary by CodeRabbit
Release Notes
New Features
identityfield in store configuration to specify which auth identity each store should use.Documentation