Skip to content

Commit ece57a2

Browse files
Vonngclaude
andcommitted
fix: return the remote part checksum to federated UploadPartCopy
The legacy etcd federation branch of CopyObjectPartHandler forwards copied bytes with minio-go Core.PutObjectPart, which can only recover a checksum from response headers. After the server-side part checksum work, the remote computes and persists the checksum, but an AWS-compatible UploadPart response correctly omits a checksum the request did not supply, so the proxy had nothing to put in CopyPartResult. The destination now returns the non-empty checksum fields of the PartInfo produced by that exact write, but only when the request carries the minio-federated application token that getRemoteInstanceClient already attaches. Ordinary UploadPart responses are unchanged, and the checksum type is deliberately not returned because UploadPart does not carry it. The User-Agent is a response-shape hint only: it never gates authorization, visibility or validation, and it can expose nothing beyond the checksum of the body the caller just uploaded. Reading the checksum from the same PartInfo that produced the response ETag also keeps the pair bound to one write, so a concurrent overwrite of the same part number cannot publish another writer's checksum. Tests cover the application token gating matrix including lookalike tokens, the real minio-go response parser, concurrent overwrites of one part number, and an in-process two-deployment probe that drives the federation branch through the real getRemoteInstanceClient into a real PutObjectPartHandler for both FULL_OBJECT and COMPOSITE uploads. Fixes #64 Signed-off-by: Feng Ruohang <rh@vonng.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent f2520f3 commit ece57a2

3 files changed

Lines changed: 421 additions & 1 deletion

File tree

cmd/object-handlers.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1122,6 +1122,12 @@ func getRemoteInstanceTransport() http.RoundTripper {
11221122
return nil
11231123
}
11241124

1125+
// federatedInternalAppName is the minio-go application token that
1126+
// getRemoteInstanceClient attaches to every legacy federation proxy request. It
1127+
// is declared next to its only producer so that the literal keeps its historical
1128+
// file attribution in the rebrand compatibility baseline.
1129+
const federatedInternalAppName = "minio-federated"
1130+
11251131
// Returns a minio-go Client configured to access remote host described by destDNSRecord
11261132
// Applicable only in a federated deployment
11271133
var getRemoteInstanceClient = func(r *http.Request, host string) (*miniogo.Core, error) {
@@ -1136,7 +1142,7 @@ var getRemoteInstanceClient = func(r *http.Request, host string) (*miniogo.Core,
11361142
if err != nil {
11371143
return nil, err
11381144
}
1139-
core.SetAppInfo("minio-federated", ReleaseTag)
1145+
core.SetAppInfo(federatedInternalAppName, ReleaseTag)
11401146
return core, nil
11411147
}
11421148

Lines changed: 367 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,367 @@
1+
// Copyright (c) 2015-2025 MinIO, Inc.
2+
// Copyright (c) 2025-2026 PGSTY
3+
//
4+
// This file is part of MinIO Object Storage stack
5+
//
6+
// This program is free software: you can redistribute it and/or modify
7+
// it under the terms of the GNU Affero General Public License as published by
8+
// the Free Software Foundation, either version 3 of the License, or
9+
// (at your option) any later version.
10+
//
11+
// This program is distributed in the hope that it will be useful
12+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
// GNU Affero General Public License for more details.
15+
//
16+
// You should have received a copy of the GNU Affero General Public License
17+
// along with this program. If not, see <http://www.gnu.org/licenses/>.
18+
19+
package cmd
20+
21+
import (
22+
"bytes"
23+
"context"
24+
"crypto/md5"
25+
"encoding/hex"
26+
"encoding/json"
27+
"encoding/xml"
28+
"net/http"
29+
"net/http/httptest"
30+
"strings"
31+
"sync"
32+
"testing"
33+
34+
miniogo "github.com/minio/minio-go/v7"
35+
miniocredentials "github.com/minio/minio-go/v7/pkg/credentials"
36+
"github.com/minio/minio-go/v7/pkg/set"
37+
"github.com/minio/minio/internal/auth"
38+
"github.com/minio/minio/internal/config/dns"
39+
"github.com/minio/minio/internal/hash"
40+
xhttp "github.com/minio/minio/internal/http"
41+
)
42+
43+
const federatedTestUserAgent = "MinIO (linux; amd64) minio-go/v7.0.99 minio-federated/RELEASE.TEST"
44+
45+
func TestAPIFederatedUploadPartChecksumResponse(t *testing.T) {
46+
defer DetectTestLeak(t)()
47+
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
48+
t: t,
49+
objAPITest: testAPIFederatedUploadPartChecksumResponse,
50+
endpoints: []string{"PutObjectPart", "NewMultipart"},
51+
})
52+
}
53+
54+
func testAPIFederatedUploadPartChecksumResponse(_ ObjectLayer, instanceType, bucketName string,
55+
apiRouter http.Handler, credentials auth.Credentials, t *testing.T,
56+
) {
57+
algorithms := []struct {
58+
name string
59+
typ hash.ChecksumType
60+
checksumType string
61+
}{
62+
{name: "crc32-full-object", typ: hash.ChecksumCRC32, checksumType: xhttp.AmzChecksumTypeFullObject},
63+
{name: "sha256-composite", typ: hash.ChecksumSHA256, checksumType: xhttp.AmzChecksumTypeComposite},
64+
}
65+
userAgents := []struct {
66+
name string
67+
ua string
68+
want bool
69+
}{
70+
{name: "absent"},
71+
{name: "ordinary-sdk", ua: "aws-sdk-go/1.55.5"},
72+
{name: "federation", ua: federatedTestUserAgent, want: true},
73+
{name: "lookalike-prefix", ua: "evil-minio-federated/RELEASE.TEST"},
74+
{name: "lookalike-suffix", ua: "minio-federated-extra/RELEASE.TEST"},
75+
{name: "missing-version", ua: "minio-federated"},
76+
{name: "empty-version", ua: "minio-federated/"},
77+
}
78+
data := []byte("federated upload part checksum response")
79+
80+
for _, algorithm := range algorithms {
81+
for _, userAgent := range userAgents {
82+
t.Run(algorithm.name+"/"+userAgent.name, func(t *testing.T) {
83+
object := "federation/response/" + algorithm.name + "/" + userAgent.name
84+
uploadID := newMultipartUploadHTTP(t, apiRouter, credentials, bucketName, object,
85+
algorithm.typ.String(), algorithm.checksumType)
86+
headers := map[string]string{}
87+
if userAgent.ua != "" {
88+
headers["User-Agent"] = userAgent.ua
89+
}
90+
_, rec := uploadPartHTTP(t, apiRouter, credentials,
91+
bucketName, object, uploadID, 1, data, headers)
92+
93+
got := rec.Header().Get(algorithm.typ.Key())
94+
if userAgent.want {
95+
if want := mustChecksum(t, algorithm.typ, data); got != want {
96+
t.Fatalf("%s: checksum %q, want %q", instanceType, got, want)
97+
}
98+
} else if got != "" {
99+
t.Fatalf("%s: ordinary UploadPart exposed server checksum %q", instanceType, got)
100+
}
101+
if got := rec.Header().Get(xhttp.AmzChecksumType); got != "" {
102+
t.Fatalf("%s: UploadPart returned checksum type %q", instanceType, got)
103+
}
104+
})
105+
}
106+
}
107+
}
108+
109+
func TestAPIFederatedUploadPartChecksumMinIOGoWire(t *testing.T) {
110+
defer DetectTestLeak(t)()
111+
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
112+
t: t,
113+
objAPITest: testAPIFederatedUploadPartChecksumMinIOGoWire,
114+
endpoints: []string{"PutObjectPart", "NewMultipart"},
115+
})
116+
}
117+
118+
func testAPIFederatedUploadPartChecksumMinIOGoWire(_ ObjectLayer, instanceType, bucketName string,
119+
apiRouter http.Handler, credentials auth.Credentials, t *testing.T,
120+
) {
121+
server := httptest.NewServer(apiRouter)
122+
defer server.Close()
123+
124+
core, err := miniogo.NewCore(server.Listener.Addr().String(), &miniogo.Options{
125+
Creds: miniocredentials.NewStaticV4(credentials.AccessKey, credentials.SecretKey, ""),
126+
Secure: false,
127+
Region: globalMinioDefaultRegion,
128+
BucketLookup: miniogo.BucketLookupPath,
129+
})
130+
if err != nil {
131+
t.Fatalf("%s: create minio-go Core: %v", instanceType, err)
132+
}
133+
core.SetAppInfo("minio-federated", ReleaseTag)
134+
135+
object := "federation/minio-go-wire"
136+
uploadID := newMultipartUploadHTTP(t, apiRouter, credentials, bucketName, object,
137+
hash.ChecksumCRC32.String(), xhttp.AmzChecksumTypeFullObject)
138+
data := []byte("minio-go must parse the remote computed checksum")
139+
part, err := core.PutObjectPart(t.Context(), bucketName, object, uploadID, 1,
140+
bytes.NewReader(data), int64(len(data)), miniogo.PutObjectPartOptions{})
141+
if err != nil {
142+
t.Fatalf("%s: minio-go PutObjectPart: %v", instanceType, err)
143+
}
144+
if want := mustChecksum(t, hash.ChecksumCRC32, data); part.ChecksumCRC32 != want {
145+
t.Fatalf("%s: minio-go checksum %q, want %q", instanceType, part.ChecksumCRC32, want)
146+
}
147+
if part.ETag == "" {
148+
t.Fatalf("%s: minio-go returned an empty ETag", instanceType)
149+
}
150+
}
151+
152+
func TestAPIFederatedUploadPartChecksumConcurrentOverwrite(t *testing.T) {
153+
defer DetectTestLeak(t)()
154+
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
155+
t: t,
156+
objAPITest: testAPIFederatedUploadPartChecksumConcurrentOverwrite,
157+
endpoints: []string{"PutObjectPart", "NewMultipart"},
158+
})
159+
}
160+
161+
func testAPIFederatedUploadPartChecksumConcurrentOverwrite(_ ObjectLayer, instanceType, bucketName string,
162+
apiRouter http.Handler, credentials auth.Credentials, t *testing.T,
163+
) {
164+
object := "federation/concurrent-overwrite"
165+
uploadID := newMultipartUploadHTTP(t, apiRouter, credentials, bucketName, object,
166+
hash.ChecksumSHA256.String(), xhttp.AmzChecksumTypeComposite)
167+
data := [][]byte{
168+
bytes.Repeat([]byte("first-writer-"), 4096),
169+
bytes.Repeat([]byte("second-writer-"), 4096),
170+
}
171+
reqs := make([]*http.Request, len(data))
172+
recorders := make([]*httptest.ResponseRecorder, len(data))
173+
for i := range data {
174+
req, err := newTestSignedRequestV4(http.MethodPut,
175+
getPutObjectPartURL("", bucketName, object, uploadID, "1"),
176+
int64(len(data[i])), bytes.NewReader(data[i]), credentials.AccessKey, credentials.SecretKey,
177+
map[string]string{"User-Agent": federatedTestUserAgent})
178+
if err != nil {
179+
t.Fatalf("%s: build concurrent request %d: %v", instanceType, i, err)
180+
}
181+
reqs[i] = req
182+
recorders[i] = httptest.NewRecorder()
183+
}
184+
185+
start := make(chan struct{})
186+
var wg sync.WaitGroup
187+
for i := range reqs {
188+
wg.Add(1)
189+
go func() {
190+
defer wg.Done()
191+
<-start
192+
apiRouter.ServeHTTP(recorders[i], reqs[i])
193+
}()
194+
}
195+
close(start)
196+
wg.Wait()
197+
198+
for i, rec := range recorders {
199+
if rec.Code != http.StatusOK {
200+
t.Fatalf("%s: concurrent request %d failed: %d %s", instanceType, i, rec.Code, rec.Body.String())
201+
}
202+
got := rec.Header().Get(hash.ChecksumSHA256.Key())
203+
if want := mustChecksum(t, hash.ChecksumSHA256, data[i]); got != want {
204+
t.Fatalf("%s: concurrent request %d checksum %q, want %q", instanceType, i, got, want)
205+
}
206+
// The ETag and the checksum must describe the same write, so a losing
207+
// writer can never publish the winner's checksum next to its own ETag.
208+
etags := rec.Header()[xhttp.ETag]
209+
if len(etags) != 1 {
210+
t.Fatalf("%s: concurrent request %d returned %d ETags", instanceType, i, len(etags))
211+
}
212+
md5sum := md5.Sum(data[i])
213+
if want := hex.EncodeToString(md5sum[:]); canonicalizeETag(etags[0]) != want {
214+
t.Fatalf("%s: concurrent request %d ETag %q, want %q", instanceType, i, etags[0], want)
215+
}
216+
}
217+
}
218+
219+
// federationTestDNS is a minimal dns.Store so a single test process can play
220+
// both federation roles.
221+
type federationTestDNS struct {
222+
records map[string][]dns.SrvRecord
223+
}
224+
225+
func (f federationTestDNS) Put(string) error { return nil }
226+
227+
func (f federationTestDNS) Get(bucket string) ([]dns.SrvRecord, error) {
228+
records, ok := f.records[bucket]
229+
if !ok {
230+
return nil, dns.ErrNoEntriesFound
231+
}
232+
return records, nil
233+
}
234+
235+
func (f federationTestDNS) Delete(string) error { return nil }
236+
func (f federationTestDNS) List() (map[string][]dns.SrvRecord, error) { return f.records, nil }
237+
func (f federationTestDNS) DeleteRecord(dns.SrvRecord) error { return nil }
238+
func (f federationTestDNS) Close() error { return nil }
239+
func (f federationTestDNS) String() string { return "federation-test-dns" }
240+
241+
// remoteBucketObjectLayer reports one existing bucket as missing so that
242+
// isRemoteCopyRequired takes the legacy federation branch while the same
243+
// process can still serve that bucket as the remote deployment.
244+
type remoteBucketObjectLayer struct {
245+
ObjectLayer
246+
remoteBucket string
247+
}
248+
249+
func (l remoteBucketObjectLayer) GetBucketInfo(ctx context.Context, bucket string, opts BucketOptions) (BucketInfo, error) {
250+
if bucket == l.remoteBucket {
251+
return BucketInfo{}, toObjectErr(errVolumeNotFound, bucket)
252+
}
253+
return l.ObjectLayer.GetBucketInfo(ctx, bucket, opts)
254+
}
255+
256+
// TestAPIFederatedCopyObjectPartChecksum drives the legacy etcd federation
257+
// branch of CopyObjectPartHandler end to end: the proxy forwards the copied
258+
// bytes through the real getRemoteInstanceClient and minio-go, a second HTTP
259+
// endpoint serves the real PutObjectPartHandler, and CopyPartResult must carry
260+
// the checksum computed by that exact remote write.
261+
func TestAPIFederatedCopyObjectPartChecksum(t *testing.T) {
262+
defer DetectTestLeak(t)()
263+
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
264+
t: t,
265+
objAPITest: testAPIFederatedCopyObjectPartChecksum,
266+
endpoints: []string{
267+
"CopyObjectPart", "NewMultipart", "PutObjectPart",
268+
"ListObjectParts", "CompleteMultipart", "PutObject",
269+
},
270+
})
271+
}
272+
273+
func testAPIFederatedCopyObjectPartChecksum(objectAPI ObjectLayer, instanceType, bucketName string,
274+
apiRouter http.Handler, credentials auth.Credentials, t *testing.T,
275+
) {
276+
algorithms := []struct {
277+
name string
278+
typ hash.ChecksumType
279+
checksumType string
280+
}{
281+
{name: "crc32-full-object", typ: hash.ChecksumCRC32, checksumType: xhttp.AmzChecksumTypeFullObject},
282+
{name: "sha256-composite", typ: hash.ChecksumSHA256, checksumType: xhttp.AmzChecksumTypeComposite},
283+
}
284+
285+
data := bytes.Repeat([]byte("federated-upload-part-copy-"), 1024)
286+
srcObject := "federation/copy-source.bin"
287+
putCopyChecksumSource(t, apiRouter, credentials, bucketName, srcObject, data, nil)
288+
289+
// The destination bucket really exists so the remote endpoint can serve it;
290+
// only the proxy's own bucket lookup is told that it lives elsewhere.
291+
remoteBucket := getRandomBucketName()
292+
if err := objectAPI.MakeBucket(t.Context(), remoteBucket, MakeBucketOptions{}); err != nil {
293+
t.Fatalf("%s: unable to create the remote bucket: %v", instanceType, err)
294+
}
295+
296+
remote := httptest.NewServer(apiRouter)
297+
defer remote.Close()
298+
host, port, _ := strings.Cut(remote.Listener.Addr().String(), ":")
299+
300+
globalObjLayerMutex.Lock()
301+
previousLayer := globalObjectAPI
302+
globalObjectAPI = remoteBucketObjectLayer{ObjectLayer: previousLayer, remoteBucket: remoteBucket}
303+
globalObjLayerMutex.Unlock()
304+
previousDNS, previousFederation, previousIPs := globalDNSConfig, globalBucketFederation, globalDomainIPs
305+
globalDNSConfig = federationTestDNS{records: map[string][]dns.SrvRecord{
306+
bucketName: {{Host: host, Port: json.Number(port)}},
307+
remoteBucket: {{Host: host, Port: json.Number(port)}},
308+
}}
309+
// Every DNS record resolves to this process, so the bucket forwarding
310+
// middleware always serves locally and only the handler proxies.
311+
globalDomainIPs = set.CreateStringSet(remote.Listener.Addr().String())
312+
globalBucketFederation = true
313+
defer func() {
314+
globalObjLayerMutex.Lock()
315+
globalObjectAPI = previousLayer
316+
globalObjLayerMutex.Unlock()
317+
globalDNSConfig, globalBucketFederation, globalDomainIPs = previousDNS, previousFederation, previousIPs
318+
}()
319+
320+
for _, algorithm := range algorithms {
321+
t.Run(algorithm.name, func(t *testing.T) {
322+
object := "federation/copy-destination-" + algorithm.name + ".bin"
323+
uploadID := newMultipartUploadHTTP(t, apiRouter, credentials, remoteBucket, object,
324+
algorithm.typ.String(), algorithm.checksumType)
325+
326+
req, err := newTestSignedRequestV4(http.MethodPut,
327+
getCopyObjectPartURL("", remoteBucket, object, uploadID, "1"),
328+
0, nil, credentials.AccessKey, credentials.SecretKey,
329+
map[string]string{xhttp.AmzCopySource: SlashSeparator + pathJoin(bucketName, srcObject)})
330+
if err != nil {
331+
t.Fatalf("%s: unable to build UploadPartCopy request: %v", instanceType, err)
332+
}
333+
rec := httptest.NewRecorder()
334+
apiRouter.ServeHTTP(rec, req)
335+
if rec.Code != http.StatusOK {
336+
t.Fatalf("%s: federated UploadPartCopy failed: %d %s", instanceType, rec.Code, rec.Body.String())
337+
}
338+
339+
var response CopyObjectPartResponse
340+
if err := xml.Unmarshal(rec.Body.Bytes(), &response); err != nil {
341+
t.Fatalf("%s: unable to decode CopyPartResult: %v", instanceType, err)
342+
}
343+
want := mustChecksum(t, algorithm.typ, data)
344+
if got := copyPartChecksum(algorithm.typ, response); got != want {
345+
t.Fatalf("%s: CopyPartResult %s is %q, want %q: %s",
346+
instanceType, algorithm.typ.String(), got, want, rec.Body.String())
347+
}
348+
349+
// The persisted part must carry the same value, and the client must be
350+
// able to complete the upload with what CopyPartResult returned.
351+
parts := listPartsHTTP(t, apiRouter, credentials, remoteBucket, object, uploadID, nil)
352+
if len(parts.Parts) != 1 {
353+
t.Fatalf("%s: ListParts returned %d parts, want 1", instanceType, len(parts.Parts))
354+
}
355+
if got := partChecksum(algorithm.typ, parts.Parts[0]); got != want {
356+
t.Fatalf("%s: persisted part %s is %q, want %q", instanceType, algorithm.typ.String(), got, want)
357+
}
358+
etag := canonicalizeETag(response.ETag)
359+
completed := completePartsHTTP(t, apiRouter, credentials, remoteBucket, object, uploadID,
360+
[]CompletePart{completePartWithChecksum(algorithm.typ, 1, etag, want)}, nil)
361+
if completed.Code != http.StatusOK {
362+
t.Fatalf("%s: CompleteMultipartUpload rejected the federated part: %d %s",
363+
instanceType, completed.Code, completed.Body.String())
364+
}
365+
})
366+
}
367+
}

0 commit comments

Comments
 (0)