S3: implement new data integrity for MultipartUpload #12183

bentsku · 2025-01-25T04:37:02Z

Motivation

This PR implements the new data integrity checks for Multipart Upload (shortened MPU), namely the ability to set the FULL_OBJECT checksum type for a MPU, allowing the user to not keep track of individual checksum parts to calculate the final one, but instead send the full object checksum to S3. This can be done thanks to CRC* type checksums, which can linearize, meaning you can get the full object checksum from the individual parts' checksums (via a somewhat complicated function).

The PR looks big (3k lines), but 70% of it is snapshots, because I had to cover a lot of different cases, operations' responses, and there was a lot of ground to cover that wasn't previously covered.

There is a lot of additional validation and default values to check (different relationship between checksum algorithm and checksum type), new validation of the MPU total size at completion time, I've also ported the fix with different error message depending if the checksum value is valid or not.

The biggest part and challenge of this PR is to implement to logic to combine CRC checksums together in order to create the FULL_OBJECT checksum. More can be read about this in the following Stack Overflow post: https://stackoverflow.com/a/23126768
I had to look everywhere online to try to find someway to do that, and it was pretty hard to find. I found an old repository of Alibaba, with the function available in Python2 to combine CRC64 checksums, I had to adapt it a little to run with Python3 and fit our use case better.

More resources to read about the new S3 data integrity push:

There's also a bit of cleanup done, removing some unused functions/code.

Changes

implement the new checksum types FULL_OBJECT and COMPOSITE for MPU
implement the helpers to combine CRC checksums together
clean up some dead code
implement a lot of new tests covering as wide as I could think

bentsku · 2025-01-25T04:38:51Z

tests/unit/test_s3.py

This file was testing botocore code, so it wasn't really useful

bentsku · 2025-01-25T04:39:32Z

tests/aws/services/s3/test_s3.py

Sorry for all the duplication in the tests, multipart uploads are super verbose and annoying to work with

bentsku · 2025-01-25T04:40:11Z

localstack-core/localstack/services/s3/utils.py

-def get_object_checksum_for_algorithm(checksum_algorithm: str, data: bytes) -> str:
-    match checksum_algorithm:
-        case ChecksumAlgorithm.CRC32:
-            return checksum_crc32(data)
-
-        case ChecksumAlgorithm.CRC32C:
-            return checksum_crc32c(data)
-
-        case ChecksumAlgorithm.SHA1:
-            return hash_sha1(data)
-
-        case ChecksumAlgorithm.SHA256:
-            return hash_sha256(data)
-
-        case _:
-            # TODO: check proper error? for now validated client side, need to check server response
-            raise InvalidRequest("The value specified in the x-amz-trailer header is not supported")
-
-
-def verify_checksum(checksum_algorithm: str, data: bytes, request: Dict):
-    # TODO: you don't have to specify the checksum algorithm
-    # you can use only the checksum-{algorithm-type} header
-    # https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html
-    key = f"Checksum{checksum_algorithm.upper()}"
-    # TODO: is there a message if the header is missing?
-    checksum = request.get(key)
-    calculated_checksum = get_object_checksum_for_algorithm(checksum_algorithm, data)
-
-    if calculated_checksum != checksum:
-        raise InvalidRequest(
-            f"Value for x-amz-checksum-{checksum_algorithm.lower()} header is invalid."
-        )
-
-


this code wasn't used anymore anywhere, since we're properly calculating checksums on streams instead of the full string

bentsku · 2025-01-25T04:40:41Z

localstack-core/localstack/services/s3/utils.py

@@ -454,40 +442,6 @@ def base_64_content_md5_to_etag(content_md5: ContentMD5) -> str | None:
    return hex_digest


-def decode_aws_chunked_object(


we did not use this code anymore, as localstack.services.s3.codec.AwsChunkedDecoder superseded it

github-actions · 2025-01-25T04:45:34Z

S3 Image Test Results (AMD64 / ARM64)

2 files ± 0 2 suites ±0 8m 8s ⏱️ + 2m 58s
475 tests +29 423 ✅ +29 52 💤 ±0 0 ❌ ±0
950 runs +58 846 ✅ +58 104 💤 ±0 0 ❌ ±0

Results for commit c3b2220. ± Comparison against base commit faef710.

This pull request removes 2 and adds 31 tests. Note that renamed tests count towards both.

tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_multipart_parts_checksum_exceptions

tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_composite[CRC32C]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_composite[CRC32]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_composite[SHA1]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_composite[SHA256]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_default
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_full_object[CRC32C]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_full_object[CRC32]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_full_object[CRC64NVME]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_multipart_checksum_type_compatibility[COMPOSITE-CRC32C]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_multipart_checksum_type_compatibility[COMPOSITE-CRC32]
…

♻️ This comment has been updated with latest results.

This reverts commit ad6e619.

github-actions · 2025-01-27T18:44:20Z

LocalStack Community integration with Pro

2 files ± 0 2 suites ±0 1h 53m 49s ⏱️ + 1m 6s
4 070 tests +46 3 755 ✅ +46 315 💤 ±0 0 ❌ ±0
4 072 runs +46 3 755 ✅ +46 317 💤 ±0 0 ❌ ±0

Results for commit c3b2220. ± Comparison against base commit faef710.

This pull request removes 2 and adds 48 tests. Note that renamed tests count towards both.

tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_multipart_parts_checksum_exceptions

tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_composite[CRC32C]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_composite[CRC32]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_composite[SHA1]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_composite[SHA256]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_default
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_full_object[CRC32C]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_full_object[CRC32]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_complete_multipart_parts_checksum_full_object[CRC64NVME]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_multipart_checksum_type_compatibility[COMPOSITE-CRC32C]
tests.aws.services.s3.test_s3.TestS3MultipartUploadChecksum ‑ test_multipart_checksum_type_compatibility[COMPOSITE-CRC32]
…

♻️ This comment has been updated with latest results.

k-a-il

Nice work! Code and the whole solution is easy to understand thanks to comments :)

bentsku added aws:s3 Amazon Simple Storage Service semver: patch Non-breaking changes which can be included in patch releases labels Jan 25, 2025

bentsku added this to the 4.1 milestone Jan 25, 2025

bentsku self-assigned this Jan 25, 2025

bentsku commented Jan 25, 2025

View reviewed changes

bentsku added semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases and removed semver: patch Non-breaking changes which can be included in patch releases labels Jan 25, 2025

bentsku changed the title ~~S3: implement S3 checksum MultipartUpload FULL_OBJECT~~ S3: implement new data integrity for MultipartUpload Jan 25, 2025

bentsku force-pushed the s3-feat-checksum branch 2 times, most recently from 1167d27 to 0c5c40e Compare January 27, 2025 10:38

Base automatically changed from s3-feat-checksum-1 to master January 27, 2025 17:35

bentsku added 12 commits January 27, 2025 18:36

Revert "remove multipart logic for follow-up"

7b5ce52

This reverts commit ad6e619.

refactor tests and write Multipart tests

9a5c2f2

implement ChecksumType compat map

02dca36

implement test for FULL_OBJECT

b3ae05b

implement FULL_OBJECT checksum calculation

a406cf7

implement FULL_OBJECT validation

10bc6f8

remove data integrity markers

5bbec95

implement default behavior, verify types logic

26b9ae4

add mpu size validation + rework

e2886d0

small updates for backward compatibility

d005f80

remove comments

50a010c

fix exception message for wrong checksum in UploadPart

e6fde0e

bentsku force-pushed the s3-feat-checksum branch from 0c5c40e to 1d155af Compare January 27, 2025 17:36

update logic to also work with trailing headers

7fc465e

bentsku force-pushed the s3-feat-checksum branch from 1d155af to 7fc465e Compare January 27, 2025 17:37

bentsku requested a review from k-a-il January 27, 2025 17:37

bentsku marked this pull request as ready for review January 27, 2025 21:58

k-a-il approved these changes Jan 28, 2025

View reviewed changes

add comments

c3b2220

bentsku merged commit 25b920c into master Jan 28, 2025
38 checks passed

bentsku deleted the s3-feat-checksum branch January 28, 2025 15:05

alexrashed mentioned this pull request Feb 3, 2025

Update CODEOWNERS #12219

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

S3: implement new data integrity for MultipartUpload #12183

S3: implement new data integrity for MultipartUpload #12183

Uh oh!

bentsku commented Jan 25, 2025 •

edited

Loading

Uh oh!

bentsku Jan 25, 2025

Uh oh!

bentsku Jan 25, 2025

Uh oh!

bentsku Jan 25, 2025

Uh oh!

bentsku Jan 25, 2025

Uh oh!

github-actions bot commented Jan 25, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jan 27, 2025 •

edited

Loading

Uh oh!

k-a-il left a comment

Uh oh!

Uh oh!

Uh oh!

		@@ -454,40 +442,6 @@ def base_64_content_md5_to_etag(content_md5: ContentMD5) -> str \| None:
		return hex_digest


		def decode_aws_chunked_object(

Uh oh!

S3: implement new data integrity for MultipartUpload #12183

S3: implement new data integrity for MultipartUpload #12183

Uh oh!

Conversation

bentsku commented Jan 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Uh oh!

bentsku Jan 25, 2025

Choose a reason for hiding this comment

Uh oh!

bentsku Jan 25, 2025

Choose a reason for hiding this comment

Uh oh!

bentsku Jan 25, 2025

Choose a reason for hiding this comment

Uh oh!

bentsku Jan 25, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

S3 Image Test Results (AMD64 / ARM64)

Uh oh!

github-actions bot commented Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LocalStack Community integration with Pro

Uh oh!

k-a-il left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bentsku commented Jan 25, 2025 •

edited

Loading

github-actions bot commented Jan 25, 2025 •

edited

Loading

github-actions bot commented Jan 27, 2025 •

edited

Loading