Skip to content

rgw: fix checksum for get part request#66042

Merged
cbodley merged 1 commit intoceph:mainfrom
sungjoon-koh:fix-cksum-part-get
Dec 4, 2025
Merged

rgw: fix checksum for get part request#66042
cbodley merged 1 commit intoceph:mainfrom
sungjoon-koh:fix-cksum-part-get

Conversation

@sungjoon-koh
Copy link
Copy Markdown
Contributor

@sungjoon-koh sungjoon-koh commented Oct 23, 2025 •
edited
Loading

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

You must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.

@sungjoon-koh
rgw: fix checksum for get part request
9b43590 
Signed-off-by: sungjoon_koh <sungjoon_koh@linecorp.com>
@sungjoon-koh sungjoon-koh requested a review from a team as a code owner October 23, 2025 08:40
@github-actions github-actions Bot added the rgw label Oct 23, 2025
@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Oct 23, 2025 •
edited
Loading

I made a PR to align GetObject checksum result with AWS S3 spec for partNumber. I appreciate it if you could review this.

  1. For a multipart object with a COMPOSITE type checksum, the GetObject result with a specified partNumber should return the checksum without the part count suffix.

For example:
The checksum for GetObject?partNumber=1 should be "ChecksumSHA1": "LpXXWCxTWD+or7VOD+eiWXySy7o=" instead of "ChecksumSHA1": "LpXXWCxTWD+or7VOD+eiWXySy7o=-2".

  • Before patch
    SHA1 & COMPOSITE
# Upload part 1 response
{
    "ETag": "\"5f363e0e58a95f06cbe9bbc662c5dfb6\"",
    "ChecksumSHA1": "LpXXWCxTWD+or7VOD+eiWXySy7o="
}

# Upload part 2 response
{
    "ETag": "\"5f363e0e58a95f06cbe9bbc662c5dfb6\"",
    "ChecksumSHA1": "LpXXWCxTWD+or7VOD+eiWXySy7o="
}

# Complete multipart upload result
{
    "Location": "http://localhost:8000/test/bigobject",
    "Bucket": "test",
    "Key": "bigobject",
    "ETag": "\"a7d414b9133d6483d9a1c4e04e856e3b-2\"",
    "ChecksumSHA1": "1XbVAA8k4RZwh70dzyuieeNOggk=-2",
    "ChecksumType": "COMPOSITE"
}

# GetObject with partNumber result
{
    "AcceptRanges": "bytes",
    "LastModified": "2025-10-13T15:29:30+00:00",
    "ContentLength": 5242880,
    "ETag": "\"5f363e0e58a95f06cbe9bbc662c5dfb6\"",
    "ChecksumSHA1": "LpXXWCxTWD+or7VOD+eiWXySy7o=-2",
    "ChecksumType": "COMPOSITE",
    "ContentType": "binary/octet-stream",
    "Metadata": {},
    "PartsCount": 2
}
  • After patch
# Upload part 1 response
{
    "ETag": "\"5f363e0e58a95f06cbe9bbc662c5dfb6\"",
    "ChecksumSHA1": "LpXXWCxTWD+or7VOD+eiWXySy7o="
}

# Upload part 2 response
{
    "ETag": "\"5f363e0e58a95f06cbe9bbc662c5dfb6\"",
    "ChecksumSHA1": "LpXXWCxTWD+or7VOD+eiWXySy7o="
}

# Complete multipart upload response
{
    "Location": "http://localhost:8000/test/bigobject",
    "Bucket": "test",
    "Key": "bigobject",
    "ETag": "\"a7d414b9133d6483d9a1c4e04e856e3b-2\"",
    "ChecksumSHA1": "1XbVAA8k4RZwh70dzyuieeNOggk=-2",
    "ChecksumType": "COMPOSITE"
}

# GetObject with partNumber result
{
    "AcceptRanges": "bytes",
    "LastModified": "2025-10-23T08:48:44+00:00",
    "ContentLength": 5242880,
    "ETag": "\"5f363e0e58a95f06cbe9bbc662c5dfb6\"",
    "ChecksumSHA1": "LpXXWCxTWD+or7VOD+eiWXySy7o=",
    "ChecksumType": "COMPOSITE",
    "ContentType": "binary/octet-stream",
    "Metadata": {},
    "PartsCount": 2
}

@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Oct 23, 2025 •
edited
Loading

  1. For a multipart object with a FULL_OBJECT type checksum, the GetObject result with a specified partNumber should return the checksum type as FULL_OBJECT and the checksum value without the part count suffix.
  • AWS S3 Spec: For objects with the FULL_OBJECT checksum type, AWS S3 does not return a checksum for GetObject?partNumber=1.
    However, I believe returning it would be useful rather than skipping it, since we already store the checksum value.

Example (CRC64NVME, FULL_OBJECT):
The checksum for GetObject?partNumber=1 should be
"ChecksumType": "FULL_OBJECT", "ChecksumCRC64NVME": "vqf3hRLTlJw="
instead of
"ChecksumType": "COMPOSITE", "ChecksumCRC64NVME": "vqf3hRLTlJw=-2".

  • Before patch
    CRC64NVME & FULL_OBJECT
{
    "ETag": "\"5f363e0e58a95f06cbe9bbc662c5dfb6\"",
    "ChecksumCRC64NVME": "vqf3hRLTlJw="
}
{
    "ETag": "\"5f363e0e58a95f06cbe9bbc662c5dfb6\"",
    "ChecksumCRC64NVME": "vqf3hRLTlJw="
}
{
    "Location": "http://localhost:8000/test/bigobject",
    "Bucket": "test",
    "Key": "bigobject",
    "ETag": "\"a7d414b9133d6483d9a1c4e04e856e3b-2\"",
    "ChecksumCRC64NVME": "r3XRsudNx4U=",
    "ChecksumType": "FULL_OBJECT"
}
{
    "AcceptRanges": "bytes",
    "LastModified": "2025-10-13T15:29:46+00:00",
    "ContentLength": 5242880,
    "ETag": "\"5f363e0e58a95f06cbe9bbc662c5dfb6\"",
    "ChecksumCRC64NVME": "vqf3hRLTlJw=-2",
    "ChecksumType": "COMPOSITE",
    "ContentType": "binary/octet-stream",
    "Metadata": {},
    "PartsCount": 2
}
  • After patch:
{
    "ETag": "\"5f363e0e58a95f06cbe9bbc662c5dfb6\"",
    "ChecksumCRC64NVME": "vqf3hRLTlJw="
}
{
    "ETag": "\"5f363e0e58a95f06cbe9bbc662c5dfb6\"",
    "ChecksumCRC64NVME": "vqf3hRLTlJw="
}
{
    "Location": "http://localhost:8000/test/bigobject",
    "Bucket": "test",
    "Key": "bigobject",
    "ETag": "\"a7d414b9133d6483d9a1c4e04e856e3b-2\"",
    "ChecksumCRC64NVME": "r3XRsudNx4U=",
    "ChecksumType": "FULL_OBJECT"
}
{
    "AcceptRanges": "bytes",
    "LastModified": "2025-10-23T08:49:01+00:00",
    "ContentLength": 5242880,
    "ETag": "\"5f363e0e58a95f06cbe9bbc662c5dfb6\"",
    "ChecksumCRC64NVME": "vqf3hRLTlJw=",
    "ChecksumType": "FULL_OBJECT",
    "ContentType": "binary/octet-stream",
    "Metadata": {},
    "PartsCount": 2
}

@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Oct 23, 2025

If it seems reasonable, I will add some tests to s3-tests. Thank you!

mattbenjamin
mattbenjamin approved these changes Oct 28, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@mattbenjamin mattbenjamin left a comment

Choose a reason for hiding this comment

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

hi!
is there an external reference that clarifies that FULL_OBJECT is the correct checksumType for part checksums? Also, would you be able to write an s3-tests test for this case? We definitely don't have one.

@sungjoon-koh sungjoon-koh mentioned this pull request Oct 29, 2025
Merged
@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Oct 29, 2025 •
edited
Loading

@mattbenjamin

is there an external reference that clarifies that FULL_OBJECT is the correct checksumType for part checksums?

No — in AWS S3, when the checksumType is FULL_OBJECT, it doesn’t return any checksum for individual parts — I’m not sure why. I updated it, as it seems more reasonable to return it rather than a composite value. While we could omit it, since it’s already stored, returning it might be useful. I’d like to get your opinion on this.

Also, would you be able to write an s3-tests test for this case? We definitely don't have one.

I’ve added a test for this PR: ceph/s3-tests#704
Could you please take a look? Thank you!

@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Oct 29, 2025

jenkins test windows

@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Dec 2, 2025

@mattbenjamin When you get a chance, please take a look at this PR. Thank you.

@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Dec 2, 2025

i prepared a test branch https://github.com/ceph/ceph-ci/commits/wip-cbodley-testing that includes #66042 and #65194, and points to an s3-tests branch https://github.com/cbodley/s3-tests/commits/wip-cbodley-testing that includes ceph/s3-tests#704 and ceph/s3-tests#699

@cbodley cbodley mentioned this pull request Dec 2, 2025
Merged
14 tasks
@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Dec 2, 2025

qa pending in https://pulpito.ceph.com/cbodley-2025-12-02_17:25:51-rgw-wip-cbodley-testing-distro-default-smithi/

@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Dec 4, 2025

qa pending in https://pulpito.ceph.com/cbodley-2025-12-02_17:25:51-rgw-wip-cbodley-testing-distro-default-smithi/

passed qa. confirmed that the updated test cases are passing (see example teuthology.log):

s3tests/functional/test_s3.py::test_multipart_use_cksum_helper_sha256 PASSED [ 56%]
s3tests/functional/test_s3.py::test_multipart_use_cksum_helper_crc64nvme PASSED [ 56%]
s3tests/functional/test_s3.py::test_multipart_use_cksum_helper_crc32 PASSED [ 56%]
s3tests/functional/test_s3.py::test_multipart_use_cksum_helper_crc32c PASSED [ 56%]
s3tests/functional/test_s3.py::test_multipart_use_cksum_helper_sha1 PASSED [ 56%]

@cbodley cbodley merged commit 590ef38 into ceph:main Dec 4, 2025
22 of 26 checks passed
@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Dec 4, 2025

is there a tracker issue so we can backport this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattbenjamin mattbenjamin mattbenjamin approved these changes

Assignees

No one assigned

Labels

rgw wip-cbodley-testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sungjoon-koh @cbodley @mattbenjamin