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

feat: Migrate S3 backend to AWS SDK for Go v2 #33469

Closed
wants to merge 2 commits into from

Conversation

siketyan
Copy link

@siketyan siketyan commented Jul 3, 2023

Fixes #30443
Fixes #32448
Fixes #32465

Since this will be my first-time contribution for Terraform, please let me know if I am missing something to do, or there are some reason(s) that are blocking migration of aws-sdk-go.

Target Release

1.6.x?

Draft CHANGELOG entry

ENHANCEMENTS

  • Internally migrated S3 remote state backend to AWS SDK for Go v2.

@hashicorp-cla
Copy link

hashicorp-cla commented Jul 3, 2023

CLA assistant check
All committers have signed the CLA.

@siketyan siketyan force-pushed the feat/aws-sdk-go-v2 branch from 31ad884 to 20b9ec2 Compare July 3, 2023 14:50
Signed-off-by: Natsuki Ikeguchi <me@s6n.jp>
@siketyan siketyan force-pushed the feat/aws-sdk-go-v2 branch from 20b9ec2 to 84b757d Compare July 3, 2023 14:51
Signed-off-by: Natsuki Ikeguchi <me@s6n.jp>
@siketyan siketyan marked this pull request as ready for review July 3, 2023 15:06
@siketyan siketyan requested a review from a team as a code owner July 3, 2023 15:06
@siketyan
Copy link
Author

siketyan commented Jul 3, 2023

I've tested on my local environment and it worked.
Authentication using AWS Identity Centre (formerly AWS SSO) also works!

Comment on lines +43 to +48
if err != nil {
var noSuchBucket *types.NoSuchBucket
if errors.As(err, &noSuchBucket) {
return nil, fmt.Errorf(errS3NoSuchBucket, err)
}
}
Copy link
Author

Choose a reason for hiding this comment

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

What should we do here if an unexpected error is occurred? Previous implementation does not have any error handling for this too, and that causes a panic.

@@ -55,8 +58,9 @@ func TestBackendConfig(t *testing.T) {
}

b := backend.TestBackendConfig(t, New(), backend.TestWrapConfig(config)).(*Backend)
options := reflect.ValueOf(*b.s3Client).Elem().FieldByName("options").Interface().(s3.Options)
Copy link
Author

@siketyan siketyan Jul 3, 2023

Choose a reason for hiding this comment

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

Are there any options to do this in much better way?

@gdavison
Copy link
Contributor

gdavison commented Jul 7, 2023

Thanks for submitting this PR, @siketyan. Because aws-sdk-go-base/v2 changes the authentication flow to match the default behaviour of the AWS CLI and SDKs, simply replacing it would be a breaking change, so we need to roll out the behaviour change over several releases of Terraform.

We have an active project to update this, and we will try to use your changes where we can.

@siketyan
Copy link
Author

aws-sdk-go v1 has backport to support AWS Identity Center now, and the SDK used in Terraform S3 backend will be updated in #33522.

@jar-b
Copy link
Member

jar-b commented Aug 30, 2023

Hi @siketyan 👋 - thanks for your effort on this!

As @gdavison mentioned earlier, we've been working on a broader effort to modernize the S3 backend, which will occur over several minor releases. #33730 covered much of the scope of this PR and will be released with Terraform 1.6, so I believe this can now be closed. Thanks again for your engagement!

@jar-b jar-b closed this Aug 30, 2023
@siketyan siketyan deleted the feat/aws-sdk-go-v2 branch August 31, 2023 01:04
Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants