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

Refactor S3 Bucket Object data source and resource to use keyvaluetags package #11964

Merged
merged 2 commits into from
Feb 12, 2020
Merged

Refactor S3 Bucket Object data source and resource to use keyvaluetags package #11964

merged 2 commits into from
Feb 12, 2020

Conversation

ewbankkit
Copy link
Contributor

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

Relates #10688.

Release note for CHANGELOG:

NONE

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccDataSourceAWSS3BucketObject_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccDataSourceAWSS3BucketObject_ -timeout 120m
=== RUN   TestAccDataSourceAWSS3BucketObject_basic
=== PAUSE TestAccDataSourceAWSS3BucketObject_basic
=== RUN   TestAccDataSourceAWSS3BucketObject_readableBody
=== PAUSE TestAccDataSourceAWSS3BucketObject_readableBody
=== RUN   TestAccDataSourceAWSS3BucketObject_kmsEncrypted
=== PAUSE TestAccDataSourceAWSS3BucketObject_kmsEncrypted
=== RUN   TestAccDataSourceAWSS3BucketObject_allParams
=== PAUSE TestAccDataSourceAWSS3BucketObject_allParams
=== RUN   TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOff
=== PAUSE TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOff
=== RUN   TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOn
=== PAUSE TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOn
=== RUN   TestAccDataSourceAWSS3BucketObject_LeadingSlash
=== PAUSE TestAccDataSourceAWSS3BucketObject_LeadingSlash
=== RUN   TestAccDataSourceAWSS3BucketObject_MultipleSlashes
=== PAUSE TestAccDataSourceAWSS3BucketObject_MultipleSlashes
=== CONT  TestAccDataSourceAWSS3BucketObject_basic
=== CONT  TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOn
=== CONT  TestAccDataSourceAWSS3BucketObject_MultipleSlashes
=== CONT  TestAccDataSourceAWSS3BucketObject_LeadingSlash
=== CONT  TestAccDataSourceAWSS3BucketObject_allParams
=== CONT  TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOff
=== CONT  TestAccDataSourceAWSS3BucketObject_kmsEncrypted
=== CONT  TestAccDataSourceAWSS3BucketObject_readableBody
--- PASS: TestAccDataSourceAWSS3BucketObject_basic (63.12s)
--- PASS: TestAccDataSourceAWSS3BucketObject_readableBody (63.17s)
--- PASS: TestAccDataSourceAWSS3BucketObject_MultipleSlashes (64.05s)
--- PASS: TestAccDataSourceAWSS3BucketObject_LeadingSlash (64.31s)
--- PASS: TestAccDataSourceAWSS3BucketObject_allParams (64.56s)
--- PASS: TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOff (65.62s)
--- PASS: TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOn (65.70s)
--- PASS: TestAccDataSourceAWSS3BucketObject_kmsEncrypted (87.14s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	87.207s
$ make testacc TEST=./aws 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_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_sse
=== CONT  TestAccAWSS3BucketObject_ObjectLockRetentionStartWithSet
=== CONT  TestAccAWSS3BucketObject_ObjectLockRetentionStartWithNone
=== CONT  TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithOn
=== CONT  TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithNone
=== CONT  TestAccAWSS3BucketObject_tagsLeadingSlash
=== CONT  TestAccAWSS3BucketObject_kms
=== CONT  TestAccAWSS3BucketObject_tags
=== CONT  TestAccAWSS3BucketObject_updatesWithVersioning
=== CONT  TestAccAWSS3BucketObject_storageClass
=== CONT  TestAccAWSS3BucketObject_metadata
=== CONT  TestAccAWSS3BucketObject_updateSameFile
=== CONT  TestAccAWSS3BucketObject_acl
=== CONT  TestAccAWSS3BucketObject_updates
=== CONT  TestAccAWSS3BucketObject_NonVersioned
=== CONT  TestAccAWSS3BucketObject_withContentCharacteristics
=== CONT  TestAccAWSS3BucketObject_contentBase64
=== CONT  TestAccAWSS3BucketObject_etagEncryption
=== CONT  TestAccAWSS3BucketObject_content
--- SKIP: TestAccAWSS3BucketObject_NonVersioned (2.13s)
    provider_test.go:1254: skipping tests; TF_ACC_ASSUME_ROLE_ARN must be set
=== CONT  TestAccAWSS3BucketObject_source
--- PASS: TestAccAWSS3BucketObject_noNameNoKey (5.50s)
=== CONT  TestAccAWSS3BucketObject_empty
--- PASS: TestAccAWSS3BucketObject_withContentCharacteristics (41.61s)
--- PASS: TestAccAWSS3BucketObject_content (42.54s)
--- PASS: TestAccAWSS3BucketObject_contentBase64 (42.76s)
--- PASS: TestAccAWSS3BucketObject_source (40.62s)
--- PASS: TestAccAWSS3BucketObject_sse (43.30s)
--- PASS: TestAccAWSS3BucketObject_etagEncryption (43.66s)
--- PASS: TestAccAWSS3BucketObject_empty (39.23s)
--- PASS: TestAccAWSS3BucketObject_kms (66.16s)
--- PASS: TestAccAWSS3BucketObject_updateSameFile (69.01s)
--- PASS: TestAccAWSS3BucketObject_updates (70.23s)
--- PASS: TestAccAWSS3BucketObject_updatesWithVersioning (70.81s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithOn (71.30s)
--- PASS: TestAccAWSS3BucketObject_metadata (94.38s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockRetentionStartWithNone (96.86s)
--- PASS: TestAccAWSS3BucketObject_acl (97.99s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithNone (98.02s)
--- PASS: TestAccAWSS3BucketObject_tags (120.78s)
--- PASS: TestAccAWSS3BucketObject_tagsLeadingSlash (121.50s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockRetentionStartWithSet (122.49s)
--- PASS: TestAccAWSS3BucketObject_storageClass (147.01s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	147.061s

@ewbankkit ewbankkit requested a review from a team February 7, 2020 20:16
@ghost ghost added needs-triage Waiting for first response or review from a maintainer. size/L Managed by automation to categorize the size of a PR. service/s3 Issues and PRs that pertain to the s3 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Feb 7, 2020
ewbankkit
ewbankkit commented Feb 7, 2020
View reviewed changes
aws/resource_aws_s3_bucket_object_test.go
Key4 = "DDD"
Key5 = "EEE"
Key5 = "E:/"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test with some characters that need to be URL encoded.

@ewbankkit ewbankkit changed the title Refactor S3 Bucket Object data source and resource to use keyvaluetags package [WIP] Refactor S3 Bucket Object data source and resource to use keyvaluetags package Feb 9, 2020
@ewbankkit

This comment has been minimized.

Sign in to view
@ewbankkit ewbankkit mentioned this pull request Feb 11, 2020
Merged
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Feb 12, 2020
@bflad bflad added technical-debt Addresses areas of the codebase that need refactoring or redesign. and removed needs-triage Waiting for first response or review from a maintainer. labels Feb 12, 2020
@bflad bflad self-assigned this Feb 12, 2020
@bflad bflad added this to the v2.49.0 milestone Feb 12, 2020
bflad
bflad approved these changes Feb 12, 2020
View reviewed changes
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @ewbankkit 🚀

Output from acceptance testing:

--- PASS: TestAccAWSS3BucketObject_acl (31.06s)
--- PASS: TestAccAWSS3BucketObject_content (22.64s)
--- PASS: TestAccAWSS3BucketObject_contentBase64 (21.80s)
--- PASS: TestAccAWSS3BucketObject_empty (22.31s)
--- PASS: TestAccAWSS3BucketObject_etagEncryption (21.89s)
--- PASS: TestAccAWSS3BucketObject_kms (36.14s)
--- PASS: TestAccAWSS3BucketObject_metadata (30.03s)
--- PASS: TestAccAWSS3BucketObject_noNameNoKey (7.02s)
--- PASS: TestAccAWSS3BucketObject_NonVersioned (22.05s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithNone (31.88s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithOn (23.42s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockRetentionStartWithNone (29.92s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockRetentionStartWithSet (35.34s)
--- PASS: TestAccAWSS3BucketObject_source (21.47s)
--- PASS: TestAccAWSS3BucketObject_sse (15.41s)
--- PASS: TestAccAWSS3BucketObject_storageClass (42.50s)
--- PASS: TestAccAWSS3BucketObject_tags (36.27s)
--- PASS: TestAccAWSS3BucketObject_tagsLeadingSlash (38.08s)
--- PASS: TestAccAWSS3BucketObject_updates (30.28s)
--- PASS: TestAccAWSS3BucketObject_updateSameFile (29.88s)
--- PASS: TestAccAWSS3BucketObject_updatesWithVersioning (26.85s)
--- PASS: TestAccAWSS3BucketObject_withContentCharacteristics (21.15s)

--- PASS: TestAccDataSourceAWSS3BucketObject_allParams (34.39s)
--- PASS: TestAccDataSourceAWSS3BucketObject_basic (36.34s)
--- PASS: TestAccDataSourceAWSS3BucketObject_kmsEncrypted (55.18s)
--- PASS: TestAccDataSourceAWSS3BucketObject_LeadingSlash (36.58s)
--- PASS: TestAccDataSourceAWSS3BucketObject_MultipleSlashes (37.72s)
--- PASS: TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOff (37.37s)
--- PASS: TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOn (37.99s)
--- PASS: TestAccDataSourceAWSS3BucketObject_readableBody (35.87s)

--- PASS: TestAccDataSourceAWSS3BucketObjects_all (40.93s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_basic (38.14s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_encoded (41.15s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_fetchOwner (40.57s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_maxKeys (40.56s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_prefixes (41.50s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_startAfter (39.73s)

@bflad bflad merged commit 4acb898 into hashicorp:master Feb 12, 2020
@ewbankkit ewbankkit changed the title [WIP] Refactor S3 Bucket Object data source and resource to use keyvaluetags package Refactor S3 Bucket Object data source and resource to use keyvaluetags package Feb 12, 2020
@ewbankkit
Copy link
Contributor Author

@bflad Thanks for merging.
I've just completed the changes to handle any ignored (either aws: prefixes or those ignored via any future ignore_tags or ignore_tag_prefixes functionality) and will create a new PR for this.

@ewbankkit ewbankkit mentioned this pull request Feb 12, 2020
Merged
@ghost
Copy link

ghost commented Feb 14, 2020

This has been released in version 2.49.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 Mar 27, 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 and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bflad bflad bflad approved these changes

Assignees

@bflad bflad

Labels
service/s3 Issues and PRs that pertain to the s3 service. size/XL Managed by automation to categorize the size of a PR. technical-debt Addresses areas of the codebase that need refactoring or redesign. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Milestone
v2.49.0
Development

Successfully merging this pull request may close these issues.
2 participants
@ewbankkit @bflad