fix(object-lock): check a directory object's lock before a bulk delete

A directory object always has the null version. DeleteObjects without a
version ID chose whether to read the object before it turned the version
into null, so with Object Lock and no replication or tiering it checked
retention against empty metadata and removed a directory object under
legal hold or retention. Choose the null version before the lookup.

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-09 19:13:21 +08:00
parent 7e9ee6befc
commit aa2d79daf2
3 changed files with 65 additions and 6 deletions
+1 -1
View File
@@ -127,7 +127,7 @@ jobs:
- name: Run Object Lock authorization and overwrite tests under race detector
env:
MINIO_API_REQUESTS_MAX: "10000"
run: go test -race -tags kqueue,dev ./cmd -run '^Test(APIPutObjectRetention|APIVersionAddressedWrite|PutObjectRetention|DirectoryObjectLock|ObjectLockOverwrite|APICopyObjectPreservesVersionAfterSuspension|MultipartNullObjectLock|PoolsObjectLockOverwrite|ReplicaDeleteObjectLock|PoolsReplicaDeleteObjectLock|MultipartVersioningTransitions|PlanReview|RecoveredMultipartUpload)' -count=1 -timeout=5m
run: go test -race -tags kqueue,dev ./cmd -run '^Test(APIPutObjectRetention|APIVersionAddressedWrite|PutObjectRetention|DirectoryObjectLock|ObjectLockOverwrite|APICopyObjectPreservesVersionAfterSuspension|MultipartNullObjectLock|PoolsObjectLockOverwrite|ReplicaDeleteObjectLock|PoolsReplicaDeleteObjectLock|BulkDeleteDirectoryObjectLock|MultipartVersioningTransitions|PlanReview|RecoveredMultipartUpload)' -count=1 -timeout=5m
- name: Run maintenance storage timeout and compatibility tests under race detector
env:
+7 -5
View File
@@ -523,6 +523,13 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
}
}
// All deletes on directory objects needs to be for `nullVersionID`.
// Decide this before the lookup below, so that the retention check
// reads the version the delete removes.
if isDirObject(object.ObjectName) && object.VersionID == "" {
object.VersionID = nullVersionID
}
opts := ObjectOptions{
VersionID: object.VersionID,
Versioned: vc.PrefixEnabled(object.ObjectName),
@@ -541,11 +548,6 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
oss[index].SetTransitionState(goi.TransitionedObject)
}
// All deletes on directory objects needs to be for `nullVersionID`
if isDirObject(object.ObjectName) && object.VersionID == "" {
object.VersionID = nullVersionID
}
if replicateDeletes {
dsc = checkReplicateDelete(ctx, bucket, ObjectToDelete{
ObjectV: ObjectV{
+57
View File
@@ -0,0 +1,57 @@
// Copyright (c) 2026 PGSTY
// SPDX-License-Identifier: AGPL-3.0-or-later
package cmd
import (
"bytes"
"encoding/xml"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/minio/minio/internal/auth"
)
// A directory object always has the null version, and a bulk delete without a
// version ID removes it. The retention check must read that version's lock.
func TestBulkDeleteDirectoryObjectLock(t *testing.T) {
defer DetectTestLeak(t)()
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
t: t, endpoints: []string{"DeleteMultipleObjects"},
objAPITest: func(obj ObjectLayer, instance, bucket string, router http.Handler, cred auth.Credentials, t *testing.T) {
enableBucketObjectLock(t, bucket)
for _, kind := range append([]string{"unlocked"}, batchLockKinds...) {
key := "dir-" + kind + "/"
if _, err := obj.PutObject(t.Context(), bucket, key, mustGetPutObjReader(t, bytes.NewReader(nil), 0, "", ""),
ObjectOptions{Versioned: true, VersionID: nullVersionID, UserDefined: batchLockMeta(kind)}); err != nil {
t.Fatal(err)
}
body := encodeResponse(DeleteObjectsRequest{Objects: []ObjectToDelete{{ObjectV: ObjectV{ObjectName: key}}}})
req, err := newTestSignedRequestV4(http.MethodPost, getDeleteMultipleObjectsURL("", bucket), int64(len(body)), bytes.NewReader(body),
cred.AccessKey, cred.SecretKey, nil)
if err != nil {
t.Fatal(err)
}
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
var resp DeleteObjectsResponse
if rec.Code != http.StatusOK || xml.Unmarshal(rec.Body.Bytes(), &resp) != nil {
t.Fatalf("%s: %s: status %d: %s", instance, key, rec.Code, rec.Body.String())
}
exists := batchVersionExists(t, obj, bucket, key, nullVersionID)
if kind == "unlocked" {
if exists || len(resp.DeletedObjects) != 1 {
t.Errorf("%s: %s: unlocked directory object was not deleted: %s", instance, key, rec.Body.String())
}
continue
}
if !exists || len(resp.Errors) != 1 || len(resp.DeletedObjects) != 0 ||
resp.Errors[0].Key != key || !strings.Contains(resp.Errors[0].Message, "WORM") {
t.Errorf("%s: %s: want the WORM error and the object kept (exists=%v): %s", instance, key, exists, rec.Body.String())
}
}
},
})
}