s3_bucket_object: Fix ETAG changes not forcing new

[YakDriver]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Closes #2411

Release note for CHANGELOG:

resource/aws_s3_bucket_object: Update object contents when the source is the same but the contents/ETAG are not [GH-2411]

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSS3BucketObject_updateSameFile'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccAWSS3BucketObject_updateSameFile -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSS3BucketObject_updateSameFile
=== PAUSE TestAccAWSS3BucketObject_updateSameFile
=== CONT  TestAccAWSS3BucketObject_updateSameFile
--- PASS: TestAccAWSS3BucketObject_updateSameFile (55.93s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	55.986s

[YakDriver]

@bflad This is rebased. Any adjust you'd like on this? This fix will make several of our processes much cleaner.

[bflad]

Hey @YakDriver, all 👋

The new acceptance test is currently passing on master for me:

--- PASS: TestAccAWSS3BucketObject_updateSameFile (49.72s)

What is the incorrect behavior that is supposed to be fixed by forced recreation of the object? The debug logging, as part of the testing passing, is showing the expected non-empty plan after rewriting the file and the PutObject call to update it.

[lorengordon]

@bflad, it's described in #2411... when the source contents change but the source value does not, etag presumably can be used to force the provider to upload the new contents to S3. That hadn't been working for a while now.

The plan I've seen wants to update just the etag value (exactly as depicted in the linked issue), which S3 does not support (makes sense). The apply was still failing in whatever the latest release was just a couple weeks ago, but maybe it's somehow now magically working?

[YakDriver]

but maybe it's somehow now magically working?

Exactly. ☝️

@bflad Although now I believe this was always an AWS bug, see my explanation #2411 (comment), I would feel better if we added the acceptance test anyway. Then if the s3 service API breaks again, we'd know about it. I updated this PR to leave just the acceptance test.

[bflad]

Adding the acceptance test to check for API regressions in the future. Thanks, @YakDriver 🚀

[ghost]

This has been released in version 2.26.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 for triage. Thanks!

[bostrowski13]

As FYI, this issue is not noted in the changelog as part of the 2.26.0 release

[bflad]

@bostrowski13 thanks for the notice. The fix occurred in the API itself, not Terraform. Because of that, this pull request became only adding an acceptance test so we can potentially detect any API regression that might occur. We typically do not CHANGELOG testing-only changes as they are not operator facing. 👍

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!