Skip to content

fix(store): resolve and refresh AWS role credentials - #61

Merged
yordis merged 1 commit into
mainfrom
yordis/feat-store-s3-role-credentials
Oct 31, 2026
Merged

yordis merged 1 commit into
mainfrom
yordis/feat-store-s3-role-credentials

Conversation

@yordis

@yordis yordis commented Oct 31, 2026

Copy link
Copy Markdown
Member
  • The S3 store refused to start without static access keys, so hosts running under an IAM role, IRSA or another AWS identity could not use S3 at all, while S3 is meant to be as first class as GCS.
  • Role credentials expire; the store has to refresh them through the provider instead of signing with a snapshot taken at startup.
  • A partial or empty explicit key pair must fail closed instead of quietly falling back to some other identity.
  • Brings in fix(store): resolve and refresh AWS role credentials tobi/walgit#60 by Tobi Lütke, cherry-picked with original authorship; adapted so the mutation and read clients share one credential cache and exchange role credentials once.

@cursor

cursor Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes object-store authentication and startup failure modes for all S3 deployments; behavior is well-tested but misconfigured env could block startup or deny bucket access until fixed.

Overview
S3 no longer requires static access keys at startup. When both configured key env vars are unset, S3Store uses the AWS default credential chain (profiles, web identity / IRSA, container and instance roles) and refreshes expiring role credentials through the SDK instead of freezing keys from env at boot.

Explicit keys still win, but partial config fails closed. Non-empty access_key_env / secret_key_env pairs (plus optional AWS_SESSION_TOKEN) override the chain; a half-set or empty pair errors instead of silently falling back to another identity. The chain is probed once during S3Store::new (now async) so hosts with no resolvable identity fail at startup, not on the first bucket call. Mutation and read S3 clients share one lazy identity cache so role exchanges are not duplicated.

Docs (README, walgit.example.toml) describe the new behavior. s3_credentials.rs adds hermetic subprocess tests against fake STS/S3 for IRSA refresh, static override, invalid partial keys, and missing identity; the S3 contract test awaits the async constructor.

Reviewed by Cursor Bugbot for commit 0f43a99. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 47 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5d60a991-d386-405b-bfe0-690a4e16c8f5
Commits

Reviewing files that changed from the base of the PR and between bc97170 and 0f43a99.

Files selected for processing: 6
  • README.md
  • crates/walgit-store/src/lib.rs
  • crates/walgit-store/src/s3.rs
  • crates/walgit-store/tests/contract.rs
  • crates/walgit-store/tests/s3_credentials.rs
  • walgit.example.toml
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@yordis yordis closed this Oct 31, 2026
@yordis yordis reopened this Oct 31, 2026
@yordis
yordis force-pushed the yordis/feat-store-s3-role-credentials branch from f7344b7 to 0cdf9bd Compare October 31, 2026 16:42
Adapt PR #48 onto conditional storage fixes. Keep explicit credentials fail-closed; retain the provider for automatic refresh. Synthetic STS/S3 credential tests: 5 passed.

The mutation and read clients share one resolver and identity cache so role credentials are exchanged and refreshed once, not once per client.

(cherry picked from commit 0372cb9)
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis force-pushed the yordis/feat-store-s3-role-credentials branch from 0cdf9bd to 0f43a99 Compare October 31, 2026 16:54
@yordis
yordis merged commit d71870e into main Oct 31, 2026
7 checks passed
@yordis
yordis deleted the yordis/feat-store-s3-role-credentials branch October 31, 2026 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants