diff --git a/test/s3/versioning/s3_deleted_prefix_test.go b/test/s3/versioning/s3_deleted_prefix_test.go new file mode 100644 index 000000000..458c92813 --- /dev/null +++ b/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") +} diff --git a/weed/s3api/s3api_object_handlers_list.go b/weed/s3api/s3api_object_handlers_list.go index 9fe39618b..f6fa358ce 100644 --- a/weed/s3api/s3api_object_handlers_list.go +++ b/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")} diff --git a/weed/s3api/s3api_object_handlers_list_deleted_prefix_test.go b/weed/s3api/s3api_object_handlers_list_deleted_prefix_test.go new file mode 100644 index 000000000..ce3b02c3a --- /dev/null +++ b/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") +}