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

Backport of s3: fix S3 Object Lock header issue for lock file writes into v1.10 #36146

Merged
merged 4 commits into from
Dec 2, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Dec 2, 2024

Backport

This PR is auto-generated from #36120 to be assessed for backporting due to the inclusion of the label 1.10-backport.

🚨

Warning automatic cherry-pick of commits failed. If the first commit failed,
you will see a blank no-op commit below. If at least one commit succeeded, you
will see the cherry-picked commits up to, not including, the commit where
the merge conflict occurred.

The person who merged in the original PR is:
@jar-b
This person should manually cherry-pick the original PR into a new backport PR,
and close this one when the manual backport PR is merged in.

merge conflict error: unable to process merge commit: "476666758b7eefb2dedf08b882ee75779394e9f0", automatic backport requires rebase workflow

The below text is copied from the body of the original PR.


Fixes #36113

When S3 Object Lock is enabled on a bucket with a retention period, Amazon S3 requires the Content-MD5 or x-amz-sdk-checksum-algorithm header to be present in object uploads (PutObject). See Uploading objects to an Object Lock enabled bucket.

It seems we overlooked maintaining the default behavior of the skip_checksum flag for the lock file when writing to S3 Object Lock-enabled buckets.

To clarify the default behavior of skip_checksum: by default, if this argument is not set in the backend, we set the S3 checksum algorithm behavior to SHA256. This causes the underlying S3 AWS SDK V2 serializers to automatically append that required x-amz-sdk-checksum-algorithm header. For more details, see the relevant code in the AWS SDK v2 serializers.

This PR updates the lock file implementation to use the same "uploader" that we rely on for writing Terraform state to S3, and preserving the default skip_checksum behavior for the lock file. To ensure a consistent and compatible experience with S3 Object Lock-enabled buckets between the two mechanisms writing data to S3.

$  go test -v
...
PASS
ok      github.com/hashicorp/terraform/internal/backend/remote-state/s3 445.979s

Overview of commits

@github-actions github-actions bot force-pushed the backport/b/s3-object-lock-file/firmly-curious-yak branch from 2700ddf to 4498299 Compare December 2, 2024 17:15
Because the S3 backend has only diverged from main with the commits to address lockfile header issues with lock enabled buckets, this commit simply copies the existing state of the `internal/backend/remote-state/s3` subfolder onto the `v1.10` branch.
@jar-b jar-b marked this pull request as ready for review December 2, 2024 17:46
@jar-b jar-b requested a review from a team as a code owner December 2, 2024 17:46
bschaatsbergen
bschaatsbergen previously approved these changes Dec 2, 2024
Copy link
Member

@jar-b jar-b 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 TF_S3_OBJECT_LOCK_TEST=1 go test -count=1 ./...
ok      github.com/hashicorp/terraform/internal/backend/remote-state/s3 256.903s

Copy link
Member

@YakDriver YakDriver left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@jar-b jar-b merged commit 2126481 into v1.10 Dec 2, 2024
6 checks passed
@jar-b jar-b deleted the backport/b/s3-object-lock-file/firmly-curious-yak branch December 2, 2024 19:06
Copy link
Contributor Author

github-actions bot commented Dec 2, 2024

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

Copy link
Contributor Author

github-actions bot commented Jan 2, 2025

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 Jan 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants