fix: resolve S3 credentials via AWS default chain when static keys are unset (#9429)#9430
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
2 issues found across 12 files
Confidence score: 3/5
- There is a concrete regression risk in
.specify/feature.json:feature_directorypoints tospecs/...instead of the checked-indev/specs/..., which can break downstream spec-tool resolution for this feature. - The issue in
dev/specs/ifc-2683-fix-s3-irsa-credentials/plan.mdis low severity (documentation accuracy), but the incorrect Pydantic version can still mislead implementation and validation choices if left uncorrected. - Given one medium-severity, high-confidence tooling-path break plus a minor documentation mismatch, this looks mergeable after a small fix but carries some user-impacting risk as-is.
- Pay close attention to
.specify/feature.json,dev/specs/ifc-2683-fix-s3-irsa-credentials/plan.md- fix the spec directory path and align the documented Pydantic version.
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
ajtmccarty
reviewed
Jun 2, 2026
ajtmccarty
left a comment
Contributor
There was a problem hiding this comment.
I'd say that the ~500 lines of spec/plan/etc for this pretty small code change is excessive and can be left out. But I don't know if we've really agreed on a limit or border for including these specs or not
…e unset (#9429) S3StorageSettings defaults the access key and secret to empty strings, and the storage layer forwarded them straight to boto3. boto3 treats an empty string as an explicit credential and skips the default credential provider chain, so S3 storage failed with AuthorizationHeaderMalformed on deployments relying on IRSA, EC2 instance profiles, or ECS task roles. InfrahubS3ObjectStorage now treats blank, whitespace-only, or incomplete static credentials as "not configured" and passes None, letting boto3 fall back to the default AWS credential provider chain. Complete static credentials are forwarded unchanged, so existing static-key and S3-compatible deployments are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
f02542d to
b9d289b
Compare
Contributor
Author
|
Pushed an update addressing the review:
Force-pushed to rewrite the single commit so the spec files are absent from history, not just deleted in a follow-up. |
ajtmccarty
approved these changes
Jun 3, 2026
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
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.
Summary
Fixes #9429. S3 object storage failed with
AuthorizationHeaderMalformed: a non-empty Access Key (AKID) must be providedon deployments that rely on AWS role-based credentials (IRSA, EC2 instance profiles, ECS task roles) instead of static keys.S3StorageSettingsdefaultsaccess_key_id/secret_access_keyto empty strings, and the storage layer forwarded those empty strings straight toboto3.resource(...). boto3 treats an empty string as an explicit credential and skips the default credential provider chain, so role-based credential resolution never happened.Change
InfrahubS3ObjectStorage.__init__now treats blank, whitespace-only, or incomplete static credentials as "not configured" and passesNone, letting boto3 fall back to the default AWS credential provider chain. Complete static credentials are forwarded unchanged, so existing static-key and S3-compatible (e.g. MinIO) deployments are unaffected.The fix lives entirely in Infrahub's subclass — the upstream
fastapi_storageslibrary is not modified.Behavior
None→ default credential chainNone→ default credential chainTests
New
backend/tests/unit/test_storage.pystubsboto3.resourceand asserts the credential kwargs across all four states (5 tests, all passing). No external services required.Verification not covered by CI
The end-to-end IRSA path (removing the
sitecustomize.pyworkaround on a live EKS deployment) can only be confirmed on a role-based deployment — seedev/specs/ifc-2683-fix-s3-irsa-credentials/quickstart.md§4.Notes
# type: ignore[assignment]are introduced (one per credential attribute):fastapi_storages.S3Storagetypes them asstr, but boto3 requiresNoneto trigger credential-chain fallback. Justified inline.changelog/9429.fixed.md.dev/specs/ifc-2683-fix-s3-irsa-credentials/.🤖 Generated with Claude Code
Summary by cubic
Fix S3 storage auth for AWS role-based credentials by letting
boto3use the default credential chain when both static keys are unset. Partial static credentials now raise a clear error. Addresses Linear IFC-2683.InfrahubS3ObjectStorage.__init__, treat blank/whitespace-only keys as not configured and set them toNonesoboto3.resource(...)resolves IRSA/instance-profile/task-role credentials.ValueErrorexplaining both or neither must be set.Written for commit 732e394. Summary will update on new commits.