aws_s3_bucket_logging: Retry on empty logging configuration output

[alewando]

Description

After creating a aws_s3_bucket_logging resource, we sometimes are getting an error message indicating the logging configuration was read as empty:

Error: error reading S3 Bucket xxxx Logging: empty output

This PR enhances the existing retry functionality in the resourceBucketLoggingRead method (which currently only retries on a 'NoSuchBucket' error) to also retry on an empty read of the bucket logging output.

References

Fixes #31139
Replaces #30749 (which had issues with extra commits)

Output from Acceptance Testing

$ make testacc TESTS=TestAccS3BucketLogging_basic PKG=s3
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 20 -run='TestAccS3BucketLogging_basic'  -timeout 180m
=== RUN   TestAccS3BucketLogging_basic
=== PAUSE TestAccS3BucketLogging_basic
=== CONT  TestAccS3BucketLogging_basic
--- PASS: TestAccS3BucketLogging_basic (13.06s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/s3	15.166s

[github-actions]

Community Note

Voting for Prioritization

• Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
• Please see our prioritization guide for information on how we prioritize.
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

• Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
• For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
• Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

[nomeelnoj]

I am unable to determine when this bug was introduced. Until this is merged, is there a known version of the AWS provider we can pin to that does not include this issue?

[alewando]

I think this was introduced by a change in AWS behavior, not in the TF provider. It started occurring for us at the beginning of April 2023, which is when AWS changed some default S3 behavior for new buckets. I don't know if the two are related, since bucket logging isn't mentioned as something that changed, but that's when we started seeing this error in our integration tests.

For what it's worth, we've been running with a custom build of this provider that has this change for a couple of weeks now and haven't had this error come up at all.

[johnsonaj]

LGTM 🚀

$ make testacc TESTS="TestAccS3BucketLogging_" PKG=s3

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 20 -run='TestAccS3BucketLogging_'  -timeout 180m
    acctest.go:79: 'AWS_S3_BUCKET_LOGGING_AMAZON_CUSTOMER_BY_EMAIL' not set, skipping test.
--- SKIP: TestAccS3BucketLogging_TargetGrantByEmail (0.00s)
--- PASS: TestAccS3BucketLogging_basic (25.20s)
--- PASS: TestAccS3BucketLogging_migrate_loggingNoChange (39.58s)
--- PASS: TestAccS3BucketLogging_migrate_loggingWithChange (39.71s)
--- PASS: TestAccS3BucketLogging_TargetGrantByGroup (57.04s)
--- PASS: TestAccS3BucketLogging_TargetGrantByID (58.93s)
--- PASS: TestAccS3BucketLogging_update (59.07s)
--- PASS: TestAccS3BucketLogging_disappears (141.52s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/s3	144.856s

[johnsonaj]

@alewando thanks for the contribution! 🎉

[thomasbaldwin]

Hey @johnsonaj! When can we expect a release for this? I see it's still in the v5.6.0 milestone but I can't find a release or due date for that milestone

[jcarlson]

What are the odds we'll see this fix back-ported to a 4.x version? We haven't upgraded to v5 at my company yet, and it's still pretty new as far as major versions go...

[github-actions]

This functionality has been released in v5.6.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!

[github-actions]

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.
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.