Add user provided SSE-C arguments to CompleteMultipartUpload call #274
Conversation
nateprewitt
force-pushed
the
SSEC_CMU
branch
3 times, most recently
from
b0bee85
to
6a9ebc7
Compare
s3transfer/__init__.py
Outdated
| @@ -380,6 +380,12 @@ class MultipartUploader: | |||
| 'RequestPayer', | |||
| ] | |||
|
|
|||
| COMPLETE_MULTIPART_ARGS = [ | |||
There was a problem hiding this comment.
Oh we do not need to update this module. This is module is no longer used in Boto3 and CLI. It was the old s3 logic that boto3 used before migrating over to the transfer manager interface. I think we should just make sure the transfer manager code path is updated to support this and not touch this module or it's tests.
There was a problem hiding this comment.
Hmm, all of our upload tests that check this are currently written against this implementation 😬
There was a problem hiding this comment.
I wrote a new test using the right infrastructure to make sure we've got this case covered.
| 'RequestPayer', | ||
| 'ExpectedBucketOwner', | ||
| 'SSECustomerKey', | ||
| 'SSECustomerAlgorithm', |
There was a problem hiding this comment.
Do we need to proxy over the SSECustomerKeyMD5 to the complete MPU as well? I noticed that it was in the upload parts list but not the complete mpu list.
There was a problem hiding this comment.
There was back and forth on this. They said only the Key/Algo for now. I've added what we know we need and we can always extend if there is further feedback. S3 has some knowledge gaps on the full feature scope, so we're meeting the known use case.
There was a problem hiding this comment.
Added it for good measure. After some testing it doesn't have any impact on the call so we might as well be complete and pass it if it was provided.
nateprewitt
force-pushed
the
SSEC_CMU
branch
from
6a9ebc7
to
3e478e8
Compare
nateprewitt
force-pushed
the
SSEC_CMU
branch
from
3e478e8
to
96de9f0
Compare
Codecov ReportPatch coverage:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #274 +/- ##
===========================================
+ Coverage 85.39% 85.54% +0.14%
===========================================
Files 16 16
Lines 2719 2719
===========================================
+ Hits 2322 2326 +4
+ Misses 397 393 -4
☔ View full report in Codecov by Sentry. |
This PR will forward SSE-C arguments provided by the user to the final CompleteMultipartUpload call to handle cases where a policy has been defined which requires these for all PutObject calls.
This is to resolve a recently published workflow by S3 which changed the requiredness of these arguments due to
PutObjectbeing used under the hood for CompleteMultipartUpload.