Skip to content

s3: migrate to aws-sdk-go-v2 #516

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

Closed

Conversation

bschaatsbergen
Copy link

@bschaatsbergen bschaatsbergen commented Feb 12, 2025

This proposes updating the S3 getter to use the new aws-sdk-go-v2 to take advantage of improved configuration loading and credential chain handling, including support for SSO-fetched credentials, for example.

It’s worth noting that the aws-sdk-go-v2 requires at least Go 1.21, so this PR adds the minimum required version to the go.mod file.

Additionally, this updates the actions/upload-artifact action to the latest stable version, as the currently implemented v3 is no longer be usable after January 30th, 2025. See deprecation notice.

This is a draft since I’m still working out the details of how the EC2 Metadata Service URL overriding should be handled. If you have any suggestions, feel free to comment them here.

@bschaatsbergen bschaatsbergen marked this pull request as ready for review February 12, 2025 19:52
@bschaatsbergen bschaatsbergen requested a review from a team as a code owner February 12, 2025 19:52
@bschaatsbergen bschaatsbergen marked this pull request as draft February 12, 2025 19:53
@crw
Copy link
Contributor

crw commented Feb 12, 2025

@bschaatsbergen you probably know this already but be aware you will need a @hashicorp/team-ip-compliance review on this PR before it can be merged. For when that time comes.

cfg, err = config.LoadDefaultConfig(
ctx,
config.WithRegion(region),
config.WithEC2IMDSEndpoint(os.Getenv("AWS_METADATA_URL")),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s unclear to me if this is the right approach and whether it provides feature parity with the current implementation of metadata URL overriding. Something to look into.

@SarahFrench
Copy link
Member

👋🏻 Hi Bruno - you may have already spotted it, but there's an old PR for the same migration that might be a useful reference: #467

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.

3 participants