-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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_copy: New resource #15461
s3_object_copy: New resource #15461
Conversation
9b0b947
to
ad0282a
Compare
f952f99
to
0a552ad
Compare
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.
I have a number of suggested changes. I'm also getting an non-empty plan for TestAccAWSS3BucketObject_ignoreTags
aws/resource_aws_s3_object_copy.go
Outdated
return nil | ||
} | ||
|
||
func resourceAwsS3ObjectCopyCopy(d *schema.ResourceData, meta interface{}) error { |
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.
Naming nit: CopyCopy
looks a little odd. Maybe something like resourceAwsS3ObjectCopyPerformCopy()
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.
What about resourceAwsS3ObjectCopyDoCopy()
?
"force_destroy": { | ||
Type: schema.TypeBool, | ||
Optional: true, | ||
Default: false, | ||
}, |
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.
force_destroy
isn't referenced anywhere, is the implementation missing?
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.
This resource uses the Delete from s3_bucket_object
(resourceAwsS3BucketObjectDelete
) which uses the force_destroy
arg.
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.
Never mind. Now it has its own delete!
aws/resource_aws_s3_object_copy.go
Outdated
return resourceAwsS3BucketObjectRead(d, meta) | ||
} | ||
|
||
func resourceAwsS3ObjectCopyIncludeGrants(input *s3.CopyObjectInput, d *schema.ResourceData) error { |
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.
This only seems to return nil
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.
Completely re-wrote the expand part.
aws/resource_aws_s3_object_copy.go
Outdated
Read: resourceAwsS3BucketObjectRead, | ||
Update: resourceAwsS3ObjectCopyUpdate, | ||
Delete: resourceAwsS3BucketObjectDelete, |
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.
Typically, we avoid reusing implementations across resource types. resourceAwsS3BucketObjectRead
and resourceAwsS3BucketObjectDelete
should have local implementations
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.
Added
0a552ad
to
b18a837
Compare
This happens all the time unrelated to this PR. See #17599. |
8a7a8f0
to
fe36cfe
Compare
d898c81
to
5d60673
Compare
@@ -482,6 +470,27 @@ func resourceAwsS3BucketObjectDelete(d *schema.ResourceData, meta interface{}) e | |||
return nil | |||
} | |||
|
|||
func resourceAwsS3BucketObjectSetKMS(d *schema.ResourceData, meta interface{}, sseKMSKeyId *string) error { |
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.
Breaking this out is necessary to avoid having 2 services (s3, kms) in aws_s3_object_copy
. This way, it can call the s3_bucket_object
function.
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.
A few suggestions, otherwise looks good to me 🚀
Commercial partition
--- PASS: TestAccAWSS3BucketObject_noNameNoKey (17.49s)
--- PASS: TestAccDataSourceAWSS3BucketObject_readableBody (62.95s)
--- PASS: TestAccDataSourceAWSS3BucketObject_basicViaAccessPoint (63.39s)
--- PASS: TestAccDataSourceAWSS3BucketObject_basic (63.50s)
--- PASS: TestAccDataSourceAWSS3BucketObject_SingleSlashAsKey (64.27s)
--- PASS: TestAccAWSS3BucketObject_empty (64.79s)
--- PASS: TestAccDataSourceAWSS3BucketObject_kmsEncrypted (64.86s)
--- PASS: TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOn (66.28s)
--- PASS: TestAccDataSourceAWSS3BucketObject_allParams (67.43s)
--- PASS: TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOff (67.58s)
--- PASS: TestAccAWSS3BucketObject_source (61.08s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_basic (111.32s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_all (113.27s)
--- PASS: TestAccDataSourceAWSS3BucketObject_MultipleSlashes (114.34s)
--- PASS: TestAccDataSourceAWSS3BucketObject_LeadingSlash (114.58s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_fetchOwner (114.71s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_prefixes (115.81s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_encoded (115.86s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_startAfter (117.67s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_basicViaAccessPoint (117.62s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_maxKeys (117.94s)
--- PASS: TestAccAWSS3BucketObject_etagEncryption (61.40s)
--- PASS: TestAccAWSS3BucketObject_content (63.72s)
--- PASS: TestAccAWSS3BucketObject_contentBase64 (64.53s)
--- PASS: TestAccAWSS3BucketObject_withContentCharacteristics (63.95s)
--- PASS: TestAccAWSS3BucketObject_NonVersioned (65.44s)
--- PASS: TestAccAWSS3BucketObject_kms (63.68s)
--- PASS: TestAccAWSS3BucketObject_sse (59.82s)
--- PASS: TestAccAWSS3BucketObject_updateSameFile (108.48s)
--- PASS: TestAccAWSS3BucketObject_updates (111.93s)
--- PASS: TestAccAWSS3BucketObject_updatesWithVersioning (110.85s)
--- PASS: TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint (111.53s)
--- PASS: TestAccAWSS3BucketObject_defaultBucketSSE (56.64s)
--- PASS: TestAccAWSS3ObjectCopy_basic (56.66s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithOn (90.43s)
--- PASS: TestAccAWSS3BucketObject_ignoreTags (85.50s)
--- PASS: TestAccAWSS3BucketObject_acl (117.94s)
--- PASS: TestAccAWSS3BucketObject_metadata (119.29s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithNone (116.09s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockRetentionStartWithNone (109.16s)
--- PASS: TestAccAWSS3BucketObject_tagsMultipleSlashes (130.43s)
--- PASS: TestAccAWSS3BucketObject_tagsLeadingMultipleSlashes (132.85s)
--- PASS: TestAccAWSS3BucketObject_tags (134.48s)
--- PASS: TestAccAWSS3BucketObject_tagsLeadingSingleSlash (133.41s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockRetentionStartWithSet (122.64s)
--- PASS: TestAccAWSS3BucketObject_storageClass (142.34s)
GovCloud partition
--- PASS: TestAccDataSourceAWSS3BucketObject_SingleSlashAsKey (46.04s)
--- PASS: TestAccDataSourceAWSS3BucketObject_basic (46.24s)
--- PASS: TestAccDataSourceAWSS3BucketObject_basicViaAccessPoint (46.24s)
--- PASS: TestAccDataSourceAWSS3BucketObject_kmsEncrypted (46.65s)
--- PASS: TestAccDataSourceAWSS3BucketObject_readableBody (47.40s)
--- PASS: TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOn (47.55s)
--- PASS: TestAccDataSourceAWSS3BucketObject_ObjectLockLegalHoldOff (47.85s)
--- PASS: TestAccDataSourceAWSS3BucketObject_allParams (48.31s)
--- PASS: TestAccAWSS3BucketObject_noNameNoKey (14.46s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_encoded (84.34s)
--- PASS: TestAccDataSourceAWSS3BucketObject_MultipleSlashes (85.05s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_basic (85.75s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_prefixes (86.46s)
--- PASS: TestAccDataSourceAWSS3BucketObject_LeadingSlash (86.02s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_all (86.19s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_basicViaAccessPoint (88.39s)
--- PASS: TestAccAWSS3BucketObject_empty (45.17s)
--- PASS: TestAccAWSS3BucketObject_content (46.48s)
--- PASS: TestAccAWSS3BucketObject_etagEncryption (46.04s)
--- PASS: TestAccAWSS3BucketObject_source (47.34s)
--- PASS: TestAccAWSS3BucketObject_contentBase64 (48.19s)
--- PASS: TestAccAWSS3BucketObject_withContentCharacteristics (48.13s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_maxKeys (86.23s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_startAfter (86.78s)
--- PASS: TestAccAWSS3BucketObject_NonVersioned (47.99s)
--- PASS: TestAccAWSS3BucketObject_kms (46.26s)
--- PASS: TestAccDataSourceAWSS3BucketObjects_fetchOwner (88.46s)
--- PASS: TestAccAWSS3BucketObject_sse (45.57s)
--- PASS: TestAccAWSS3BucketObject_updateSameFile (82.10s)
--- PASS: TestAccAWSS3BucketObject_updates (84.08s)
--- PASS: TestAccAWSS3BucketObject_updatesWithVersioning (84.95s)
--- PASS: TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint (86.55s)
--- PASS: TestAccAWSS3BucketObject_metadata (116.25s)
--- PASS: TestAccAWSS3BucketObject_defaultBucketSSE (43.76s)
--- PASS: TestAccAWSS3BucketObject_acl (117.96s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithOn (81.39s)
--- PASS: TestAccAWSS3ObjectCopy_basic (44.82s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockLegalHoldStartWithNone (101.64s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockRetentionStartWithNone (100.87s)
--- PASS: TestAccAWSS3BucketObject_ignoreTags (66.85s)
--- PASS: TestAccAWSS3BucketObject_tags (131.22s)
--- PASS: TestAccAWSS3BucketObject_storageClass (152.12s)
--- PASS: TestAccAWSS3BucketObject_tagsLeadingSingleSlash (115.50s)
--- PASS: TestAccAWSS3BucketObject_tagsLeadingMultipleSlashes (115.33s)
--- PASS: TestAccAWSS3BucketObject_tagsMultipleSlashes (114.98s)
--- PASS: TestAccAWSS3BucketObject_ObjectLockRetentionStartWithSet (110.35s)
Co-authored-by: Graham Davison <[email protected]>
Co-authored-by: Graham Davison <[email protected]>
Co-authored-by: Graham Davison <[email protected]>
Co-authored-by: Graham Davison <[email protected]>
Co-authored-by: Graham Davison <[email protected]>
This has been released in version 3.29.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! |
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! |
Community Note
Closes #15277
This is designed differently than the typical resource with its focus on object copying.
Release note for CHANGELOG:
The output from acceptance testing: