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

Add user provided SSE-C arguments to CompleteMultipartUpload call #274

Merged
merged 1 commit into from
Sep 25, 2023
Merged

Add user provided SSE-C arguments to CompleteMultipartUpload call #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion s3transfer/copies.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,13 @@ class CopySubmissionTask(SubmissionTask):
'TaggingDirective',
]

COMPLETE_MULTIPART_ARGS = ['RequestPayer', 'ExpectedBucketOwner']
COMPLETE_MULTIPART_ARGS = [
'SSECustomerKey',
'SSECustomerAlgorithm',
'SSECustomerKeyMD5',
'RequestPayer',
'ExpectedBucketOwner',
]

def _submit(
self, client, config, osutil, request_executor, transfer_future
Expand Down
8 changes: 7 additions & 1 deletion s3transfer/upload.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,13 @@ class UploadSubmissionTask(SubmissionTask):
'ExpectedBucketOwner',
]

COMPLETE_MULTIPART_ARGS = ['RequestPayer', 'ExpectedBucketOwner']
COMPLETE_MULTIPART_ARGS = [
'SSECustomerKey',
'SSECustomerAlgorithm',
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

'SSECustomerKeyMD5',
'RequestPayer',
'ExpectedBucketOwner',
]

def _get_upload_input_manager_cls(self, transfer_future):
"""Retrieves a class for managing input for an upload based on file type
Expand Down
4 changes: 3 additions & 1 deletion tests/functional/test_copy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,9 @@ def test_copy_passes_args_to_create_multipart_and_upload_part(self):
self.add_head_object_response(expected_params=head_params)

self._add_params_to_expected_params(
add_copy_kwargs, ['create_mpu', 'copy'], self.extra_args
add_copy_kwargs,
['create_mpu', 'copy', 'complete_mpu'],
self.extra_args,
)
self.add_successful_copy_responses(**add_copy_kwargs)

Expand Down
25 changes: 25 additions & 0 deletions tests/functional/test_upload.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,3 +599,28 @@ def test_multipart_upload_passes_checksums(self):
)
future.result()
self.assert_expected_client_calls_were_correct()

def test_multipart_upload_with_ssec_args(self):
params = {
'RequestPayer': 'requester',
'SSECustomerKey': 'key',
'SSECustomerAlgorithm': 'AES256',
'SSECustomerKeyMD5': 'key-hash',
}
self.extra_args.update(params)

self.add_create_multipart_response_with_default_expected_params(
extra_expected_params=params
)

self.add_upload_part_responses_with_default_expected_params(
extra_expected_params=params
)
self.add_complete_multipart_response_with_default_expected_params(
extra_expected_params=params
)
future = self.manager.upload(
self.filename, self.bucket, self.key, self.extra_args
)
future.result()
self.assert_expected_client_calls_were_correct()