fix(batch): authorize the versions that a purge of all versions deletes

A batch expiration that keeps no versions authorized the versions its
walk saw, and the purge then deleted the versions it read under its
lock. A version written after the walk, or tags changed on one since,
was deleted without being authorized for the submitter.

The purge takes an authorization that a batch purge must set and no
other delete may. Under the object lock, once it has approved the
history and checked every drive, and before its first delete, it
hands the writes it is about to delete to the job: every copy of every
approved version and every revision of one that a drive holds, one per
distinct version ID and tags, as they are then. The job evaluates the
expire actions on each for the submitter. A denial deletes nothing and
ends the job as refused; the time it takes counts against the purge's
budget. A locked version still refuses the whole key, counted as a
refused key. The checks on the walk's snapshot are removed.

Reviewed adversarially; the review's regressions are included.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
Feng Ruohang
2026-11-05 07:10:32 +08:00
parent ae54718d84
commit 9fe32b7743
6 changed files with 661 additions and 80 deletions
@@ -25,6 +25,8 @@ import (
"sync/atomic"
"testing"
"time"
xhttp "github.com/minio/minio/internal/http"
)
// The parts of the purge of all versions that need its own options.
@@ -65,16 +67,22 @@ func TestPurgeAllVersionsOptions(t *testing.T) {
p.version(0, "ea/key", "v1", 5)
match := &deleteAllMatch{}
expire := ExpirationOptions{Expire: true}
allow := func([]ObjectInfo) error { return nil }
for name, opts := range map[string]ObjectOptions{
"no approval": {DeleteAllVersions: true},
"two approvals": {DeleteAllVersions: true, Expiration: expire, DeleteAllLatest: match},
"prefix": {DeleteAllVersions: true, Expiration: expire, DeletePrefix: true},
"prefix object": {DeleteAllVersions: true, Expiration: expire, DeletePrefixObject: true},
"version": {DeleteAllVersions: true, Expiration: expire, VersionID: mustGetUUID()},
"movement": {DeleteAllVersions: true, Expiration: expire, DataMovement: true},
"marker": {DeleteAllVersions: true, Expiration: expire, DeleteMarker: true},
"versioned": {DeleteAllVersions: true, Expiration: expire, Versioned: true},
"suspended": {DeleteAllVersions: true, Expiration: expire, VersionSuspended: true},
"two approvals": {DeleteAllVersions: true, Expiration: expire, DeleteAllLatest: match, deleteAllAuthorize: allow},
// A batch purge deletes as the submitter of its job, an ILM purge
// as the server, and no other delete takes an authorization.
"batch unauthorized": {DeleteAllVersions: true, DeleteAllLatest: match},
"ilm authorized": {DeleteAllVersions: true, Expiration: expire, deleteAllAuthorize: allow},
"authorized delete": {deleteAllAuthorize: allow},
"prefix": {DeleteAllVersions: true, Expiration: expire, DeletePrefix: true},
"prefix object": {DeleteAllVersions: true, Expiration: expire, DeletePrefixObject: true},
"version": {DeleteAllVersions: true, Expiration: expire, VersionID: mustGetUUID()},
"movement": {DeleteAllVersions: true, Expiration: expire, DataMovement: true},
"marker": {DeleteAllVersions: true, Expiration: expire, DeleteMarker: true},
"versioned": {DeleteAllVersions: true, Expiration: expire, Versioned: true},
"suspended": {DeleteAllVersions: true, Expiration: expire, VersionSuspended: true},
} {
if _, err := z.DeleteObject(t.Context(), p.bucket, "ea/key", opts); !errors.As(err, new(InvalidArgument)) {
t.Errorf("%s: %v", name, err)
@@ -134,9 +142,10 @@ func TestPurgeAllVersionsBudgetCoversReads(t *testing.T) {
t.Cleanup(func() { pool.erasureDisksMu.Lock(); pool.erasureDisks = prior; pool.erasureDisksMu.Unlock() })
var res deleteAllResult
_, err := z.DeleteObject(t.Context(), p.bucket, key, ObjectOptions{
DeleteAllVersions: true,
DeleteAllLatest: &deleteAllMatch{VersionID: exp.VersionID, ModTime: exp.ModTime},
DeleteAllResult: &res,
DeleteAllVersions: true,
DeleteAllLatest: &deleteAllMatch{VersionID: exp.VersionID, ModTime: exp.ModTime},
DeleteAllResult: &res,
deleteAllAuthorize: func([]ObjectInfo) error { return nil },
})
if !errors.Is(err, errDeleteAllBudget) || res.Started || res.Removed != 0 {
t.Errorf("purge after its budget ran out on reads: err=%v %+v", err, res)
@@ -213,9 +222,10 @@ func TestPurgeAllVersionsDeletesOnCheckedDrives(t *testing.T) {
})
var res deleteAllResult
if _, err := z.DeleteObject(t.Context(), p.bucket, key, ObjectOptions{
DeleteAllVersions: true,
DeleteAllLatest: &deleteAllMatch{VersionID: exp.VersionID, ModTime: exp.ModTime},
DeleteAllResult: &res,
DeleteAllVersions: true,
DeleteAllLatest: &deleteAllMatch{VersionID: exp.VersionID, ModTime: exp.ModTime},
DeleteAllResult: &res,
deleteAllAuthorize: func([]ObjectInfo) error { return nil },
}); err != nil || !res.Complete {
t.Fatalf("purge: %v %+v", err, res)
}
@@ -223,3 +233,62 @@ func TestPurgeAllVersionsDeletesOnCheckedDrives(t *testing.T) {
t.Errorf("reads=%d deletes on the replacement handle=%d", reads.Load(), deletes.Load())
}
}
// A batch purge has every copy of every version it approved authorized, as
// the copy is now, before it deletes any; an authorization error deletes
// nothing and is returned as it is.
func TestPurgeAllVersionsAuthorizesEveryCopy(t *testing.T) {
z, _ := consistencyPools(t)
p := newPurgeTest(t, z, "purge-authorize", MakeBucketOptions{VersioningEnabled: true}, "")
const key = "bx/authorize"
p.put(0, key, "old", daysAgo(6), ObjectOptions{})
p.put(1, key, "new", daysAgo(5), ObjectOptions{UserDefined: map[string]string{xhttp.AmzObjectTagging: "copy=new"}})
tagged := p.put(0, key, "v1", daysAgo(4), ObjectOptions{Versioned: true, UserDefined: map[string]string{xhttp.AmzObjectTagging: "keep=yes"}})
exp := p.matched(key)
denied := errors.New("denied")
var seen []string // the distinct versions and tags authorized
purge := func(authorize func([]ObjectInfo) error) (deleteAllResult, error) {
var res deleteAllResult
_, err := z.DeleteObject(t.Context(), p.bucket, key, ObjectOptions{
DeleteAllVersions: true,
DeleteAllLatest: &deleteAllMatch{VersionID: exp.VersionID, ModTime: exp.ModTime},
DeleteAllResult: &res,
deleteAllAuthorize: authorize,
})
return res, err
}
res, err := purge(func(versions []ObjectInfo) error {
for _, v := range versions {
if got := v.VersionID + " " + v.UserTags; !slices.Contains(seen, got) {
seen = append(seen, got)
}
}
return denied
})
if !errors.Is(err, denied) || res.Started || res.Removed != 0 {
t.Errorf("denied purge: err=%v %+v", err, res)
}
want := []string{"null ", "null copy=new", tagged.VersionID + " keep=yes"}
slices.Sort(seen)
slices.Sort(want)
if !slices.Equal(seen, want) {
t.Errorf("authorized %q, want every copy %q", seen, want)
}
for pool := range z.serverPools {
if got := p.held(pool, key); len(got) == 0 {
t.Errorf("pool %d lost its copies to a denied purge", pool)
}
}
if got := p.get(key); got != "v1" {
t.Errorf("reads %s after a denied purge, want v1", got)
}
// Control: an allowed purge deletes them all.
if res, err = purge(func([]ObjectInfo) error { return nil }); err != nil || !res.Complete {
t.Fatalf("allowed purge: %v %+v", err, res)
}
for pool := range z.serverPools {
if got := p.held(pool, key); got != nil {
t.Errorf("pool %d keeps %v", pool, got)
}
}
}