-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
b/fix PreconditionFailed errors on aws_cloudfront_distribution #24537
b/fix PreconditionFailed errors on aws_cloudfront_distribution #24537
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀.
% make testacc TESTARGS='-run=TestAccCloudFrontDistribution_preconditionFailed\|TestAccCloudFrontDistribution_s3OriginWithTags' PKG=cloudfront
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudfront/... -v -count 1 -parallel 20 -run=TestAccCloudFrontDistribution_preconditionFailed\|TestAccCloudFrontDistribution_s3OriginWithTags -timeout 180m
=== RUN TestAccCloudFrontDistribution_s3OriginWithTags
=== PAUSE TestAccCloudFrontDistribution_s3OriginWithTags
=== RUN TestAccCloudFrontDistribution_preconditionFailed
=== PAUSE TestAccCloudFrontDistribution_preconditionFailed
=== CONT TestAccCloudFrontDistribution_s3OriginWithTags
=== CONT TestAccCloudFrontDistribution_preconditionFailed
--- PASS: TestAccCloudFrontDistribution_s3OriginWithTags (616.86s)
--- PASS: TestAccCloudFrontDistribution_preconditionFailed (627.03s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/cloudfront 631.029s
@iandrewt Thanks for the contribution 🎉 👏. |
This functionality has been released in v4.13.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
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 issues. |
Amazon's docs say to fetch the ETag immediately before updating to ensure it is up to date. This fix only updates it if the PreconditonFailed error occurs and tries again, since it seems AWS fails fast if the ETag is incorrect.
https://docs.aws.amazon.com/cloudfront/latest/APIReference/API_UpdateDistribution.html
From my findings, CloudFront redeploys the distribution automatically when something it depends on (like a Response Header Policy) changes, creating a new ETag variable. Fair enough, but not really conducive to having a state file with a value that can be modified by an external resource. IMO the ETag shouldn't be stored in the state at all because of this, but for simple distributions that have no external resources having the ETag can in theory speed things up, as long as all changes are made in Terraform.
Since adding this fix locally, I haven't encountered a PreconditonFailed error once, in quite a complicated distribution, however if this fix isn't an ideal solution I'm happy to be included in discussions for better ideas.
Community Note
Closes #24033
Closes #22816
Output from acceptance testing: