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

S3 object version id is not correctly updated during diff #14900

Merged
merged 3 commits into from
Nov 13, 2020

Conversation

wernerb
Copy link
Contributor

@wernerb wernerb commented Aug 28, 2020

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" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

The configuration keys listed always result in changes to version_id. We make terraform aware of this. Currently terraform bails out during apply and refers to the AWS provider to fix this.
This also fixes unrelated bugs in the tests

Closes #14899

Release note for CHANGELOG:

resource/aws_s3_bucket_object: version_id is correctly updated when certain configuration keys are changed

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSS3BucketObject'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSS3BucketObject -timeout 120m
=== RUN   TestAccAWSS3BucketObject_noNameNoKey
=== PAUSE TestAccAWSS3BucketObject_noNameNoKey
=== RUN   TestAccAWSS3BucketObject_empty
=== PAUSE TestAccAWSS3BucketObject_empty
=== RUN   TestAccAWSS3BucketObject_source
=== PAUSE TestAccAWSS3BucketObject_source
=== RUN   TestAccAWSS3BucketObject_content
=== PAUSE TestAccAWSS3BucketObject_content
=== RUN   TestAccAWSS3BucketObject_etagEncryption
=== PAUSE TestAccAWSS3BucketObject_etagEncryption
=== RUN   TestAccAWSS3BucketObject_contentBase64
=== PAUSE TestAccAWSS3BucketObject_contentBase64
=== RUN   TestAccAWSS3BucketObject_withContentCharacteristics
=== PAUSE TestAccAWSS3BucketObject_withContentCharacteristics
=== RUN   TestAccAWSS3BucketObject_NonVersioned
=== PAUSE TestAccAWSS3BucketObject_NonVersioned
=== RUN   TestAccAWSS3BucketObject_updates
=== PAUSE TestAccAWSS3BucketObject_updates
=== RUN   TestAccAWSS3BucketObject_updateSameFile
=== PAUSE TestAccAWSS3BucketObject_updateSameFile
=== RUN   TestAccAWSS3BucketObject_updatesWithVersioning
=== PAUSE TestAccAWSS3BucketObject_updatesWithVersioning
=== RUN   TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint
=== PAUSE TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint
=== RUN   TestAccAWSS3BucketObject_kms
=== PAUSE TestAccAWSS3BucketObject_kms
=== RUN   TestAccAWSS3BucketObject_sse
=== PAUSE TestAccAWSS3BucketObject_sse
=== RUN   TestAccAWSS3BucketObject_acl
=== PAUSE TestAccAWSS3BucketObject_acl
=== RUN   TestAccAWSS3BucketObject_metadata
=== PAUSE TestAccAWSS3BucketObject_metadata
=== RUN   TestAccAWSS3BucketObject_storageClass
=== PAUSE TestAccAWSS3BucketObject_storageClass
=== RUN   TestAccAWSS3BucketObject_tags
=== PAUSE TestAccAWSS3BucketObject_tags
=== RUN   TestAccAWSS3BucketObject_tagsLeadingSlash
=== PAUSE TestAccAWSS3BucketObject_tagsLeadingSlash
=== RUN   TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithNone
=== PAUSE TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithNone
=== RUN   TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithOn
=== PAUSE TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithOn
=== RUN   TestAccAWSS3BucketObject_ObjectLockRetentionStartWithNone
=== PAUSE TestAccAWSS3BucketObject_ObjectLockRetentionStartWithNone
=== RUN   TestAccAWSS3BucketObject_ObjectLockRetentionStartWithSet
=== PAUSE TestAccAWSS3BucketObject_ObjectLockRetentionStartWithSet
=== CONT  TestAccAWSS3BucketObject_noNameNoKey
=== CONT  TestAccAWSS3BucketObject_kms
=== CONT  TestAccAWSS3BucketObject_storageClass
=== CONT  TestAccAWSS3BucketObject_withContentCharacteristics
=== CONT  TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint
=== CONT  TestAccAWSS3BucketObject_ObjectLockRetentionStartWithSet
=== CONT  TestAccAWSS3BucketObject_updatesWithVersioning
=== CONT  TestAccAWSS3BucketObject_updateSameFile
=== CONT  TestAccAWSS3BucketObject_ObjectLockRetentionStartWithNone
=== CONT  TestAccAWSS3BucketObject_updates
=== CONT  TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithOn
=== CONT  TestAccAWSS3BucketObject_NonVersioned
=== CONT  TestAccAWSS3BucketObject_acl
=== CONT  TestAccAWSS3BucketObject_metadata
=== CONT  TestAccAWSS3BucketObject_content
=== CONT  TestAccAWSS3BucketObject_tags
=== CONT  TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithNone
=== CONT  TestAccAWSS3BucketObject_contentBase64
=== CONT  TestAccAWSS3BucketObject_etagEncryption
=== CONT  TestAccAWSS3BucketObject_tagsLeadingSlash
--- PASS: TestAccAWSS3BucketObject_noNameNoKey (5.69s)
=== CONT  TestAccAWSS3BucketObject_sse
=== CONT  TestAccAWSS3BucketObject_updateSameFile
    resource_aws_s3_bucket_object_test.go:350: [INFO] Got non-empty plan, as expected
    resource_aws_s3_bucket_object_test.go:350: [INFO] Got non-empty plan, as expected
--- PASS: TestAccAWSS3BucketObject_NonVersioned (48.64s)
=== CONT  TestAccAWSS3BucketObject_empty
--- PASS: TestAccAWSS3BucketObject_content (50.02s)
=== CONT  TestAccAWSS3BucketObject_source
--- PASS: TestAccAWSS3BucketObject_contentBase64 (50.38s)
--- PASS: TestAccAWSS3BucketObject_withContentCharacteristics (50.43s)
--- PASS: TestAccAWSS3BucketObject_etagEncryption (50.82s)
--- PASS: TestAccAWSS3BucketObject_kms (53.53s)
--- PASS: TestAccAWSS3BucketObject_sse (49.89s)
--- PASS: TestAccAWSS3BucketObject_updateSameFile (96.28s)
--- PASS: TestAccAWSS3BucketObject_empty (48.69s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithOn (97.77s)
--- PASS: TestAccAWSS3BucketObject_updates (98.41s)
--- PASS: TestAccAWSS3BucketObject_source (49.26s)
--- PASS: TestAccAWSS3BucketObject_updatesWithVersioning (99.45s)
--- PASS: TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint (107.91s)
--- PASS: TestAccAWSS3BucketObject_metadata (138.22s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockRetentionStartWithNone (142.66s)
--- PASS: TestAccAWSS3BucketObject_acl (143.81s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithNone (144.76s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockRetentionStartWithSet (185.26s)
--- PASS: TestAccAWSS3BucketObject_tags (186.29s)
--- PASS: TestAccAWSS3BucketObject_tagsLeadingSlash (186.29s)
--- PASS: TestAccAWSS3BucketObject_storageClass (229.40s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	229.446

@wernerb wernerb requested a review from a team August 28, 2020 21:12
@ghost ghost added size/S Managed by automation to categorize the size of a PR. needs-triage Waiting for first response or review from a maintainer. service/s3 Issues and PRs that pertain to the s3 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Aug 28, 2020
@wernerb wernerb changed the title Staleversion s3 object version id is not correctly updated during diff Aug 28, 2020
@wernerb wernerb changed the title s3 object version id is not correctly updated during diff S3 object version id is not correctly updated during diff Aug 28, 2020
@gdavison gdavison self-assigned this Sep 3, 2020
@gdavison gdavison removed the needs-triage Waiting for first response or review from a maintainer. label Sep 3, 2020
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.

Thanks for the PR, @wernerb. It looks great!

One change to make, and one suggestion

aws/resource_aws_s3_bucket_object.go Show resolved Hide resolved
aws/resource_aws_s3_bucket_object_test.go Outdated Show resolved Hide resolved
@ghost ghost added size/M Managed by automation to categorize the size of a PR. and removed size/S Managed by automation to categorize the size of a PR. labels Sep 4, 2020
@wernerb
Copy link
Contributor Author

wernerb commented Sep 4, 2020

@gdavison I have implemented your changes.

@wernerb
Copy link
Contributor Author

wernerb commented Sep 10, 2020

@gdavison I understand this is not the proper etiquette, but our company is currently suffering from this bug with cloudformation stacks unable to update without this change. This update hurts as it happened after the terraform 12 upgrade and we cannot go back anymore.

Could you provide perhaps, at least an ETA so we can decide to create custom providers ourselves?

Fixes that version_id is stale during apply. Changes in certain keys
existing object always results in a new version_id.
@wernerb
Copy link
Contributor Author

wernerb commented Oct 27, 2020

@gdavison do you have a chance to look at my commenst on your comments. I rebased everything and the above test failure seems to be a problem in the pipeline?

gdavison added a commit that referenced this pull request Nov 13, 2020
@gdavison gdavison merged commit 4560456 into hashicorp:master Nov 13, 2020
gdavison added a commit that referenced this pull request Nov 13, 2020
@gdavison gdavison added this to the v3.16.0 milestone Nov 13, 2020
@gdavison
Copy link
Contributor

Thanks for your patience, @wernerb, and thanks again for the PR. The test failure was due to a timeout being too short.

@wernerb wernerb deleted the staleversion branch November 13, 2020 20:45
@ghost
Copy link

ghost commented Nov 18, 2020

This has been released in version 3.16.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!

@ghost
Copy link

ghost commented Dec 14, 2020

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!

@ghost ghost locked as resolved and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
service/s3 Issues and PRs that pertain to the s3 service. size/M Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error/Bug: aws_s3_bucket_object inconsistent final plan with version_id
2 participants