Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for AWS shared credentials file #11614

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

brandond
Copy link
Member

@brandond brandond commented Jan 15, 2025

Proposed Changes

Add support for AWS shared credentials file

Also adds a CLI flag and fields for session token, which must be passed alongside the access key and secret when using temporary credentials. Ref: https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-envvars.html#envvars-list-AWS_SESSION_TOKEN

Types of Changes

enhancement

Verification

Use shared credentials file (~/.aws/credentials) for S3:

  1. Run aws configure as root
  2. Enter access and secret key
  3. Confirm that values are written to default profile in /root/.aws/credentials
  4. Start K3s and save snapshots to S3 - you will need to specify the bucket and folder but NOT the access or secret key.

Testing

Linked Issues

User-Facing Change

etcd snapshot backup/restore now supports loading s3 credentials from an AWS SDK shared credentials file.

Further Comments

@brandond brandond requested a review from a team as a code owner January 15, 2025 22:32
VestigeJ
VestigeJ previously approved these changes Jan 15, 2025
@brandond brandond force-pushed the etcd-s3-shared-creds branch from 0dbaea5 to 0de71f5 Compare January 15, 2025 23:05
@brandond brandond requested a review from VestigeJ January 15, 2025 23:09
@brandond brandond force-pushed the etcd-s3-shared-creds branch 2 times, most recently from 0e86ca3 to 390f773 Compare January 16, 2025 00:41
Copy link

codecov bot commented Jan 16, 2025

Codecov Report

Attention: Patch coverage is 88.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 47.66%. Comparing base (95700aa) to head (dc589a6).
Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
pkg/etcd/s3/s3.go 86.66% 1 Missing and 1 partial ⚠️
pkg/cli/server/server.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11614      +/-   ##
==========================================
- Coverage   49.90%   47.66%   -2.24%     
==========================================
  Files         185      185              
  Lines       19344    19353       +9     
==========================================
- Hits         9654     9225     -429     
- Misses       8298     8795     +497     
+ Partials     1392     1333      -59     
Flag Coverage Δ
e2etests 40.36% <0.00%> (-3.80%) ⬇️
inttests 35.04% <0.00%> (+0.03%) ⬆️
unittests 17.03% <88.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@brandond brandond requested a review from a team January 16, 2025 02:08
dereknola
dereknola previously approved these changes Jan 16, 2025
matttrach
matttrach previously approved these changes Jan 16, 2025
@brandond brandond dismissed stale reviews from matttrach and dereknola via f3806e9 January 27, 2025 19:01
@brandond brandond force-pushed the etcd-s3-shared-creds branch from 390f773 to f3806e9 Compare January 27, 2025 19:01
Also adds a CLI flag and fields for session token, which must be passed
alongside the access key and secret when using temporary credentials.

Signed-off-by: Brad Davidson <[email protected]>
@brandond brandond force-pushed the etcd-s3-shared-creds branch from f3806e9 to dc589a6 Compare January 27, 2025 19:03
@brandond brandond merged commit 0d028a2 into k3s-io:master Jan 29, 2025
39 checks passed
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.

5 participants