diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go index 5f6268cb6..5142c8e32 100644 --- a/cmd/bucket-replication.go +++ b/cmd/bucket-replication.go @@ -931,11 +931,17 @@ func equals(k1 string, keys ...string) bool { return false } +// nullVersionExcludedFromResync reports the exclusion at the head of getReplicationAction, kept +// verbatim from upstream: an existing object resync leaves a null version alone when the source +// modification time is later than the one the target reports, without comparing anything else. +func nullVersionExcludedFromResync(oi1 ObjectInfo, oi2 minio.ObjectInfo, opType replication.Type) bool { + return opType == replication.ExistingObjectReplicationType && + oi1.ModTime.Unix() > oi2.LastModified.Unix() && oi1.VersionID == nullVersionID +} + // returns replicationAction by comparing metadata between source and target func getReplicationAction(oi1 ObjectInfo, oi2 minio.ObjectInfo, opType replication.Type) replicationAction { - // Avoid resyncing null versions created prior to enabling replication if target has a newer copy - if opType == replication.ExistingObjectReplicationType && - oi1.ModTime.Unix() > oi2.LastModified.Unix() && oi1.VersionID == nullVersionID { + if nullVersionExcludedFromResync(oi1, oi2, opType) { return replicateNone } sz, _ := oi1.GetActualSize() @@ -986,9 +992,21 @@ func getReplicationAction(oi1 ObjectInfo, oi2 minio.ObjectInfo, opType replicati "X-Amz-Meta-", } + // An empty object lock mode or retain-until-date records a removed retention, but + // it is omitted from GET/HEAD response headers: setObjectHeaders() skips both keys + // when the value is empty, and FilterObjectLockMetadata() drops them when the mode + // is not valid. The target can therefore never report them, so treat empty and + // absent as equal rather than as a permanent difference. + emptyLockValue := func(k, v string) bool { + return v == "" && equals(k, xhttp.AmzObjectLockMode, xhttp.AmzObjectLockRetainUntilDate) + } + // compare metadata on both maps to see if meta is identical compareMeta1 := make(map[string]string) for k, v := range oi1.UserDefined { + if emptyLockValue(k, v) { + continue + } var found bool for _, prefix := range compareKeys { if !stringsHasPrefixFold(k, prefix) { @@ -1004,6 +1022,10 @@ func getReplicationAction(oi1 ObjectInfo, oi2 minio.ObjectInfo, opType replicati compareMeta2 := make(map[string]string) for k, v := range oi2.Metadata { + val := strings.Join(v, ",") + if emptyLockValue(k, val) { + continue + } var found bool for _, prefix := range compareKeys { if !stringsHasPrefixFold(k, prefix) { @@ -1013,7 +1035,7 @@ func getReplicationAction(oi1 ObjectInfo, oi2 minio.ObjectInfo, opType replicati break } if found { - compareMeta2[strings.ToLower(k)] = strings.Join(v, ",") + compareMeta2[strings.ToLower(k)] = val } } @@ -1024,6 +1046,65 @@ func getReplicationAction(oi1 ObjectInfo, oi2 minio.ObjectInfo, opType replicati return replicateNone } +// objectRetentionGetter is the part of the replication target client used to confirm whether a +// destination version still holds Object Lock retention. +type objectRetentionGetter interface { + GetObjectRetention(ctx context.Context, bucketName, objectName, versionID string) (*minio.RetentionMode, *time.Time, error) +} + +// retentionRemovedAtSource reports whether oi carries the shape a removed retention leaves behind, +// an object lock key present with an empty value (cmd/object-handlers.go:3211-3215). +func retentionRemovedAtSource(oi ObjectInfo) bool { + lkMap := caseInsensitiveMap(oi.UserDefined) + for _, k := range []string{xhttp.AmzObjectLockMode, xhttp.AmzObjectLockRetainUntilDate} { + if v, ok := lkMap.Lookup(k); ok && v == "" { + return true + } + } + return false +} + +// targetRetentionConfirmedAbsent reports whether the destination version is known to hold no +// retention. A HEAD response omits retention both when the version has none and when the +// replication credential lacks s3:GetObjectRetention (cmd/object-handlers.go:942-946), so the +// comparison in getReplicationAction on its own cannot tell a removal that is already in sync from +// one the destination still holds. Only NoSuchObjectLockConfiguration, the answer for a version +// that carries no retention, and a response naming no retention mode count as absent. Everything +// else is uncertainty and is treated as still present, so that the removal is resent exactly as it +// is today: a denied or unreachable destination, a mode the SDK returned without recognizing since +// it does not validate it, and InvalidRequest, which names a bucket with no Object Lock +// configuration but is also what a destination answers when its own read of that configuration +// fails (cmd/bucket-object-lock.go:39-50 returns an error with a zero Retention, discarded at +// cmd/object-handlers.go:3275). +func targetRetentionConfirmedAbsent(ctx context.Context, tgt objectRetentionGetter, bucket, object, versionID string) bool { + mode, _, err := tgt.GetObjectRetention(ctx, bucket, object, versionID) + if err != nil { + return minio.ToErrorResponse(err).Code == "NoSuchObjectLockConfiguration" + } + // An absent or empty mode is no retention. A non-empty mode is retention, whether or not this + // SDK recognizes it. + return mode == nil || *mode == "" +} + +// replicationActionForTarget returns the action for a source version against a destination that +// answered HEAD. It is getReplicationAction plus the confirmation that a removed retention which +// compares as in sync really is: see targetRetentionConfirmedAbsent. +func replicationActionForTarget(ctx context.Context, oi1 ObjectInfo, oi2 minio.ObjectInfo, opType replication.Type, tgt objectRetentionGetter, bucket, object string) replicationAction { + rAction := getReplicationAction(oi1, oi2, opType) + if rAction != replicateNone || !retentionRemovedAtSource(oi1) { + return rAction + } + // A null version the resync deliberately leaves alone is not a comparison result, so it is + // not the confirmation's to reopen. + if nullVersionExcludedFromResync(oi1, oi2, opType) { + return rAction + } + if targetRetentionConfirmedAbsent(ctx, tgt, bucket, object, oi1.VersionID) { + return rAction + } + return replicateMetadata +} + // replicateObject replicates the specified version of the object to destination bucket // The source object is then updated to reflect the replication status. func replicateObject(ctx context.Context, ri ReplicateObjectInfo, objectAPI ObjectLayer) { @@ -1467,7 +1548,7 @@ func (ri ReplicateObjectInfo) replicateAll(ctx context.Context, objectAPI Object sOpts.Set(xhttp.AmzTagDirective, "ACCESS") oi, cerr := tgt.StatObject(ctx, tgt.Bucket, object, sOpts) if cerr == nil { - rAction = getReplicationAction(objInfo, oi, ri.OpType) + rAction = replicationActionForTarget(ctx, objInfo, oi, ri.OpType, tgt, tgt.Bucket, object) rinfo.ReplicationStatus = replication.Completed if rAction == replicateNone { if ri.OpType == replication.ExistingObjectReplicationType && diff --git a/cmd/bucket-replication_test.go b/cmd/bucket-replication_test.go index db57f5eb0..58bd247fc 100644 --- a/cmd/bucket-replication_test.go +++ b/cmd/bucket-replication_test.go @@ -18,13 +18,19 @@ package cmd import ( + "context" + "errors" "fmt" "net/http" + "net/http/httptest" "path" + "strings" "testing" "time" "github.com/minio/madmin-go/v3" + "github.com/minio/minio-go/v7" + objectlock "github.com/minio/minio/internal/bucket/object/lock" "github.com/minio/minio/internal/bucket/replication" xhttp "github.com/minio/minio/internal/http" ) @@ -307,3 +313,318 @@ func TestReplicationValidationObjectUsesRulePrefix(t *testing.T) { }) } } + +// newMatchingReplicationPair returns a source/target pair that getReplicationAction must +// classify as replicateNone: same ETag, version id, size, modification time and content +// type. Any action other than replicateNone is therefore attributable to the object lock +// entries a caller adds on top. +func newMatchingReplicationPair() (ObjectInfo, minio.ObjectInfo) { + mtime := time.Date(2026, 9, 5, 10, 0, 0, 0, time.UTC) + size := int64(7) + src := ObjectInfo{ + Bucket: "bucket", + Name: "object", + ETag: "d41d8cd98f00b204e9800998ecf8427e", + VersionID: "b0ff1d6e-0000-4000-8000-000000000001", + Size: size, + ActualSize: &size, + ModTime: mtime, + ContentType: "application/octet-stream", + UserDefined: map[string]string{"content-type": "application/octet-stream"}, + } + tgt := minio.ObjectInfo{ + ETag: src.ETag, + VersionID: src.VersionID, + Size: size, + LastModified: mtime, + ContentType: src.ContentType, + Metadata: http.Header{}, + } + return src, tgt +} + +// TestGetReplicationActionEmptyObjectLockValues covers the comparison of object lock entries +// whose value is empty. Removing retention from a version stores the mode and retain-until-date +// keys with empty values, while the target's HEAD response omits them entirely, so the two must +// compare equal or the version can never be reported as in sync. Cases 3 and 4 are synthetic +// comparison inputs, since a SILO target cannot return empty lock headers; cases 7 and 8 guard +// against over-normalizing. +func TestGetReplicationActionEmptyObjectLockValues(t *testing.T) { + var ( + modeKey = strings.ToLower(xhttp.AmzObjectLockMode) + dateKey = strings.ToLower(xhttp.AmzObjectLockRetainUntilDate) + until = "2026-10-05T10:00:00.000Z" + ) + emptyRetention := map[string]string{modeKey: "", dateKey: ""} + realRetention := map[string]string{modeKey: "GOVERNANCE", dateKey: until} + + tests := []struct { + name string + srcMeta map[string]string + tgtHdr map[string]string + want replicationAction + }{ + {"1-both-clean-never-had-retention", nil, nil, replicateNone}, + {"2-source-present-empty-target-absent", emptyRetention, nil, replicateNone}, + {"3-source-absent-target-present-empty", nil, emptyRetention, replicateNone}, + {"4-both-present-empty", emptyRetention, emptyRetention, replicateNone}, + {"5-both-governance-equal", realRetention, realRetention, replicateNone}, + {"6-source-governance-target-absent", realRetention, nil, replicateMetadata}, + {"7-source-empty-target-real-retention", emptyRetention, realRetention, replicateMetadata}, + {"8-empty-user-metadata-is-not-normalized", map[string]string{"x-amz-meta-foo": ""}, nil, replicateMetadata}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + src, tgt := newMatchingReplicationPair() + for k, v := range test.srcMeta { + src.UserDefined[k] = v + } + for k, v := range test.tgtHdr { + tgt.Metadata.Set(k, v) + } + if got := getReplicationAction(src, tgt, replication.HealReplicationType); got != test.want { + t.Fatalf("getReplicationAction() = %q, want %q (source %v, target %v)", got, test.want, src.UserDefined, tgt.Metadata) + } + }) + } +} + +// TestEmptyRetentionValuesAreOmittedFromObjectResponseHeaders records why the target half of the +// comparison in getReplicationAction can never report an empty object lock entry: +// FilterObjectLockMetadata drops both keys because an empty mode is not a valid retention mode, +// and setObjectHeaders skips them when writing response headers. Neither filter reaches the +// replication wire: the empty entries are still carried by getCopyObjMetadata and sent by the +// metadata CopyObject, which is why the sender's comparison is what has to tolerate them. +// FilterObjectLockMetadata is also applied by CopyObject (cmd/object-handlers.go:1708), where it +// strips the source's lock metadata before the destination re-derives it from the request. +func TestEmptyRetentionValuesAreOmittedFromObjectResponseHeaders(t *testing.T) { + modeKey := strings.ToLower(xhttp.AmzObjectLockMode) + dateKey := strings.ToLower(xhttp.AmzObjectLockRetainUntilDate) + meta := map[string]string{ + modeKey: "", + dateKey: "", + "content-type": "application/octet-stream", + } + + filtered := objectlock.FilterObjectLockMetadata(meta, false, false) + if _, ok := filtered[modeKey]; ok { + t.Errorf("FilterObjectLockMetadata() kept the empty lock mode key: %v", filtered) + } + if _, ok := filtered[dateKey]; ok { + t.Errorf("FilterObjectLockMetadata() kept the empty retain-until-date key: %v", filtered) + } + + rec := httptest.NewRecorder() + if err := setObjectHeaders(t.Context(), rec, ObjectInfo{UserDefined: meta, ModTime: time.Now(), Size: 7}, nil, ObjectOptions{}); err != nil { + t.Fatalf("setObjectHeaders() = %v", err) + } + if v, ok := rec.Header()[http.CanonicalHeaderKey(xhttp.AmzObjectLockMode)]; ok { + t.Errorf("setObjectHeaders() emitted an empty lock mode header: %v", v) + } + if v, ok := rec.Header()[http.CanonicalHeaderKey(xhttp.AmzObjectLockRetainUntilDate)]; ok { + t.Errorf("setObjectHeaders() emitted an empty retain-until-date header: %v", v) + } +} + +// fakeRetentionGetter answers GetObjectRetention with a fixed result and counts its calls. +type fakeRetentionGetter struct { + mode *minio.RetentionMode + err error + calls int +} + +func (f *fakeRetentionGetter) GetObjectRetention(_ context.Context, _, _, _ string) (*minio.RetentionMode, *time.Time, error) { + f.calls++ + return f.mode, nil, f.err +} + +func TestRetentionRemovedAtSource(t *testing.T) { + modeKey := strings.ToLower(xhttp.AmzObjectLockMode) + dateKey := strings.ToLower(xhttp.AmzObjectLockRetainUntilDate) + tests := []struct { + name string + meta map[string]string + want bool + }{ + {"no lock keys", map[string]string{"content-type": "text/plain"}, false}, + {"empty pair", map[string]string{modeKey: "", dateKey: ""}, true}, + {"empty mode only", map[string]string{modeKey: ""}, true}, + {"empty date only", map[string]string{dateKey: ""}, true}, + {"real retention", map[string]string{modeKey: "GOVERNANCE", dateKey: "2026-10-05T10:00:00.000Z"}, false}, + {"canonical case", map[string]string{xhttp.AmzObjectLockMode: ""}, true}, + {"empty user metadata", map[string]string{"x-amz-meta-foo": ""}, false}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if got := retentionRemovedAtSource(ObjectInfo{UserDefined: test.meta}); got != test.want { + t.Fatalf("retentionRemovedAtSource() = %v, want %v", got, test.want) + } + }) + } +} + +// TestTargetRetentionConfirmedAbsent pins the rule that only an explicit answer from the +// destination clears a removed retention. A denied or unreachable destination must read as still +// holding retention, because HEAD hides a real retention from a credential without +// s3:GetObjectRetention exactly as it hides one that does not exist. +func TestTargetRetentionConfirmedAbsent(t *testing.T) { + governance := minio.Governance + var emptyMode minio.RetentionMode + unknownMode := minio.RetentionMode("ARCHIVE") + tests := []struct { + name string + mode *minio.RetentionMode + err error + want bool + }{ + {"version holds governance retention", &governance, nil, false}, + {"no retention on the version", nil, minio.ErrorResponse{Code: "NoSuchObjectLockConfiguration"}, true}, + { + // The destination also answers this when its own read of the bucket's Object Lock + // configuration fails, so it does not establish that Object Lock is disabled. + "invalid request naming a missing object lock configuration", + nil, + minio.ErrorResponse{Code: "InvalidRequest", Message: "Bucket is missing ObjectLockConfiguration"}, + false, + }, + { + "unrelated invalid request", + nil, + minio.ErrorResponse{Code: "InvalidRequest", Message: "Object is WORM protected and cannot be overwritten"}, + false, + }, + {"retention read denied", nil, minio.ErrorResponse{Code: "AccessDenied"}, false}, + {"destination unreachable", nil, errors.New("dial tcp: connection refused"), false}, + {"empty mode returned", &emptyMode, nil, true}, + {"unknown non-empty mode returned", &unknownMode, nil, false}, + {"nil mode returned", nil, nil, true}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + tgt := &fakeRetentionGetter{mode: test.mode, err: test.err} + if got := targetRetentionConfirmedAbsent(t.Context(), tgt, "bucket", "object", "v1"); got != test.want { + t.Fatalf("targetRetentionConfirmedAbsent() = %v, want %v", got, test.want) + } + if tgt.calls != 1 { + t.Fatalf("GetObjectRetention called %d times, want 1", tgt.calls) + } + }) + } +} + +// TestReplicationActionForTargetRetentionRemoval covers the decision the replication worker makes +// for a version whose retention was removed. The destination's HEAD never reports the empty keys, +// so the comparison alone reads every one of these as in sync; only the confirmation separates a +// destination that really dropped the retention from one that is hiding it. +func TestReplicationActionForTargetRetentionRemoval(t *testing.T) { + modeKey := strings.ToLower(xhttp.AmzObjectLockMode) + dateKey := strings.ToLower(xhttp.AmzObjectLockRetainUntilDate) + governance := minio.Governance + + tests := []struct { + name string + srcMeta map[string]string + mode *minio.RetentionMode + err error + want replicationAction + wantCalls int + }{ + { + name: "removal confirmed by destination", + srcMeta: map[string]string{modeKey: "", dateKey: ""}, + err: minio.ErrorResponse{Code: "NoSuchObjectLockConfiguration"}, + want: replicateNone, + wantCalls: 1, + }, + { + name: "destination still holds the retention hidden from HEAD", + srcMeta: map[string]string{modeKey: "", dateKey: ""}, + mode: &governance, + want: replicateMetadata, + wantCalls: 1, + }, + { + name: "retention hidden from HEAD by permissions", + srcMeta: map[string]string{modeKey: "", dateKey: ""}, + err: minio.ErrorResponse{Code: "AccessDenied"}, + want: replicateMetadata, + wantCalls: 1, + }, + { + // A destination that names a missing Object Lock configuration answers the same way + // when its own read of that configuration failed, so it confirms nothing. + name: "destination reports no object lock configuration", + srcMeta: map[string]string{modeKey: "", dateKey: ""}, + err: minio.ErrorResponse{Code: "InvalidRequest", Message: "Bucket is missing ObjectLockConfiguration"}, + want: replicateMetadata, + wantCalls: 1, + }, + { + name: "version never had retention is not confirmed", + srcMeta: nil, + want: replicateNone, + wantCalls: 0, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + src, tgtInfo := newMatchingReplicationPair() + for k, v := range test.srcMeta { + src.UserDefined[k] = v + } + tgt := &fakeRetentionGetter{mode: test.mode, err: test.err} + got := replicationActionForTarget(t.Context(), src, tgtInfo, replication.HealReplicationType, tgt, "bucket", "object") + if got != test.want { + t.Fatalf("replicationActionForTarget() = %q, want %q", got, test.want) + } + if tgt.calls != test.wantCalls { + t.Fatalf("GetObjectRetention called %d times, want %d", tgt.calls, test.wantCalls) + } + }) + } +} + +// TestReplicationActionForTargetNullVersionResync pins that the confirmation does not reopen the +// null-version exclusion at the head of getReplicationAction. An existing object resync returns +// replicateNone for a null version whose source modification time is later than the target's, +// before comparing anything, and that must stand even when the source carries a removed retention +// and the destination would report retention or refuse to answer. +func TestReplicationActionForTargetNullVersionResync(t *testing.T) { + modeKey := strings.ToLower(xhttp.AmzObjectLockMode) + dateKey := strings.ToLower(xhttp.AmzObjectLockRetainUntilDate) + governance := minio.Governance + + tests := []struct { + name string + mode *minio.RetentionMode + err error + }{ + {"destination holds retention", &governance, nil}, + {"retention read denied", nil, minio.ErrorResponse{Code: "AccessDenied"}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + src, tgtInfo := newMatchingReplicationPair() + // A null version whose source modification time is later, and whose content differs, + // so only the exclusion can hold the action at replicateNone. + src.VersionID = nullVersionID + src.ModTime = tgtInfo.LastModified.Add(time.Hour) + src.ETag = "5d41402abc4b2a76b9719d911017c592" + src.UserDefined[modeKey] = "" + src.UserDefined[dateKey] = "" + tgtInfo.VersionID = nullVersionID + + tgt := &fakeRetentionGetter{mode: test.mode, err: test.err} + got := replicationActionForTarget(t.Context(), src, tgtInfo, replication.ExistingObjectReplicationType, tgt, "bucket", "object") + if got != replicateNone { + t.Fatalf("replicationActionForTarget() = %q, want %q", got, replicateNone) + } + if tgt.calls != 0 { + t.Fatalf("GetObjectRetention called %d times, want 0", tgt.calls) + } + }) + } +}