Fix multipart uploads with checksums on object locked buckets

[avantgardnerio]

Which issue does this PR close?

Closes #6793.

Rationale for this change

Described in issue

What changes are included in this PR?

A failing test

Are there any user-facing changes?

Multipart uploads fail

[thinkharderdev]

So there are a couple of issues with how multi-part upload is implemented:

1. The initial PUT request to CreateMultipartUpload must specify the checksum algo or else calling PutPart with the checksum will fail (eg set x-amz-checksum-algorithm: SHA256 on the request)
2. When calling CompleteMultipartUpload we need to send all the part checksums in the request (see `https://docs.aws.amazon.com/AmazonS3/latest/API/API_CompleteMultipartUpload.html)

So this means that we need to keep the checksums in the PartId for when we complete the upload

[tustvold]

I'm not sure about this, an AWS specific detail now leaks into the public trait for all stores... It also is not immediately obvious when the checksums are needed or not...

I think this needs a bit more thought, I'll try to find some time in the next few weeks

[avantgardnerio]

this, an AWS specific detail now leaks into the public trait for all stores... It also is not immediately obvious when the checksums are needed or not...

FWIW: I verified in the new tests we are now writing the checksums at the correct times for when the digest functionality is enabled.

Now I am just trying to get the original tests working for when it isn't.

[thinkharderdev]

It also is not immediately obvious when the checksums are needed or not...

The AWS docs are not very helpful here but we pieced together that to write to an S3 bucket using multi-part with object locking enabled you need to:

1. Specify the checksum algo in the initial API call to CreateMultiPartUpload
2. Send the checksum for each part while calling PutPart
3. When calling CompleteMultipartUpload you need to send the checksum again for each part that was uploaded

The current implementation does 2 but not 1 or 3.

[avantgardnerio]

The current implementation does 2 but not 1 or 3.

I'd like to re-iterate that this PR is functionally complete. Although I am unsure about the docs, I am sure it works.

Now I'm just trying to fix the existing s3_test.

[thinkharderdev]

The current implementation does 2 but not 1 or 3.

I'd like to re-iterate that this PR is functionally complete. Although I am unsure about the docs, I am sure it works.

Now I'm just trying to fix the existing s3_test.

Sorry, by current implementation I meant what is on master not what is in this PR (which does it correctly) 😄

Was just trying to clarify for @tustvold based on his earlier comment

[tustvold]

Was just trying to clarify for @tustvold based on his earlier comment

Right, my comment pertained to the changes to the public API, we now have a somewhat peculiar quirk of AWS leaking out into the public API in a manner that also makes it very unclear when or if I need to specify the checksum or not. I need to find some time to sit down and think about how we could support this, ideally without making a breaking API change.

[tustvold]

So I think there is an less intrusive way to support this.

UploadPart is not an issue as the per-part meta is never exposed.

The issue solely concerns MultipartStore, in particular how to carry the checksum through https://docs.rs/object_store/latest/object_store/multipart/struct.PartId.html.

Now we could add a checksum field to this, but this would be a breaking change. However, the content id is intentionally opaque. We could therefore just use the XML encoded https://docs.aws.amazon.com/AmazonS3/latest/API/API_CompletedPart.html instead of an ETag when necessary.

There would need to still be logic to handle an ETag based PartId, as these may have been stored in a database, but detecting this should be trivial e.g. by attempting to decode the XML.

This would avoid needing to make any breaking changes, whilst also being future proof for any of the other checksum types

[avantgardnerio]

We could therefore just use the XML encoded

@tustvold done

[tustvold]

I think it is important we don't add implicit checksum enablement, not only because this could break workloads (e.g. those that aren't actually S3), but also because S3 recently added a load of new checksum algorithms

[tustvold]

Just some minor nits

[avantgardnerio]

@tustvold some unrelated tests started failing. Do you know what's up?

https://github.com/apache/arrow-rs/actions/runs/12221975779/job/34091689971?pr=6794