From 602ddb31f2c93df1d3add24f09d78fd4b2bd2c60 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sat, 17 Oct 2026 15:25:56 -0700 Subject: [PATCH] s3: count stored versions as bucket contents on delete (#11775) * s3: count stored versions as bucket contents A .versions directory holding version entries is user data; skipping it let bucketHasUserObjects report an empty bucket while the recursive delete dropped every stored version. * test: rename stale reserved-folders case in bucket delete test .versions directories are now descended into, so the case only holds because the folder is empty. --- weed/s3api/s3api_bucket_delete_test.go | 17 +++++++++++++++-- weed/s3api/s3api_bucket_handlers.go | 12 +++++++----- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/weed/s3api/s3api_bucket_delete_test.go b/weed/s3api/s3api_bucket_delete_test.go index 9176ce38c..ab57dee2c 100644 --- a/weed/s3api/s3api_bucket_delete_test.go +++ b/weed/s3api/s3api_bucket_delete_test.go @@ -152,7 +152,7 @@ func TestBucketHasUserObjects_EmptyDirectories(t *testing.T) { wantHasUser: true, }, { - name: "bucket with only reserved folders (.uploads, *.versions)", + name: "empty *.versions folder and .uploads do not count", entriesByDir: map[string][]*filer_pb.Entry{ "/buckets/b": { {Name: s3_constants.MultipartUploadsFolder, IsDirectory: true}, @@ -162,7 +162,7 @@ func TestBucketHasUserObjects_EmptyDirectories(t *testing.T) { wantHasUser: false, }, { - name: "nested versions-named directories are internal", + name: "nested versions directory with stored versions counts", entriesByDir: map[string][]*filer_pb.Entry{ "/buckets/b": { {Name: "logs", IsDirectory: true}, @@ -174,6 +174,19 @@ func TestBucketHasUserObjects_EmptyDirectories(t *testing.T) { {Name: "v1", IsDirectory: false}, }, }, + wantHasUser: true, + }, + { + name: "nested versions directory left empty does not count", + entriesByDir: map[string][]*filer_pb.Entry{ + "/buckets/b": { + {Name: "logs", IsDirectory: true}, + }, + "/buckets/b/logs": { + {Name: "foo" + s3_constants.VersionsFolder, IsDirectory: true}, + }, + "/buckets/b/logs/foo" + s3_constants.VersionsFolder: {}, + }, wantHasUser: false, }, { diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index a2ec2dfdb..f6b1d32dd 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -521,8 +521,9 @@ func (s3a *S3ApiServer) DeleteBucketHandler(w http.ResponseWriter, r *http.Reque } // bucketHasUserObjects checks whether a bucket contains any user objects. -// Empty directories left behind by deleted objects and internal folders -// (.uploads, *.versions) do not count as user objects. +// Empty directories left behind by deleted objects and the .uploads staging +// folder do not count as user objects; stored versions inside a .versions +// directory do. func (s3a *S3ApiServer) bucketHasUserObjects(bucket string) (bool, error) { return s3a.dirHasUserObjects(s3a.option.BucketsPath + "/" + bucket) } @@ -556,9 +557,10 @@ func (s3a *S3ApiServer) dirHasUserObjects(root string) (bool, error) { if entry.IsDirectoryKeyObject() { return true, nil } - // Internal folders are skipped by object listing at every level, - // so a directory with a reserved name never counts. - if isReservedDirectoryName(entry.Name) { + // Multipart staging folders are invisible to object listing at + // every level, so they never count. A .versions directory + // is descended into since it may still hold stored versions. + if entry.Name == s3_constants.MultipartUploadsFolder { continue } dirs = append(dirs, dir+"/"+entry.Name)