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.

Sign up for GitHub

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

Jump to bottom

Transition to S3-native state locking and deprecate DynamoDB arguments #36257

Merged
merged 13 commits into from
Jan 8, 2025
Merged

Transition to S3-native state locking and deprecate DynamoDB arguments #36257

merged 13 commits into from
Jan 8, 2025
+104 −100

Conversation

bschaatsbergen
Copy link
Member

  • Removed experimental verbiage from use_lockfile, as S3-native state locking is now stable and no longer considered experimental.
  • Deprecated DynamoDB-related arguments, encouraging the use of S3-native state locking instead.
  • Improved documentation to better explain S3-native state locking and highlight the deprecation of DynamoDB-related state locking.

jar-b
jar-b reviewed Jan 3, 2025
View reviewed changes
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, just a typo and a couple small suggestions

website/docs/language/upgrade-guides/index.mdx Outdated Show resolved Hide resolved
website/docs/language/backend/s3.mdx Outdated Show resolved Hide resolved
website/docs/language/backend/s3.mdx Outdated Show resolved Hide resolved
website/docs/language/backend/s3.mdx Show resolved Hide resolved
@bschaatsbergen bschaatsbergen mentioned this pull request Jan 6, 2025
Open
@bschaatsbergen bschaatsbergen marked this pull request as ready for review January 7, 2025 14:41
@bschaatsbergen bschaatsbergen requested review from a team as code owners January 7, 2025 14:41
@jar-b
backend/s3(test): add dynamodb_table deprecation case
164d1e2 
```console
% go test -count=1 -run="TestBackendConfig_PrepareConfigValidation" ./...
ok      github.com/hashicorp/terraform/internal/backend/remote-state/s3 0.507s
```
@bschaatsbergen
Copy link
Member Author

bschaatsbergen commented Jan 7, 2025 •
edited
Loading

One thing I noticed while working on deprecating DynamoDB-related arguments is that no warning diagnostics are being surfaced to the console when you're using a deprecated argument in the s3 block. It’s something to look into, but it’s not blocking since this behavior is also present in 1.10. cc @jar-b / @gdavidson.

jar-b
jar-b previously approved these changes Jan 7, 2025
View reviewed changes
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 go test ./...
ok      github.com/hashicorp/terraform/internal/backend/remote-state/s3 203.342s

@jar-b
Copy link
Member

jar-b commented Jan 7, 2025

Thanks for the updates @bschaatsbergen! 👍

@bschaatsbergen
s3: add verbiage regarding s3:DeleteObject permission
cd4a181 
Clarify S3 permissions when using S3-native state locking.
jar-b
jar-b previously approved these changes Jan 7, 2025
View reviewed changes
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 🚀

gdavison
gdavison previously approved these changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@gdavison gdavison left a comment

Choose a reason for hiding this comment

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

Looks good! 🚀

@bschaatsbergen bschaatsbergen dismissed stale reviews from gdavison and jar-b via 9609070 January 7, 2025 18:34
RubenSandwich
RubenSandwich approved these changes Jan 8, 2025
View reviewed changes
Copy link
Contributor

@RubenSandwich RubenSandwich left a comment

Choose a reason for hiding this comment

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

The Vercel build failing is a red herring, and can safetly be ignored. I believe something with the co-commit broke the Vercel preview pipeline. (It seems to be using an older version of our docs deploy pipeline. 🤷🏻‍♂️ )

I tested this fork locally and it worked fine:
Screenshot 2025-01-07 at 6 59 22 PM

@jar-b jar-b merged commit 4074d35 into hashicorp:main Jan 8, 2025
7 of 8 checks passed
@bschaatsbergen bschaatsbergen deleted the s3-native-state-lock-nonexp branch January 8, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jar-b jar-b jar-b approved these changes

@RubenSandwich RubenSandwich RubenSandwich approved these changes

@gdavison gdavison gdavison left review comments

Assignees
No one assigned
Labels
backend/s3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bschaatsbergen @jar-b @gdavison @RubenSandwich