Skip to content

Migrate S3 clients to AWS SDK Go V2#4564

Merged
mye956 merged 1 commit into
aws:devfrom
mye956:migrate-s3
Apr 28, 2025
Merged

Migrate S3 clients to AWS SDK Go V2#4564
mye956 merged 1 commit into
aws:devfrom
mye956:migrate-s3

Conversation

@mye956
Copy link
Copy Markdown
Contributor

@mye956 mye956 commented Apr 9, 2025

Summary

This PR will migrate the S3 clients implementations and corresponding code paths from AWS SDK Go V1 to AWS SDK Go V2.

Implementation details

  • Now using LoadDefaultConfig fromNewConfig as part of V2 migration (ref) in createAWSConfig()
  • Now using s3.NewFromConfig() to create the S3 client
  • Now using NewDownloader from NewDownloaderWithClient for S3ManagerClient
  • Updated S3ManagerClient to now use Download(ctx context.Context, w io.WriterAt, input *s3.GetObjectInput, options ...func(*s3manager.Downloader)) from DownloadWithContext()
  • Updated the function parameters for GetObject() of S3Client interface
  • aws.*Value to aws.To* updates
  • Couple of other replacements of aws-sdk-go/* to aws-sdk-go-v2/*

Testing

Launched a task on a FIPs-enabled host in us-west-2

level=debug time=2025-04-23T20:12:01Z msg="FIPS mode detected, using virtual-host–style URLs for bucket location"
SDK 2025/04/23 20:12:01 DEBUG Request
level=debug time=2025-04-23T20:12:01Z msg="FIPS mode detected, using FIPS-compliant S3 endpoint in supported regions"

Environment variables within the container

/ # echo $TEST1
ABC
/ # echo $TEST2
XYZ

Launched a task on a FIPs-enabled host in a non-supported region ()

level=info time=2025-04-23T20:36:51Z msg="FIPS mode detected on the host"
level=debug time=2025-04-23T20:37:19Z msg="FIPS mode detected, using virtual-host–style URLs for bucket location"

Environment variables within the container

/ # echo $TEST1
ABC
/ # echo $TEST2
XYZ

Tested using dualstack subnet and confirmed the behavior remained the same

Host: blah-agent-test-files-us-west-2-prod.s3.dualstack.us-west-2.amazonaws.com

New tests cover the changes: yes

Description for the changelog

Enhancement - Migrate S3 clients to AWS SDK Go V2

Additional Information

Does this PR include breaking model changes? If so, Have you added transformation functions?

Does this PR include the addition of new environment variables in the README?

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mye956 mye956 force-pushed the migrate-s3 branch 3 times, most recently from 7de5ae6 to 5e4f26e Compare April 10, 2025 16:57
@mye956 mye956 force-pushed the migrate-s3 branch 2 times, most recently from e6b29b9 to 39ace36 Compare April 23, 2025 18:25
@mye956 mye956 marked this pull request as ready for review April 23, 2025 18:26
@mye956 mye956 requested a review from a team as a code owner April 23, 2025 18:26
@mye956 mye956 force-pushed the migrate-s3 branch 3 times, most recently from 644de0c to 9dd8fad Compare April 23, 2025 20:47
@mye956 mye956 force-pushed the migrate-s3 branch 4 times, most recently from 97786b1 to b6c0945 Compare April 24, 2025 23:08
@mye956 mye956 force-pushed the migrate-s3 branch 3 times, most recently from 05e2de2 to 5778ca7 Compare April 25, 2025 18:33
Comment thread agent/s3/factory/factory.go Outdated
Comment thread agent/s3/factory/factory.go Outdated
@mye956 mye956 changed the title WIP Migrate S3 clients to AWS SDK Go V2 Apr 25, 2025
@mye956 mye956 force-pushed the migrate-s3 branch 2 times, most recently from 960e5f9 to 78e1565 Compare April 25, 2025 21:26
ShelbyZ
ShelbyZ previously approved these changes Apr 25, 2025
Comment thread agent/s3/factory/factory.go
Comment thread agent/s3/factory/factory.go
Comment thread agent/s3/factory/factory_test.go Outdated
fixing mocks and tests

fixing more unit tests

testing with including the config opts as a list of funcs

changes for testing, reminder to remove/revert this commit

Revert "changes for testing, reminder to remove/revert this commit"

This reverts commit 265ed57.
@mye956 mye956 enabled auto-merge (rebase) April 28, 2025 20:57
@mye956 mye956 merged commit d9b0ae5 into aws:dev Apr 28, 2025
40 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.

4 participants