Wip cbodley multipart nostreaming #564
Conversation
Signed-off-by: Casey Bodley <cbodley@redhat.com>
this removes a Pytest warning during execution Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
…-multipart As described in https://tracker.ceph.com/issues/65746, retrying complete-multipart after having attempted to complete the same upload with a bad checksum argument fails with an internal error. The status code is 500, but I'm unsure if it can be retried again, or whether the upload can be aborted later. Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
tests a full multipart upload cycle with 3 unique parts, which verifies composite checksum computation and the logic to propagate parts_count to ComleteMultipart Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
| response = client.complete_multipart_upload(Bucket=bucket, Key=key, UploadId=upload_id, ChecksumSHA256=composite_sha256sum, MultipartUpload={'Parts': [ | ||
| {'ETag': etag1, 'ChecksumSHA256': response['ChecksumSHA256'], 'PartNumber': 1}, | ||
| {'ETag': etag2, 'ChecksumSHA256': response['ChecksumSHA256'], 'PartNumber': 2}, | ||
| {'ETag': etag3, 'ChecksumSHA256': response['ChecksumSHA256'], 'PartNumber': 3}]}) |
There was a problem hiding this comment.
this is passing the same ChecksumSHA256 value for each part, which comes from the last upload_part() response. CompleteMultipart should check that these checksums match the parts and reject such a request
you should be able to pass part1_sha256sum etc for these values
| # should reject the missing part checksum | ||
| e = assert_raises(ClientError, client.complete_multipart_upload, Bucket=bucket, Key=key, UploadId=upload_id, ChecksumSHA256='bad', MultipartUpload={'Parts': [ | ||
| {'ETag': response['ETag'].strip('"'), 'PartNumber': 1}]}) |
There was a problem hiding this comment.
oops, i'd made a mistake here - above we already tested that ChecksumSHA256='bad' leads to 400 InvalidRequest
this one was meant to test the case where ChecksumSHA256 is missing from the Parts array
ChecksumSHA256='bad' here should pass the correct value ChecksumSHA256=composite_sha256sum (as below) so that we correctly test the Parts behavior
since test_multipart_checksum_3parts is passing with the wrong checksum values in Parts, i think ceph/ceph#55076 is missing the logic to validate those
There was a problem hiding this comment.
oh, true; it validates the checksums of the parts as actually uploaded, though
this tests a two-megabyte binary upload with validated (awscli-computed) SHA256 checksum, and also verifies failure when a bad checksum is provided Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
| size = 1024 | ||
| body = FakeWriteFile(size, 'A') |
There was a problem hiding this comment.
| size = 1024 | |
| body = FakeWriteFile(size, 'A') | |
| body = FakeWriteFile(1024, 'A') |
Since it is not used inside the test except this place, can some some memory to not assign a variable to it :)
| assert 'SHA256' == response['ChecksumAlgorithm'] | ||
| upload_id = response['UploadId'] | ||
|
|
||
| size = 1024 |
There was a problem hiding this comment.
| size = 1024 |
Can use size variable from 13466 line
| assert 'SHA256' == response['ChecksumAlgorithm'] | ||
| upload_id = response['UploadId'] | ||
|
|
||
| size = 1024 |
There was a problem hiding this comment.
| size = 1024 |
Same here
No description provided.