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-09 12:04:04 +03:00 · 2026-09-07 10:15:15 +08:00
parent 2bc103b80c
commit 4b25f7e819
2 changed files with 166 additions and 0 deletions
@@ -21,6 +21,7 @@ package cmd
 import (
 	"bytes"
 	"encoding/json"
+	"encoding/xml"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -31,6 +32,7 @@ import (
 	"github.com/minio/minio-go/v7/pkg/set"
 	"github.com/minio/minio/internal/auth"
 	"github.com/minio/minio/internal/config/dns"
+	"github.com/minio/minio/internal/hash"
 	xhttp "github.com/minio/minio/internal/http"
 )

@@ -197,3 +199,109 @@ func testAPIFederatedCopyObjectInlineSource(objectAPI ObjectLayer, instanceType,
 		t.Fatalf("%s: destination lost copied user metadata: %v", instanceType, gr.ObjInfo.UserDefined)
 	}
 }
+
+// TestAPIFederatedCopyObjectRequestedChecksum drives the legacy etcd federation
+// branch of CopyObjectHandler and verifies that a server-side checksum is both
+// returned and persisted, matching the local CopyObject path. Before the fix
+// the federated copy forwarded the write without asking for a checksum and
+// discarded whatever the remote returned, so the response carried an empty
+// checksum even when the client requested one (#99).
+//
+// The no-algorithm case is included deliberately: a checksum-less source gains
+// the S3 default CRC-64NVME full-object checksum on the local path, so the
+// federated path must return the same. "No requested algorithm" does not mean
+// "no checksum".
+func TestAPIFederatedCopyObjectRequestedChecksum(t *testing.T) {
+	defer DetectTestLeak(t)()
+	ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
+		t:          t,
+		objAPITest: testAPIFederatedCopyObjectRequestedChecksum,
+		endpoints:  []string{"CopyObject", "PutObject", "HeadObject", "GetObject"},
+	})
+}
+
+func testAPIFederatedCopyObjectRequestedChecksum(objectAPI ObjectLayer, instanceType, bucketName string,
+	apiRouter http.Handler, credentials auth.Credentials, t *testing.T,
+) {
+	data := []byte("federated copy checksum body")
+	srcObject := "federation/checksum-source"
+	putCopyChecksumSource(t, apiRouter, credentials, bucketName, srcObject, data, nil)
+
+	remoteBucket, _, cleanup := setupCopyObjectFederation(t, objectAPI, apiRouter, instanceType, bucketName)
+	defer cleanup()
+
+	cases := []struct {
+		name     string
+		typ      hash.ChecksumType
+		explicit bool
+	}{
+		{name: "CRC32", typ: hash.ChecksumCRC32, explicit: true},
+		{name: "CRC32C", typ: hash.ChecksumCRC32C, explicit: true},
+		{name: "SHA256", typ: hash.ChecksumSHA256, explicit: true},
+		{name: "CRC64NVME", typ: hash.ChecksumCRC64NVME, explicit: true},
+		// Default: no requested algorithm still yields the S3 CRC-64NVME.
+		{name: "default", typ: hash.ChecksumCRC64NVME},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			var headers map[string]string
+			if tc.explicit {
+				headers = map[string]string{xhttp.AmzChecksumAlgo: tc.typ.String()}
+			}
+			dstObject := "federation/checksum-destination-" + tc.name
+			rec := federatedCopyRequest(t, apiRouter, credentials, bucketName, srcObject, remoteBucket, dstObject, headers)
+			if rec.Code != http.StatusOK {
+				t.Fatalf("%s: federated CopyObject failed: %d %s", instanceType, rec.Code, rec.Body.String())
+			}
+			// The CopyObjectResult must carry the checksum of the copied bytes.
+			assertCopyChecksumResponse(t, rec, tc.typ, data)
+			// The remote must have persisted that same checksum.
+			assertCopyChecksum(t, objectAPI, remoteBucket, dstObject, tc.typ, data, false, nil)
+		})
+	}
+}
+
+// TestAPIFederatedCopyObjectChecksumIsBoundToWrite guards the checksum
+// representation: a federated copy that requests one algorithm must return only
+// that algorithm, and a copy of a checksum-less source without a requested
+// algorithm must never fabricate one other than the S3 default.
+func TestAPIFederatedCopyObjectChecksumIsBoundToWrite(t *testing.T) {
+	defer DetectTestLeak(t)()
+	ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
+		t:          t,
+		objAPITest: testAPIFederatedCopyObjectChecksumIsBoundToWrite,
+		endpoints:  []string{"CopyObject", "PutObject", "HeadObject", "GetObject"},
+	})
+}
+
+func testAPIFederatedCopyObjectChecksumIsBoundToWrite(objectAPI ObjectLayer, instanceType, bucketName string,
+	apiRouter http.Handler, credentials auth.Credentials, t *testing.T,
+) {
+	data := []byte("federated copy single checksum body")
+	srcObject := "federation/single-checksum-source"
+	putCopyChecksumSource(t, apiRouter, credentials, bucketName, srcObject, data, nil)
+
+	remoteBucket, _, cleanup := setupCopyObjectFederation(t, objectAPI, apiRouter, instanceType, bucketName)
+	defer cleanup()
+
+	dstObject := "federation/single-checksum-destination"
+	rec := federatedCopyRequest(t, apiRouter, credentials, bucketName, srcObject, remoteBucket, dstObject,
+		map[string]string{xhttp.AmzChecksumAlgo: hash.ChecksumCRC32.String()})
+	if rec.Code != http.StatusOK {
+		t.Fatalf("%s: federated CopyObject failed: %d %s", instanceType, rec.Code, rec.Body.String())
+	}
+
+	var response CopyObjectResponse
+	if err := xml.Unmarshal(rec.Body.Bytes(), &response); err != nil {
+		t.Fatalf("%s: unable to decode CopyObjectResult: %v", instanceType, err)
+	}
+	if response.ChecksumCRC32 == "" {
+		t.Fatalf("%s: requested CRC32 checksum missing from response: %s", instanceType, rec.Body.String())
+	}
+	// Only the requested algorithm may be present.
+	if response.ChecksumCRC32C != "" || response.ChecksumSHA1 != "" ||
+		response.ChecksumSHA256 != "" || response.ChecksumCRC64NVME != "" {
+		t.Fatalf("%s: response carried checksums beyond the requested CRC32: %s", instanceType, rec.Body.String())
+	}
+}
@@ -1217,6 +1217,43 @@ var getRemoteInstanceClient = func(r *http.Request, host string) (*miniogo.Core,
 	return core, nil
 }

+// federatedChecksumType maps a requested server-side checksum type to the
+// minio-go checksum the federation proxy asks the remote deployment to compute
+// on the forwarded PutObject. Returns ChecksumNone for an unset/unknown type.
+func federatedChecksumType(t hash.ChecksumType) miniogo.ChecksumType {
+	switch t.Base() {
+	case hash.ChecksumCRC32:
+		return miniogo.ChecksumCRC32
+	case hash.ChecksumCRC32C:
+		return miniogo.ChecksumCRC32C
+	case hash.ChecksumSHA1:
+		return miniogo.ChecksumSHA1
+	case hash.ChecksumSHA256:
+		return miniogo.ChecksumSHA256
+	case hash.ChecksumCRC64NVME:
+		return miniogo.ChecksumCRC64NVME
+	}
+	return miniogo.ChecksumNone
+}
+
+// federatedChecksumValue returns the base64 checksum the remote deployment
+// reported for the requested type on the forwarded PutObject.
+func federatedChecksumValue(t hash.ChecksumType, info miniogo.UploadInfo) string {
+	switch t.Base() {
+	case hash.ChecksumCRC32:
+		return info.ChecksumCRC32
+	case hash.ChecksumCRC32C:
+		return info.ChecksumCRC32C
+	case hash.ChecksumSHA1:
+		return info.ChecksumSHA1
+	case hash.ChecksumSHA256:
+		return info.ChecksumSHA256
+	case hash.ChecksumCRC64NVME:
+		return info.ChecksumCRC64NVME
+	}
+	return ""
+}
+
 // Check if the destination bucket is on a remote site, this code only gets executed
 // when federation is enabled, ie when globalDNSConfig is non 'nil'.
 //
@@ -1901,6 +1938,17 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
 			ServerSideEncryption: dstOpts.ServerSideEncryption,
 			UserTags:             tag.ToMap(),
 		}
+		// When a server-side checksum was requested (explicitly, inherited from
+		// the source, or the S3 default for a checksum-less object), the local
+		// path computes, persists and returns it; the federated path must do the
+		// same. Ask the remote to compute and persist that checksum by streaming
+		// it as a trailing checksum, so the forwarded write's response carries
+		// the value back to us. Without this the federated copy silently returns
+		// an empty checksum (#99).
+		wantChecksumType := dstOpts.WantServerSideChecksumType
+		if wantChecksumType.IsSet() {
+			opts.Checksum = federatedChecksumType(wantChecksumType)
+		}
 		remoteObjInfo, rerr := core.PutObject(ctx, dstBucket, dstObject, srcInfo.Reader,
 			srcInfo.Size, "", "", opts)
 		if rerr != nil {
@@ -1910,6 +1958,16 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
 		objInfo.UserDefined = cloneMSS(opts.UserMetadata)
 		objInfo.ETag = remoteObjInfo.ETag
 		objInfo.ModTime = remoteObjInfo.LastModified
+		// Bind the checksum the remote computed for this exact write into the
+		// response, matching the local CopyObject path and the federated
+		// UploadPartCopy repair in #72. A malformed or absent remote value
+		// leaves objInfo.Checksum unset, so an ordinary copy returns none.
+		if wantChecksumType.IsSet() {
+			if cs := hash.NewChecksumWithType(wantChecksumType,
+				federatedChecksumValue(wantChecksumType, remoteObjInfo)); cs != nil {
+				objInfo.Checksum = cs.AppendTo(nil, nil)
+			}
+		}
 	} else {
 		os = newObjSweeper(dstBucket, dstObject).WithVersioning(dstOpts.Versioned, dstOpts.VersionSuspended)
 		// Get appropriate object info to identify the remote object to delete