mirror of
https://github.com/pgsty/minio.git
synced 2026-10-31 12:52:27 +03:00
fix: recognize timestamp-only retention-removal tombstone in resend compare
retentionRemovedAtSource only recognized representation (1) of a removed retention: the object lock key present with an empty value. But a removal that arrived by replication persists representation (2): restoreRetention (and the receiver's replica update path) writes only the retention ordering timestamp when the mode is empty, leaving the mode and retain-until-date keys absent. For that shape the helper returned false, so replicationActionForTarget skipped the GetObjectRetention confirmation and let getReplicationAction's replicateNone stand, silently dropping a needed removal when the destination HEAD hides retention behind a permission-filtered credential. Recognize representation (2) as well: a present retention ordering timestamp with the mode value absent or empty is a removal. A present timestamp paired with a non-empty mode is a retention that was set, not removed, and still returns false. 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>
This commit is contained in:
@@ -1067,15 +1067,29 @@ type objectRetentionGetter interface {
|
|||||||
GetObjectRetention(ctx context.Context, bucketName, objectName, versionID string) (*minio.RetentionMode, *time.Time, error)
|
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,
|
// 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).
|
// Two representations persist. A retention removed directly on this cluster keeps the object lock
|
||||||
|
// keys present with empty values (PutObjectRetentionHandler, cmd/object-handlers.go:3309-3316). A
|
||||||
|
// removal that arrived by replication keeps only the retention ordering timestamp, with the mode
|
||||||
|
// and retain-until-date keys absent, because restoreRetention and the replica update path write
|
||||||
|
// the timestamp alone when the mode is empty (cmd/bucket-object-lock.go:388-399,
|
||||||
|
// cmd/object-handlers.go:1782-1797). A present ordering timestamp paired with a non-empty mode is
|
||||||
|
// a retention that was set, not removed, and must not be mistaken for one.
|
||||||
func retentionRemovedAtSource(oi ObjectInfo) bool {
|
func retentionRemovedAtSource(oi ObjectInfo) bool {
|
||||||
lkMap := caseInsensitiveMap(oi.UserDefined)
|
lkMap := caseInsensitiveMap(oi.UserDefined)
|
||||||
|
// Representation (1): an object lock key is present with an empty value.
|
||||||
for _, k := range []string{xhttp.AmzObjectLockMode, xhttp.AmzObjectLockRetainUntilDate} {
|
for _, k := range []string{xhttp.AmzObjectLockMode, xhttp.AmzObjectLockRetainUntilDate} {
|
||||||
if v, ok := lkMap.Lookup(k); ok && v == "" {
|
if v, ok := lkMap.Lookup(k); ok && v == "" {
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// Representation (2): a recorded retention ordering timestamp with the mode value absent or
|
||||||
|
// empty is a removal restoreRetention persisted without the empty public keys.
|
||||||
|
if _, ok := oi.UserDefined[ReservedMetadataPrefixLower+ObjectLockRetentionTimestamp]; ok {
|
||||||
|
if v, ok := lkMap.Lookup(xhttp.AmzObjectLockMode); !ok || v == "" {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -844,6 +844,8 @@ func (f *fakeRetentionGetter) GetObjectRetention(_ context.Context, _, _, _ stri
|
|||||||
func TestRetentionRemovedAtSource(t *testing.T) {
|
func TestRetentionRemovedAtSource(t *testing.T) {
|
||||||
modeKey := strings.ToLower(xhttp.AmzObjectLockMode)
|
modeKey := strings.ToLower(xhttp.AmzObjectLockMode)
|
||||||
dateKey := strings.ToLower(xhttp.AmzObjectLockRetainUntilDate)
|
dateKey := strings.ToLower(xhttp.AmzObjectLockRetainUntilDate)
|
||||||
|
tsKey := ReservedMetadataPrefixLower + ObjectLockRetentionTimestamp
|
||||||
|
stamp := "2026-09-06T01:00:00Z"
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
meta map[string]string
|
meta map[string]string
|
||||||
@@ -856,6 +858,11 @@ func TestRetentionRemovedAtSource(t *testing.T) {
|
|||||||
{"real retention", map[string]string{modeKey: "GOVERNANCE", dateKey: "2026-10-05T10:00:00.000Z"}, false},
|
{"real retention", map[string]string{modeKey: "GOVERNANCE", dateKey: "2026-10-05T10:00:00.000Z"}, false},
|
||||||
{"canonical case", map[string]string{xhttp.AmzObjectLockMode: ""}, true},
|
{"canonical case", map[string]string{xhttp.AmzObjectLockMode: ""}, true},
|
||||||
{"empty user metadata", map[string]string{"x-amz-meta-foo": ""}, false},
|
{"empty user metadata", map[string]string{"x-amz-meta-foo": ""}, false},
|
||||||
|
// Representation (2): a replicated removal persists the ordering timestamp alone,
|
||||||
|
// with the mode and retain-until-date keys absent (restoreRetention).
|
||||||
|
{"timestamp only, mode absent", map[string]string{tsKey: stamp}, true},
|
||||||
|
{"timestamp with empty mode", map[string]string{tsKey: stamp, modeKey: ""}, true},
|
||||||
|
{"timestamp with real retention is a set, not a removal", map[string]string{tsKey: stamp, modeKey: "GOVERNANCE", dateKey: "2026-10-05T10:00:00.000Z"}, false},
|
||||||
}
|
}
|
||||||
for _, test := range tests {
|
for _, test := range tests {
|
||||||
t.Run(test.name, func(t *testing.T) {
|
t.Run(test.name, func(t *testing.T) {
|
||||||
@@ -1030,3 +1037,68 @@ func TestReplicationActionForTargetNullVersionResync(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestReplicationActionForTargetTimestampOnlyRemoval covers representation (2) of a removed
|
||||||
|
// retention. A removal that arrived by replication persists only the retention ordering timestamp,
|
||||||
|
// with the mode and retain-until-date keys absent, because restoreRetention writes the timestamp
|
||||||
|
// alone when the mode is empty (cmd/bucket-object-lock.go). The comparison in getReplicationAction
|
||||||
|
// reads such a source as in sync with a matching destination, so only the GetObjectRetention
|
||||||
|
// confirmation separates a destination that dropped the retention from one hiding it behind a
|
||||||
|
// permission-filtered HEAD. The source is built through the real restoreRetention path so the
|
||||||
|
// fixture is the metadata a replicated removal actually leaves on disk, not a hand-rolled map.
|
||||||
|
func TestReplicationActionForTargetTimestampOnlyRemoval(t *testing.T) {
|
||||||
|
governance := minio.Governance
|
||||||
|
stamp := time.Date(2026, 9, 6, 1, 0, 0, 0, time.UTC).Format(time.RFC3339Nano)
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
mode *minio.RetentionMode
|
||||||
|
err error
|
||||||
|
want replicationAction
|
||||||
|
wantCalls int
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
// The reference case: a destination that denies the retention read is
|
||||||
|
// indistinguishable from one still holding it, so the removal is resent.
|
||||||
|
name: "retention hidden from HEAD by permissions",
|
||||||
|
err: minio.ErrorResponse{Code: "AccessDenied"},
|
||||||
|
want: replicateMetadata,
|
||||||
|
wantCalls: 1,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "destination still holds the retention",
|
||||||
|
mode: &governance,
|
||||||
|
want: replicateMetadata,
|
||||||
|
wantCalls: 1,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "removal confirmed by destination",
|
||||||
|
err: minio.ErrorResponse{Code: "NoSuchObjectLockConfiguration"},
|
||||||
|
want: replicateNone,
|
||||||
|
wantCalls: 1,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, test := range tests {
|
||||||
|
t.Run(test.name, func(t *testing.T) {
|
||||||
|
src, tgtInfo := newMatchingReplicationPair()
|
||||||
|
// Persist the timestamp-only tombstone the same way an applied replica removal does.
|
||||||
|
objectLockState{retentionTimestamp: stamp}.restoreRetention(src.UserDefined)
|
||||||
|
if !retentionRemovedAtSource(src) {
|
||||||
|
t.Fatalf("restoreRetention fixture not recognized as a removal: %v", src.UserDefined)
|
||||||
|
}
|
||||||
|
if _, ok := src.UserDefined[strings.ToLower(xhttp.AmzObjectLockMode)]; ok {
|
||||||
|
t.Fatalf("restoreRetention fixture wrote a mode key, fixture is not timestamp-only: %v", src.UserDefined)
|
||||||
|
}
|
||||||
|
|
||||||
|
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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user