Commit Graph

7 Commits

Author SHA1 Message Date
Feng Ruohang 25cb3511c9 test: cover multipart SSE sources on federated CopyObject
A multipart SSE-S3 source is encrypted per part, so its logical size is
the sum of the parts' decrypted sizes and the decrypting reader crosses
a part boundary. Copy such a source across the federation to a plain
and to an SSE-S3 destination and check the destination plaintext and
single-encryption size.

The test router registers routes in endpoint order and the plain
PutObject route has no query matcher, so the multipart endpoints are
listed first.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FodsDpa6VkghaeRE6WjmEe
Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-09-09 17:51:32 +08:00
Feng Ruohang cfefc049c1 fix: send the SSE-KMS context as a JSON object on federated copies
putOptsFromReq handed the parsed kms.Context straight to
encrypt.NewSSEKMS. kms.Context implements encoding.TextMarshaler, so the
SDK serialized it as a JSON string, and a request without a context
still produced one because the nil Context is a typed nil inside the
interface value and marshals to "{}". The receiving ParseHTTP rejects
both forms, so every federated CopyObject to an SSE-KMS destination
failed with InvalidArgument once the forwarded stream was correct.

Pass a plain map, or nothing when no context was requested, and cover
SSE-KMS destinations with and without an explicit context.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FodsDpa6VkghaeRE6WjmEe
Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-09-09 17:51:32 +08:00
Feng Ruohang af56d17630 fix: forward plaintext on federated CopyObject of SSE objects (#158)
The legacy etcd bucket-federation branch of CopyObjectHandler reads its
source through getObjectNInfo, which yields the decrypted and
decompressed bytes, but it also ran the destination encryption locally
and then forwarded that stream to the remote PutObject with the source's
stored size and the destination SSE option. SSE to plain and plain to
SSE therefore failed on a Content-Length mismatch, while SSE to SSE
matched by coincidence: the remote encrypted the ciphertext a second
time and stored an unreadable object, and a destination GET returned
the inner ciphertext with HTTP 200.

The remote write owns the destination's storage transformations, so
hand it the logical bytes at their logical size and let it encrypt
exactly once. Compression was already excluded on this branch; apply
the same rule to encryption, size the forwarded reader by actualSize,
and declare that size on the forwarded PutObject.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FodsDpa6VkghaeRE6WjmEe
Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-09-09 17:51:32 +08:00
Feng Ruohang 2c50d11f72 fix: correct federated CopyObject checksum edge cases (#99 follow-ups)
Three residual checksum defects in the legacy etcd federation branch of
CopyObjectHandler, found by post-merge review of #157.

1. Empty-source 500 regression. A checksum-less object gains the S3 default
   CRC-64NVME (WantServerSideChecksumType is set), but minio-go streams no
   trailing checksum for a 0-byte body (contentLength == 0), so the remote
   computed none, federatedChecksumValue was empty, hash.NewChecksumWithType
   returned nil, and the handler returned 500 -- so every empty-object
   federated copy failed. For a 0-byte source, forward the empty-content digest
   as an ordinary checksum request header instead, so the remote validates,
   persists and returns it, matching the local path (e.g. CRC32 "AAAAAA==").

2. Inherited full-object checksum dropped. When the source already carries a
   full-object checksum, the local path sets dstOpts.WantChecksum, not
   WantServerSideChecksumType (only multipart-composite sources are promoted).
   The federated branch inspected only WantServerSideChecksumType, so a
   checksum-bearing source's checksum was silently discarded on a federated
   copy that requested no algorithm. Forward WantChecksum.Encoded (always a
   plain digest) as a checksum header so the remote validates and persists it,
   and bind the returned value, matching local persistence.

3. Multipart-suffixed remote value accepted. The bind accepted a value like
   "NSRBwg==-0": NewChecksumWithType parses the "-N" as ChecksumMultipart with
   WantParts 0 and the length-only validator passes, so the destination was
   returned as COMPOSITE. A single forwarded PutObject must yield a full-object
   digest, so reject a multipart-marked parsed value in addition to the
   existing nil (missing/malformed) rejection.

A forwarded checksum request header is stripped from objInfo.UserDefined so it
is not mistaken for object metadata.

Out of scope: the SSE federated-copy corruption (srcInfo.Reader/Size mismatch
for encrypted sources) predates this work and is filed separately.

New federated regressions cover empty source with requested and default
checksum (200 + correct value + persisted), an inherited full-object checksum
preserved without a requested algorithm, and a multipart-suffixed remote value
rejected. Red/green verified for each against the merged code.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-09-08 15:38:11 +08:00
Feng Ruohang 885bd2c20a fix: reject missing remote checksums on federated copies
Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-09-08 13:05:45 +08:00
Feng Ruohang 4b25f7e819 fix: return and persist checksum on federated CopyObject (#99)
The legacy etcd federation branch of CopyObjectHandler forwards the copied
bytes with minio-go Core.PutObject but never asked the remote for a checksum
and discarded any it returned, so a cross-deployment whole-object copy that
requested a checksum returned 200 with an empty checksum, and a checksum-less
source did not gain the S3 default CRC-64NVME that the local path assigns. The
request was neither honored nor rejected. This is the whole-object counterpart
of #72, which repaired the same class of defect for federated UploadPartCopy.

When a server-side checksum is wanted -- explicitly requested, inherited from a
multipart source, or the CRC-64NVME default for a checksum-less object, all
already captured in dstOpts.WantServerSideChecksumType -- the forwarded
PutObject now streams a trailing checksum of that type, so the remote computes
and persists it and echoes it in the response. The value the remote reports for
that exact write is bound into objInfo.Checksum, matching how the local
CopyObject path carries checksums into the CopyObjectResult. Reading the value
from the same UploadInfo that produced the ETag keeps the pair bound to one
write.

Only the requested algorithm is returned; a malformed or absent remote value
leaves objInfo.Checksum unset, so an ordinary copy that wanted no checksum
still returns none. Two small mapping helpers convert between the server's
hash.ChecksumType and the minio-go request type and response field.

New end-to-end tests drive the real federation branch through
getRemoteInstanceClient and minio-go into a second in-process deployment and
assert that CRC32/CRC32C/SHA256/CRC64NVME and the no-algorithm default are all
returned in the CopyObjectResult and persisted on the destination, and that a
requested algorithm never leaks other algorithms into the response.

Fixes #99

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7qJqWwy8oFA6aCXWRzXQe
Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-09-07 10:32:59 +08:00
Feng Ruohang 2bc103b80c fix: strip all reserved metadata on federated CopyObject (#100)
The legacy etcd federation branch of CopyObjectHandler forwards the copied
source metadata to the remote deployment with minio-go Core.PutObject, after
removing only two reserved keys (compression and actual-size). Every small
object is stored inline, so its stored metadata also carries
X-Minio-Internal-inline-data; the remote's setRequestLimitMiddleware rejects
any request bearing a reserved-prefix header (containsReservedMetadata), so
the forwarded write failed with 400 InvalidArgument "Your metadata headers
are not supported." for the default COPY metadata directive.

A plain federated PutObject must not carry any internal storage metadata, so
strip the whole reserved-prefix class before forwarding instead of an
enumerated subset. Enumerating a third key would only defer the next leak:
besides inline-data, replication bookkeeping (replica/replication status and
timestamps) is added to the same map earlier in the handler and would be
rejected just the same. None of these keys is required by the remote for a
correct plain PutObject; they are internal storage details the remote sets
for itself. The stripping uses stringsHasPrefixFold, matching the remote's
own case-insensitive detection. Ordinary user metadata (x-amz-meta-*) is
untouched and still copied.

The pre-existing UUID ETag on the federated write (no Content-MD5 is sent) is
out of scope and left unchanged, as recorded in the issue.

A new end-to-end test drives the real federation branch through
getRemoteInstanceClient and minio-go into a second in-process deployment,
copying an inline source with the default COPY directive. It asserts the copy
now succeeds, that no reserved-prefix header reaches the remote on any
forwarded request, and that copied user metadata survives.

Fixes #100

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7qJqWwy8oFA6aCXWRzXQe
Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-09-07 10:32:59 +08:00