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

backend/s3: Update S3 client to AWS SDK Go V2 #33730

Merged
merged 23 commits into from
Aug 24, 2023
Merged

backend/s3: Update S3 client to AWS SDK Go V2 #33730

merged 23 commits into from
Aug 24, 2023

Conversation

jar-b
Copy link
Member

@jar-b jar-b commented Aug 23, 2023

This change upgrades the S3 backend AWS clients to AWS SDK Go V2. In addition to moving the backend off the aging V1 SDK, this will enable parity with the AWS provider configuration arguments and resolution of several feature requests captured in the S3 backend modernization meta-issue, #33687. Some of these enhancements have already been incorporated into the feature branch, including:

The remaining items captured in the meta-issue will be incorporated as standalone pull requests to avoid stretching the scope of this PR beyond its current size.

% TF_ACC=1 go test -count=1 ./internal/backend/remote-state/s3/...
ok      github.com/hashicorp/terraform/internal/backend/remote-state/s3 106.875s

Closes #30443
Closes #31244
Closes #30493

Target Release

1.6.0

Draft CHANGELOG entry

UPGRADE NOTES

  • Adds an assume_role_with_web_identity argument. (#31244)
  • Supports the default AWS environment variables for credentials and configuration files: AWS_CONFIG_FILE and AWS_SHARED_CREDENTIALS_FILE. (#30493)
  • Adds shared_config_files and shared_credentials_files arguments.
    This deprecates the shared_credentials_file argument. (#30493)
  • Upgrades the S3 backend to use AWS SDK Go V2. (#30443)

gdavison and others added 22 commits August 3, 2023 17:37
backend/s3: Adds nested attribute for `assume_role`
backend/s3: Updates `aws-sdk-go-base` to `v2`
…tion

backend/s3: Additional validation to `modernization` branch
backend/s3: add `use_legacy_workflow` argument to `modernization` branch
backend/s3: support multiple shared config/credentials files
…ith-web-identity-support

Add assume role with web identity support for the S3 backend
@jar-b jar-b requested a review from a team as a code owner August 23, 2023 20:32
Comment on lines -1012 to +1015
slices.SortFunc(states, func(a, b *TestFileState) bool {
slices.SortFunc(states, func(a, b *TestFileState) int {
// We want to clean up later run blocks first. So, we'll sort this in
// reverse according to index. This means larger indices first.
return a.Run.Index > b.Run.Index
return b.Run.Index - a.Run.Index
Copy link
Member Author

Choose a reason for hiding this comment

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

aws-sdk-go-base/v2 uses the latest version of golang.org/x/exp, which includes a breaking change to the slices.SortFunc signature:

func SortFunc[S ~[]E, E any](x S, cmp func(a, b E) int)

SortFunc sorts the slice x in ascending order as determined by the cmp function. This sort is not guaranteed to be stable. cmp(a, b) should return a negative number when a < b, a positive number when a > b and zero when a == b.

To continue sorting in descending order the arguments were switched (b - a). A simple sanity is included below, for reference.

// Old signature
func main() {
	sl := []int{5, 1, 3, 4, 2, 6}
	fmt.Printf("before: %v\n", sl)
	// before: [5 1 3 4 2 6]

	slices.SortFunc(sl, func(a, b int) bool {
		return a > b
	})
	fmt.Printf("after: %v\n", sl)
	// after: [6 5 4 3 2 1]
}
// New signature
func main() {
	sl := []int{5, 1, 3, 4, 2, 6}
	fmt.Printf("before: %v\n", sl)
	// before: [5 1 3 4 2 6]

	slices.SortFunc(sl, func(a, b int) int {
		return b - a
	})
	fmt.Printf("after: %v\n", sl)
	// after: [6 5 4 3 2 1]
}

Lastly, this appears to be the only place the x/exp package is utilized outside of the S3 backend, so there should be no impact elsewhere.

% rg "golang.org/x/exp" -g '!/go.*'
internal/command/test.go
17:     "golang.org/x/exp/slices"

internal/backend/remote-state/s3/backend_test.go
34:     "golang.org/x/exp/maps"

@ewbankkit ewbankkit self-assigned this Aug 24, 2023
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

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

LGTM 🚀.

% TF_ACC=1 go test -count=1 ./internal/backend/remote-state/s3/...
ok  	github.com/hashicorp/terraform/internal/backend/remote-state/s3	112.041s

@jar-b jar-b merged commit c566bcd into main Aug 24, 2023
@github-actions
Copy link
Contributor

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

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