diff --git a/pkg/storage/s3.go b/pkg/storage/s3.go index 25c3a4ee1..3d2f6eac8 100644 --- a/pkg/storage/s3.go +++ b/pkg/storage/s3.go @@ -378,10 +378,16 @@ func (rs *S3Storage) WalkDir(ctx context.Context, opt *WalkOption, fn func(strin if opt == nil { opt = &WalkOption{} } - prefix := rs.options.Prefix + opt.SubDir - if !strings.HasSuffix(prefix, "/") { - prefix += "/" + + // NOTE: leave prefix empty if subDir is not set. Else, s3 will return empty result! + prefix := "" + if len(opt.SubDir) > 0 { + prefix = rs.options.Prefix + opt.SubDir + if len(prefix) > 0 && !strings.HasSuffix(prefix, "/") { + prefix += "/" + } } + maxKeys := int64(1000) if opt.ListCount > 0 { maxKeys = opt.ListCount @@ -389,9 +395,11 @@ func (rs *S3Storage) WalkDir(ctx context.Context, opt *WalkOption, fn func(strin req := &s3.ListObjectsInput{ Bucket: aws.String(rs.options.Bucket), - Prefix: aws.String(prefix), MaxKeys: aws.Int64(maxKeys), } + if len(prefix) > 0 { + req.Prefix = aws.String(prefix) + } for { // FIXME: We can't use ListObjectsV2, it is not universally supported. // (Ceph RGW supported ListObjectsV2 since v15.1.0, released 2020 Jan 30th) diff --git a/pkg/storage/s3_test.go b/pkg/storage/s3_test.go index 97b715277..ea73f8a1e 100644 --- a/pkg/storage/s3_test.go +++ b/pkg/storage/s3_test.go @@ -30,7 +30,12 @@ type s3Suite struct { storage *S3Storage } -var _ = Suite(&s3Suite{}) +type s3SuiteCustom struct{} + +var ( + _ = Suite(&s3Suite{}) + _ = Suite(&s3SuiteCustom{}) +) // FIXME: Cannot use the real SetUpTest/TearDownTest to set up the mock // otherwise the mock error will be ignored. @@ -893,3 +898,62 @@ func (s *s3Suite) TestWalkDir(c *C) { c.Assert(err, IsNil) c.Assert(i, Equals, len(contents)) } + +// TestWalkDirBucket checks WalkDir retrieves all directory content under a bucket. +func (s *s3SuiteCustom) TestWalkDirWithEmptyPrefix(c *C) { + controller := gomock.NewController(c) + s3API := mock.NewMockS3API(controller) + storage := NewS3StorageForTest( + s3API, + &backup.S3{ + Region: "us-west-2", + Bucket: "bucket", + Prefix: "", + Acl: "acl", + Sse: "sse", + StorageClass: "sc", + }, + ) + defer controller.Finish() + ctx := aws.BackgroundContext() + + contents := []*s3.Object{ + { + Key: aws.String("sp/.gitignore"), + Size: aws.Int64(437), + }, + { + Key: aws.String("prefix/sp/01.jpg"), + Size: aws.Int64(27499), + }, + } + s3API.EXPECT(). + ListObjectsWithContext(ctx, gomock.Any()). + DoAndReturn(func(_ context.Context, input *s3.ListObjectsInput) (*s3.ListObjectsOutput, error) { + c.Assert(aws.StringValue(input.Bucket), Equals, "bucket") + c.Assert(aws.StringValue(input.Prefix), Equals, "") + c.Assert(aws.StringValue(input.Marker), Equals, "") + c.Assert(aws.Int64Value(input.MaxKeys), Equals, int64(2)) + c.Assert(aws.StringValue(input.Delimiter), Equals, "") + return &s3.ListObjectsOutput{ + IsTruncated: aws.Bool(false), + Contents: contents, + }, nil + }) + + // Ensure we receive the items in order. + i := 0 + err := storage.WalkDir( + ctx, + &WalkOption{SubDir: "", ListCount: 2}, + func(path string, size int64) error { + comment := Commentf("index = %d", i) + c.Assert(path, Equals, *contents[i].Key, comment) + c.Assert(size, Equals, *contents[i].Size, comment) + i++ + return nil + }, + ) + c.Assert(err, IsNil) + c.Assert(i, Equals, len(contents)) +}