s3: stop listing prefixes whose objects are all delete-marked (#10419)

Deleting the only object under a prefix in a versioned bucket writes a
delete marker and keeps the version history, so the filer directory
survives with nothing a current-version listing would return. A delimited
ListObjects kept reporting that path in CommonPrefixes, because the
prefixes come from the directory tree rather than from the keys, while a
listing scoped inside the prefix correctly came back empty.

Probe a directory before reporting it: one that holds entries but no key
the listing returns is neither a CommonPrefix nor a path the
trailing-slash probe answers for. Empty directories keep the meaning they
have today, and the probe only runs for buckets with versioning
configured, the only ones that can reach this state.

test/s3/versioning/s3_deleted_prefix_test.go

@@ -0,0 +1,106 @@
+package s3api
+
+import (
+	"context"
+	"testing"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/s3"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestDeletedPrefixLeavesNoCommonPrefix covers the versioned bucket case where the only
+// object under a prefix is deleted. The delete marker and the version history stay, so
+// the directory survives in the filer, but a current-version listing has no key under
+// that path and AWS reports no CommonPrefix for it.
+func TestDeletedPrefixLeavesNoCommonPrefix(t *testing.T) {
+	client := getS3Client(t)
+	bucketName := getNewBucketName()
+
+	createBucket(t, client, bucketName)
+	defer deleteBucket(t, client, bucketName)
+	enableVersioning(t, client, bucketName)
+
+	putObject(t, client, bucketName, "backup/20260101/manifest", "first backup")
+	putObject(t, client, bucketName, "backup/20260102/manifest", "second backup")
+
+	listPrefixes := func(prefix string) []string {
+		t.Helper()
+		input := &s3.ListObjectsV2Input{
+			Bucket:    aws.String(bucketName),
+			Delimiter: aws.String("/"),
+		}
+		if prefix != "" {
+			input.Prefix = aws.String(prefix)
+		}
+		resp, err := client.ListObjectsV2(context.TODO(), input)
+		require.NoError(t, err)
+		prefixes := make([]string, 0, len(resp.CommonPrefixes))
+		for _, p := range resp.CommonPrefixes {
+			prefixes = append(prefixes, *p.Prefix)
+		}
+		return prefixes
+	}
+	listKeys := func(prefix string) []string {
+		t.Helper()
+		input := &s3.ListObjectsV2Input{
+			Bucket:    aws.String(bucketName),
+			Delimiter: aws.String("/"),
+		}
+		if prefix != "" {
+			input.Prefix = aws.String(prefix)
+		}
+		resp, err := client.ListObjectsV2(context.TODO(), input)
+		require.NoError(t, err)
+		keys := make([]string, 0, len(resp.Contents))
+		for _, c := range resp.Contents {
+			keys = append(keys, *c.Key)
+		}
+		return keys
+	}
+	deleteKey := func(key string) {
+		t.Helper()
+		_, err := client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{
+			Bucket: aws.String(bucketName),
+			Key:    aws.String(key),
+		})
+		require.NoError(t, err)
+	}
+
+	assert.ElementsMatch(t, []string{"backup/20260101/", "backup/20260102/"}, listPrefixes("backup/"))
+
+	deleteKey("backup/20260101/manifest")
+	assert.Equal(t, []string{"backup/20260102/"}, listPrefixes("backup/"),
+		"the delete-marked date must drop out of CommonPrefixes")
+	assert.Equal(t, []string{"backup/"}, listPrefixes(""),
+		"the parent still holds a live object so it keeps its prefix")
+
+	deleteKey("backup/20260102/manifest")
+	assert.Empty(t, listPrefixes("backup/"), "no date is left under backup/")
+	assert.Empty(t, listKeys("backup/"), "and backup/ itself is not a key")
+	assert.Empty(t, listPrefixes(""), "the bucket lists as empty once every object is delete-marked")
+
+	// The version history is untouched: both objects still have their version and their
+	// delete marker, and removing a delete marker brings the prefix back.
+	versions, err := client.ListObjectVersions(context.TODO(), &s3.ListObjectVersionsInput{
+		Bucket: aws.String(bucketName),
+	})
+	require.NoError(t, err)
+	assert.Len(t, versions.Versions, 2)
+	assert.Len(t, versions.DeleteMarkers, 2)
+
+	for _, marker := range versions.DeleteMarkers {
+		if *marker.Key != "backup/20260102/manifest" {
+			continue
+		}
+		_, err := client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{
+			Bucket:    aws.String(bucketName),
+			Key:       marker.Key,
+			VersionId: marker.VersionId,
+		})
+		require.NoError(t, err)
+	}
+	assert.Equal(t, []string{"backup/20260102/"}, listPrefixes("backup/"),
+		"removing the delete marker restores the prefix")
+}

weed/s3api/s3api_object_handlers_list.go

@@ -306,6 +306,9 @@ func (s3a *S3ApiServer) listFilerEntries(ctx context.Context, req listObjectsReq
 		// Hoist versioning check out of per-entry callback
 		versioningState, _ := s3a.getVersioningState(bucket)
 		versioningEnabled := versioningState == "Enabled"
+		// Suspending versioning keeps the delete markers already written, so both states
+		// can hold a directory whose objects are all gone from the current-version view.
+		cursor.hideDeletedPrefixes = versioningState != ""
 
 		// Helper function to handle dedup/append logic
 		appendOrDedup := func(newEntry ListEntry) {
@@ -504,6 +507,10 @@ type ListingCursor struct {
 	maxKeys               uint16
 	isTruncated           bool
 	prefixEndsOnDelimiter bool
+	// hideDeletedPrefixes turns on the dirHoldsOnlyHiddenEntries probe, which only has
+	// something to find once a bucket has version history to leave behind.
+	hideDeletedPrefixes bool
+	probedEntries       int
 }
 
 // the prefix and marker may be in different directories
@@ -758,7 +765,10 @@ func (s3a *S3ApiServer) doListFilerEntries(ctx context.Context, client filer_pb.
 					// identical to a directory created via PutObject with a trailing "/", so
 					// tools like hadoop-aws can find it. Plain listings are left untouched, so
 					// empty directories left behind by deleted objects are not shown as keys.
-					if explicitDirProbe && !isKeyObject && !childEmitted && !cursor.isTruncated && entry.Attributes != nil {
+					// A directory that still holds version history no longer names anything,
+					// so it gets no marker either.
+					if explicitDirProbe && !isKeyObject && !childEmitted && !cursor.isTruncated && entry.Attributes != nil &&
+						!s3a.dirHoldsOnlyHiddenEntries(ctx, client, bucket, dir+"/"+entry.Name, cursor) {
 						entry.Attributes.Mime = s3_constants.FolderMimeType
 						eachEntryFn(dir, entry)
 					}
@@ -771,7 +781,7 @@ func (s3a *S3ApiServer) doListFilerEntries(ctx context.Context, client filer_pb.
 						return
 					}
 					// println("doListFilerEntries2 nextMarker", nextMarker)
-				} else {
+				} else if entry.IsDirectoryKeyObject() || !s3a.dirHoldsOnlyHiddenEntries(ctx, client, bucket, dir+"/"+entry.Name, cursor) {
 					eachEntryFn(dir, entry)
 				}
 			} else {
@@ -791,6 +801,106 @@ func (s3a *S3ApiServer) doListFilerEntries(ctx context.Context, client filer_pb.
 	}
 }
 
+// hiddenProbePageSize is the window one probe request asks the filer for, and
+// hiddenProbeBudget caps how many entries a single list request may look at while
+// deciding which directories still stand for a prefix.
+const (
+	hiddenProbePageSize = 64
+	hiddenProbeBudget   = 10000
+)
+
+// dirHoldsOnlyHiddenEntries reports whether dir holds entries but none that a
+// current-version listing returns. Deleting the last object under a prefix in a
+// versioned bucket leaves the version history and a delete marker behind, so the filer
+// directory survives with nothing listable in it. AWS derives CommonPrefixes from the
+// keys a listing returns, so that path is no longer a prefix and no longer a directory
+// to answer a probe for. An empty directory is left alone: mount and mkdir create them
+// and empty-folder cleanup owns their lifetime.
+//
+// The scan stops at the first key it finds, so a populated prefix costs one ListEntries
+// answered by its first entry. A subtree that is entirely delete-marked costs a walk of
+// that subtree, bounded by the request's probe budget; once the budget is spent the
+// prefix is reported, as it was before this check existed.
+func (s3a *S3ApiServer) dirHoldsOnlyHiddenEntries(ctx context.Context, client filer_pb.SeaweedFilerClient, bucket, dir string, cursor *ListingCursor) bool {
+	if !cursor.hideDeletedPrefixes {
+		return false
+	}
+
+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+
+	sawEntry := false
+	startFrom := ""
+	for {
+		request := &filer_pb.ListEntriesRequest{
+			Directory:         dir,
+			StartFromFileName: startFrom,
+			Limit:             hiddenProbePageSize,
+		}
+		stream, listErr := client.ListEntries(ctx, request)
+		if listErr != nil {
+			if !errors.Is(listErr, filer_pb.ErrNotFound) {
+				glog.V(1).Infof("dirHoldsOnlyHiddenEntries %s: %v", dir, listErr)
+			}
+			return false
+		}
+
+		var entriesReceived uint32
+		for {
+			resp, recvErr := stream.Recv()
+			if recvErr != nil {
+				if recvErr != io.EOF {
+					glog.V(1).Infof("dirHoldsOnlyHiddenEntries %s: %v", dir, recvErr)
+					return false
+				}
+				break
+			}
+			entry := resp.Entry
+			if entry == nil {
+				continue
+			}
+			entriesReceived++
+			startFrom = entry.Name
+			sawEntry = true
+
+			cursor.probedEntries++
+			if cursor.probedEntries > hiddenProbeBudget {
+				return false
+			}
+
+			if !entry.IsDirectory {
+				return false
+			}
+			if entry.Name == s3_constants.MultipartUploadsFolder {
+				continue
+			}
+			if strings.HasSuffix(entry.Name, s3_constants.VersionsFolder) {
+				// Each write that changes an object's current version stamps the answer
+				// onto its .versions directory entry, which the listing above already
+				// carries, so a delete-marked object costs nothing to recognize. A
+				// missing stamp leaves the current version unknown - the pointer is
+				// written on the key's owner filer and may not have reached the filer
+				// serving this list - and an unknown object keeps its prefix rather than
+				// turning one listing into a version rescan per object.
+				if isDeleteMarker, stamped := entry.Extended[s3_constants.ExtLatestVersionIsDeleteMarker]; stamped && string(isDeleteMarker) == "true" {
+					continue
+				}
+				return false
+			}
+			if entry.IsDirectoryKeyObject() {
+				return false
+			}
+			if !s3a.dirHoldsOnlyHiddenEntries(ctx, client, bucket, dir+"/"+entry.Name, cursor) {
+				return false
+			}
+		}
+
+		if entriesReceived < request.Limit {
+			return sawEntry
+		}
+	}
+}
+
 func getListObjectsV2Args(values url.Values) (prefix, startAfter, delimiter string, token OptionalString, encodingTypeUrl bool, fetchOwner bool, maxkeys uint16, allowUnordered bool, errCode s3err.ErrorCode) {
 	prefix = values.Get("prefix")
 	token = OptionalString{set: values.Has("continuation-token"), string: values.Get("continuation-token")}

weed/s3api/s3api_object_handlers_list_deleted_prefix_test.go

@@ -0,0 +1,224 @@
+package s3api
+
+import (
+	"context"
+	"strconv"
+	"testing"
+	"time"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
+	"github.com/stretchr/testify/assert"
+)
+
+// deleteMarkedVersionsDir builds the .versions directory left behind when the current
+// version of a versioned object is a delete marker.
+func deleteMarkedVersionsDir(object string) *filer_pb.Entry {
+	return &filer_pb.Entry{
+		Name:        object + s3_constants.VersionsFolder,
+		IsDirectory: true,
+		Attributes:  &filer_pb.FuseAttributes{Mtime: time.Now().Unix()},
+		Extended: map[string][]byte{
+			s3_constants.ExtLatestVersionIdKey:          []byte("v-deleted"),
+			s3_constants.ExtLatestVersionIsDeleteMarker: []byte("true"),
+		},
+	}
+}
+
+// liveVersionsDir builds the .versions directory of a versioned object whose current
+// version is a real object.
+func liveVersionsDir(object string) *filer_pb.Entry {
+	now := time.Now().Unix()
+	return &filer_pb.Entry{
+		Name:        object + s3_constants.VersionsFolder,
+		IsDirectory: true,
+		Attributes:  &filer_pb.FuseAttributes{Mtime: now},
+		Extended: map[string][]byte{
+			s3_constants.ExtLatestVersionIdKey:          []byte("v-live"),
+			s3_constants.ExtLatestVersionSizeKey:        []byte("6"),
+			s3_constants.ExtLatestVersionMtimeKey:       []byte(strconv.FormatInt(now, 10)),
+			s3_constants.ExtLatestVersionETagKey:        []byte(`"b1946ac92492d2347c6235b4d2611184"`),
+			s3_constants.ExtLatestVersionIsDeleteMarker: []byte("false"),
+		},
+	}
+}
+
+func newDir(name string) *filer_pb.Entry {
+	return &filer_pb.Entry{Name: name, IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}}
+}
+
+// listedNames collects what a listing would emit. Like listFilerEntries, every emitted
+// entry spends one slot of the page budget, so a cursor passed in with a small maxKeys
+// exercises the same truncation the real callback drives.
+func listedNames(t *testing.T, client filer_pb.SeaweedFilerClient, req listDirectoryRequest, cursor *ListingCursor) []string {
+	t.Helper()
+	s3a := &S3ApiServer{option: &S3ApiServerOption{BucketsPath: "/buckets"}}
+	var seen []string
+	_, err := s3a.doListFilerEntries(context.Background(), client, req, cursor, func(dir string, entry *filer_pb.Entry) {
+		seen = append(seen, entry.Name)
+		cursor.maxKeys--
+	})
+	assert.NoError(t, err)
+	return seen
+}
+
+// TestDeletedPrefixIsNotACommonPrefix covers the versioned bucket case: deleting the
+// only object under a prefix writes a delete marker, so no current version remains
+// under it. The filer keeps the directory and the version history, but AWS derives
+// CommonPrefixes from the keys a listing returns, so the prefix must be gone.
+func TestDeletedPrefixIsNotACommonPrefix(t *testing.T) {
+	client := &testFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/buckets/test":                 {newDir("backup")},
+			"/buckets/test/backup":          {newDir("20260101")},
+			"/buckets/test/backup/20260101": {deleteMarkedVersionsDir("manifest")},
+		},
+	}
+
+	seen := listedNames(t, client, listDirectoryRequest{dir: "/buckets/test", delimiter: "/", bucket: "test"}, &ListingCursor{maxKeys: 1000, hideDeletedPrefixes: true})
+	assert.Empty(t, seen, "a prefix whose only object is delete-marked must not be listed")
+
+	seen = listedNames(t, client, listDirectoryRequest{dir: "/buckets/test/backup", delimiter: "/", bucket: "test"}, &ListingCursor{maxKeys: 1000, hideDeletedPrefixes: true})
+	assert.Empty(t, seen, "the parent listing must not report the deleted date prefix either")
+}
+
+// TestLivePrefixIsStillACommonPrefix guards the other half: a prefix whose versioned
+// object still has a current version keeps being reported.
+func TestLivePrefixIsStillACommonPrefix(t *testing.T) {
+	client := &testFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/buckets/test":                 {newDir("backup")},
+			"/buckets/test/backup":          {newDir("20260101"), newDir("20260102")},
+			"/buckets/test/backup/20260101": {deleteMarkedVersionsDir("manifest")},
+			"/buckets/test/backup/20260102": {liveVersionsDir("manifest")},
+		},
+	}
+
+	seen := listedNames(t, client, listDirectoryRequest{dir: "/buckets/test", delimiter: "/", bucket: "test"}, &ListingCursor{maxKeys: 1000, hideDeletedPrefixes: true})
+	assert.Equal(t, []string{"backup"}, seen)
+
+	seen = listedNames(t, client, listDirectoryRequest{dir: "/buckets/test/backup", delimiter: "/", bucket: "test"}, &ListingCursor{maxKeys: 1000, hideDeletedPrefixes: true})
+	assert.Equal(t, []string{"20260102"}, seen, "only the date prefix with a current version is listed")
+}
+
+// TestDeletedPrefixGetsNoDirectoryMarker checks the trailing-slash probe. The empty
+// directory marker exists for directories created out of band; a directory that only
+// holds version history names nothing, so the probe answers empty like AWS does.
+func TestDeletedPrefixGetsNoDirectoryMarker(t *testing.T) {
+	client := &testFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/buckets/test":        {newDir("backup")},
+			"/buckets/test/backup": {deleteMarkedVersionsDir("manifest")},
+		},
+	}
+
+	seen := listedNames(t, client, listDirectoryRequest{dir: "/buckets/test", prefix: "backup", delimiter: "/", bucket: "test"},
+		&ListingCursor{maxKeys: 1000, prefixEndsOnDelimiter: true, hideDeletedPrefixes: true})
+	assert.Empty(t, seen, "prefix=backup/ must not answer with a backup/ key")
+}
+
+// TestEmptyDirectoryStaysACommonPrefix pins the boundary of the change: an empty
+// directory keeps the meaning it has today, since mount and mkdir create them and
+// empty-folder cleanup owns their lifetime.
+func TestEmptyDirectoryStaysACommonPrefix(t *testing.T) {
+	client := &testFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/buckets/test":      {newDir("logs")},
+			"/buckets/test/logs": {},
+		},
+	}
+
+	seen := listedNames(t, client, listDirectoryRequest{dir: "/buckets/test", delimiter: "/", bucket: "test"}, &ListingCursor{maxKeys: 1000, hideDeletedPrefixes: true})
+	assert.Equal(t, []string{"logs"}, seen)
+}
+
+// TestDirectoryKeyObjectListedDespiteDeletedChildren covers a directory created with
+// PutObject on a trailing-slash key: that key exists in its own right, so it is
+// reported no matter what happened to the objects below it.
+func TestDirectoryKeyObjectListedDespiteDeletedChildren(t *testing.T) {
+	keyObject := newDir("backup")
+	keyObject.Attributes.Mime = s3_constants.FolderMimeType
+
+	client := &testFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/buckets/test":        {keyObject},
+			"/buckets/test/backup": {deleteMarkedVersionsDir("manifest")},
+		},
+	}
+
+	seen := listedNames(t, client, listDirectoryRequest{dir: "/buckets/test", delimiter: "/", bucket: "test"}, &ListingCursor{maxKeys: 1000, hideDeletedPrefixes: true})
+	assert.Equal(t, []string{"backup"}, seen)
+}
+
+// TestUnstampedVersionKeepsThePrefix covers a .versions entry carrying no
+// current-version stamp, which happens while the pointer written on the key's owner
+// filer has not reached the filer serving the list. An object whose current version is
+// unknown keeps its prefix instead of disappearing on a guess.
+func TestUnstampedVersionKeepsThePrefix(t *testing.T) {
+	unstamped := &filer_pb.Entry{
+		Name:        "obj" + s3_constants.VersionsFolder,
+		IsDirectory: true,
+		Attributes:  &filer_pb.FuseAttributes{},
+	}
+	client := &testFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/buckets/test":   {newDir("p")},
+			"/buckets/test/p": {unstamped},
+		},
+	}
+
+	seen := listedNames(t, client, listDirectoryRequest{dir: "/buckets/test", delimiter: "/", bucket: "test"}, &ListingCursor{maxKeys: 1000, hideDeletedPrefixes: true})
+	assert.Equal(t, []string{"p"}, seen)
+}
+
+// TestLiveVersionStampKeepsThePrefix pins the other stamp value: a current version that
+// is not a delete marker is a key, so its prefix is reported.
+func TestLiveVersionStampKeepsThePrefix(t *testing.T) {
+	client := &testFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/buckets/test":   {newDir("p")},
+			"/buckets/test/p": {liveVersionsDir("obj")},
+		},
+	}
+
+	seen := listedNames(t, client, listDirectoryRequest{dir: "/buckets/test", delimiter: "/", bucket: "test"}, &ListingCursor{maxKeys: 1000, hideDeletedPrefixes: true})
+	assert.Equal(t, []string{"p"}, seen)
+}
+
+// TestBucketWithoutVersioningSkipsTheProbe pins the gate. A bucket that never had
+// versioning cannot grow a directory full of delete markers, so its listings keep
+// reporting every directory without paying for a probe per prefix.
+func TestBucketWithoutVersioningSkipsTheProbe(t *testing.T) {
+	client := &testFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/buckets/test":        {newDir("backup")},
+			"/buckets/test/backup": {deleteMarkedVersionsDir("manifest")},
+		},
+	}
+
+	seen := listedNames(t, client, listDirectoryRequest{dir: "/buckets/test", delimiter: "/", bucket: "test"}, &ListingCursor{maxKeys: 1000})
+	assert.Equal(t, []string{"backup"}, seen)
+}
+
+// TestDeletedPrefixesDoNotConsumeMaxKeys makes sure paging steps over the deleted
+// prefixes instead of returning short pages of nothing.
+func TestDeletedPrefixesDoNotConsumeMaxKeys(t *testing.T) {
+	root := []*filer_pb.Entry{newDir("p1"), newDir("p2"), newDir("p3"), newDir("p4")}
+	client := &testFilerClient{
+		entriesByDir: map[string][]*filer_pb.Entry{
+			"/buckets/test":    root,
+			"/buckets/test/p1": {deleteMarkedVersionsDir("obj")},
+			"/buckets/test/p2": {liveVersionsDir("obj")},
+			"/buckets/test/p3": {deleteMarkedVersionsDir("obj")},
+			"/buckets/test/p4": {liveVersionsDir("obj")},
+		},
+	}
+
+	// Room for exactly the two live prefixes. A hidden prefix that spent a slot would
+	// push p4 out of the page and report the listing as truncated.
+	cursor := &ListingCursor{maxKeys: 2, hideDeletedPrefixes: true}
+	seen := listedNames(t, client, listDirectoryRequest{dir: "/buckets/test", delimiter: "/", bucket: "test"}, cursor)
+	assert.Equal(t, []string{"p2", "p4"}, seen)
+	assert.False(t, cursor.isTruncated, "stepping over deleted prefixes must not truncate the page")
+	assert.Zero(t, cursor.maxKeys, "only the live prefixes may spend the page budget")
+}