-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Enable sessionToken to be unset in S3Client
#16817
base: main
Are you sure you want to change the base?
Conversation
0d94d99
to
0a44a22
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one comment, will manually test and check how we can test this using CI
Co-authored-by: Ciro Spaciari <[email protected]>
acb28d6
to
8d3403a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for the test In test/js/bun/s3/s3.test.ts
we should be able to start a process passing the credentials and a invalid sessionToken in the env, using Bun.spawn, and write/read to the bucket cleaning the sessionToken parameter, check bunExe
, bunEnv
and allCredentials
, we can exit 1 for a error and exit 0 if everything is right, and check the exitCode
@cirospaciari I tried to write a test but I actually couldn't figure out how to get it to work. I might need some help finishing this PR. |
Sure will help you with that! |
What does this PR do?
Fixes #16810
This PR enables users to opt out of using the
AWS_SESSION_TOKEN
environment variable when using the S3Client. This is useful for users who are using Bun in an environment where theAWS_SESSION_TOKEN
environment variable is set, but they do not want to use it (eg AWS Lambda functions, or session token is being used for another purpose).WIth this change, users can opt out of using the
AWS_SESSION_TOKEN
(andS3_SESSION_TOKEN
) by passingnull
to thesessionToken
property in constructor.How did you verify your code works?