Repository navigation
fix(hooks): store-output hooks inherit the run's default identity - #2662
Merged
Merged
Conversation
The after-apply `store-outputs` hook resolves store credentials on a separate, freshly-loaded config from the main terraform path. It injected the store auth resolver but no default identity, so identity-less stores fell back to the default AWS SDK credential chain — empty under Atmos auth — and failed with "no EC2 IMDS role found". The main terraform path already inherits the run's identity; this brings the hook path in line (the follow-up deferred in #2625). - internal/exec: add HookStoreDefaultIdentity, mirroring the main path (auto-detect the active identity from the auth manager chain, normalize empty/select/disabled to ""). Returns "" with no auth manager, so runs without Atmos auth keep the prior ambient/default-credential behavior. - cmd/terraform: injectHookStoreAuthResolver now injects the resolver WITH the default identity via SetAuthContextResolverWithDefaultIdentity. - pkg/store: defaultIdentityForStore was missing *SecretsManagerStore (aws/asm), so AWS Secrets Manager stores never inherited a default identity on any path. Add the case so aws/asm behaves like aws/ssm. Tests: internal/exec.TestHookStoreDefaultIdentity (new); cmd/terraform TestInjectHookStoreAuthResolver_InheritsDefaultIdentity (replaces _ResolverOnly); pkg/store default-identity test updated so identity-less aws/asm asserts inheritance; Floci E2E TestAWSStoreHooks_InheritedIdentity_FlociE2E with new fixture aws-store-hooks-floci-inherit (no per-store identity, ambient creds cleared, hook write succeeds). Fix doc under docs/fixes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAfter-apply ChangesHook Store Identity Inheritance
Sequence DiagramsequenceDiagram
participant Terraform as Terraform apply
participant Hook as injectHookStoreAuthResolver
participant Helper as HookStoreDefaultIdentity
participant AuthMgr as authManager
participant Store as SSMStore / SecretsManagerStore
Terraform->>Hook: store-outputs hook setup
Hook->>Helper: authManager, info
Helper->>AuthMgr: GetChain() when identity is empty
AuthMgr-->>Helper: chain leaf identity
Helper-->>Hook: defaultIdentity
Hook->>Store: SetAuthContextResolverWithDefaultIdentity(resolver, defaultIdentity)
Store-->>Terraform: write outputs with inherited identity
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 4
🤖 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 `@cmd/terraform/utils_hooks_test.go`:
- Around line 777-783: Update the explanatory note in the `utils_hooks_test.go`
test to include Secrets Manager in the list of concrete store types handled by
`defaultIdentityForStore`, since `SetAuthContext` now applies the default to
`*SecretsManagerStore` as well. Keep the rest of the seam-behavior explanation
intact, but revise the comment around the test that mentions `GetChain`,
`info.Identity`, and resolver wiring so it accurately reflects the current store
coverage.
- Around line 788-789: The struct field comments in the test helper type need to
follow the Go comment style rule by ending with periods. Update the comments on
the fields in the relevant struct near the chain and expectedIdentity
declarations so they each end with a period, keeping the rest of the wording
unchanged.
In `@internal/exec/utils_auth_test.go`:
- Line 451: The new inline comments in utils_auth_test.go need trailing periods
to satisfy the Go comment rule enforced by godot. Update the affected comment
text near expectedIdentity and the other matching inline comments in the same
test file so each ends with a period, keeping the existing meaning and placement
intact.
In `@tests/fixtures/scenarios/aws-store-hooks-floci-inherit/atmos.yaml`:
- Around line 48-50: The fixture is using a Unix-specific log sink via logs.file
in the atmos.yaml scenario, which breaks cross-platform execution before the
hook path runs. Update the aws-store-hooks-floci-inherit scenario config by
removing the logs.file entry or replacing it with a portable sink that the test
harness sets up, while keeping the logs.level setting intact. Use the logs block
in this fixture as the target for the change.
🪄 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: CHILL
Plan: Pro
Run ID: bc8ec8e8-8388-4f09-ba61-15d2ea2e4fff
📒 Files selected for processing (13)
cmd/terraform/utils.gocmd/terraform/utils_hooks_test.godocs/fixes/2026-06-27-store-hook-inherit-default-identity.mdinternal/exec/terraform_execute_helpers.gointernal/exec/utils_auth_test.gopkg/store/identity_test.gopkg/store/registry.gotests/aws_store_hooks_inherit_floci_test.gotests/fixtures/scenarios/aws-store-hooks-floci-inherit/atmos.yamltests/fixtures/scenarios/aws-store-hooks-floci-inherit/components/terraform/output-demo/main.tftests/fixtures/scenarios/aws-store-hooks-floci-inherit/components/terraform/output-demo/outputs.tftests/fixtures/scenarios/aws-store-hooks-floci-inherit/stacks/consumer.yamltests/fixtures/scenarios/aws-store-hooks-floci-inherit/stacks/producer.yaml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2662 +/- ##
==========================================
+ Coverage 80.44% 80.46% +0.02%
==========================================
Files 1444 1444
Lines 134759 134768 +9
==========================================
+ Hits 108406 108444 +38
+ Misses 20348 20317 -31
- Partials 6005 6007 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Andriy Knysh (aknysh)
requested a review
from Erik Osterman (Cloud Posse) (osterman)
June 27, 2026 20:41
Erik Osterman (Cloud Posse) (osterman)
approved these changes
Jun 28, 2026
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
|
These changes were released in v1.222.0-rc.12. |
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
store-outputshook path inherit the run's auto-detected identity forstores that don't declare their own
identity, matching the main terraform path.internal/exec.HookStoreDefaultIdentityhelper (auto-detect the active identity from theauth manager's chain, normalize empty/
select/disabledto"");cmd/terraform'sinjectHookStoreAuthResolvernow callsSetAuthContextResolverWithDefaultIdentityinstead of theresolver-only variant.
pkg/store.defaultIdentityForStorewas missing*SecretsManagerStore(
aws/asm), so AWS Secrets Manager stores never inherited a default identity on any path. Addedthe case so
aws/asmbehaves likeaws/ssm.internal/exec.TestHookStoreDefaultIdentity(new),cmd/terraformTestInjectHookStoreAuthResolver_InheritsDefaultIdentity(replaces…_ResolverOnly), updatedpkg/storedefault-identity test so identity-lessaws/asmasserts inheritance, and Floci E2ETestAWSStoreHooks_InheritedIdentity_FlociE2Ewith fixtureaws-store-hooks-floci-inherit.docs/fixes/2026-06-27-store-hook-inherit-default-identity.md.why
Hook fix. Under Atmos auth,
atmos terraform applyon a component with astore-outputshookapplied successfully but then failed in the hook when the target store had no
identity:Hooks run in a freshly-loaded config, so the apply-phase store registry (and its injected default
identity) is gone. The hook re-injected the resolver but no default identity, so identity-less
stores fell back to the default AWS SDK credential chain — empty under Atmos auth (credentials live
in the keyring, not the environment) — and dropped to EC2 IMDS. The main terraform path and
!storereads already inherit the run's identity; this removes a surprising asymmetry and completes the
follow-up explicitly deferred in Fix AWS store auth and add Floci E2E coverage #2625 ("Component-identity inheritance for identity-less stores is
intentionally left for a follow-up design decision").
ASM fix.
defaultIdentityForStorehandled*SSMStore,*AzureKeyVaultStore, and*GSMStorebut not
*SecretsManagerStore, soaws/asmstores without an explicitidentitycould neverinherit one. This was latent before (and was even codified by the old test); the hook fix's E2E
surfaced it.
Backward compatible.
HookStoreDefaultIdentityreturns""whenever no identity is resolved(no auth manager, or empty/
select/disabled), andSetAuthContextResolverWithDefaultIdentity("")is a no-op for the default — so runs without Atmos auth keep their prior ambient/default-SDK
credential behavior, and stores with an explicit
identityare never overridden.references
docs/fixes/2026-06-17-aws-stores-secrets-auth-and-gists.md,docs/fixes/2026-05-25-store-hook-missing-backend-role-assumption.md.