From 4d6ec783bf422cc4be700ac0a683c15b3b0b4a22 Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Mon, 28 Oct 2024 16:26:08 -0400 Subject: [PATCH] feat: Implements pagination for ListBuckets --- backend/azure/azure.go | 82 +++++++----- backend/backend.go | 4 +- backend/common.go | 11 +- backend/posix/posix.go | 33 +++-- backend/s3proxy/s3.go | 24 ++-- backend/walk_test.go | 6 +- s3api/controllers/backend_moq_test.go | 34 ++--- s3api/controllers/base.go | 26 +++- s3api/controllers/base_test.go | 6 +- s3err/s3err.go | 6 + s3response/s3response.go | 16 ++- tests/integration/group-tests.go | 6 + tests/integration/tests.go | 186 +++++++++++++++++++++++--- tests/integration/utils.go | 13 +- 14 files changed, 334 insertions(+), 119 deletions(-) diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 465398bb..52bbaa2b 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -149,8 +149,8 @@ func (az *Azure) String() string { func (az *Azure) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, acl []byte) error { meta := map[string]*string{ - string(keyAclCapital): backend.GetStringPtr(encodeBytes(acl)), - string(keyOwnership): backend.GetStringPtr(encodeBytes([]byte(input.ObjectOwnership))), + string(keyAclCapital): backend.GetPtrFromString(encodeBytes(acl)), + string(keyOwnership): backend.GetPtrFromString(encodeBytes([]byte(input.ObjectOwnership))), } acct, ok := ctx.Value("account").(auth.Account) @@ -170,7 +170,7 @@ func (az *Azure) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, return fmt.Errorf("parse default bucket lock state: %w", err) } - meta[string(keyBucketLock)] = backend.GetStringPtr(encodeBytes(defaultLockParsed)) + meta[string(keyBucketLock)] = backend.GetPtrFromString(encodeBytes(defaultLockParsed)) } _, err := az.client.CreateContainer(ctx, *input.Bucket, &container.CreateOptions{Metadata: meta}) @@ -195,48 +195,56 @@ func (az *Azure) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, return azureErrToS3Err(err) } -func (az *Azure) ListBuckets(ctx context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error) { +func (az *Azure) ListBuckets(ctx context.Context, input s3response.ListBucketsInput) (s3response.ListAllMyBucketsResult, error) { + fmt.Printf("%+v\n", input) pager := az.client.NewListContainersPager( &service.ListContainersOptions{ Include: service.ListContainersInclude{ Metadata: true, }, + Marker: &input.ContinuationToken, + MaxResults: &input.MaxBuckets, + Prefix: &input.Prefix, }) var buckets []s3response.ListAllMyBucketsEntry - var result s3response.ListAllMyBucketsResult + result := s3response.ListAllMyBucketsResult{ + Prefix: input.Prefix, + } - for pager.More() { - resp, err := pager.NextPage(ctx) - if err != nil { - return result, azureErrToS3Err(err) - } - for _, v := range resp.ContainerItems { - if isAdmin { + resp, err := pager.NextPage(ctx) + if err != nil { + return result, azureErrToS3Err(err) + } + for _, v := range resp.ContainerItems { + if input.IsAdmin { + buckets = append(buckets, s3response.ListAllMyBucketsEntry{ + Name: *v.Name, + // TODO: using modification date here instead of creation, is that ok? + CreationDate: *v.Properties.LastModified, + }) + } else { + acl, err := getAclFromMetadata(v.Metadata, keyAclLower) + if err != nil { + return result, err + } + + if acl.Owner == input.Owner { buckets = append(buckets, s3response.ListAllMyBucketsEntry{ Name: *v.Name, // TODO: using modification date here instead of creation, is that ok? CreationDate: *v.Properties.LastModified, }) - } else { - acl, err := getAclFromMetadata(v.Metadata, keyAclLower) - if err != nil { - return result, err - } - - if acl.Owner == owner { - buckets = append(buckets, s3response.ListAllMyBucketsEntry{ - Name: *v.Name, - // TODO: using modification date here instead of creation, is that ok? - CreationDate: *v.Properties.LastModified, - }) - } } } } + if resp.NextMarker != nil { + result.ContinuationToken = *resp.NextMarker + } + result.Buckets.Bucket = buckets - result.Owner.ID = owner + result.Owner.ID = input.Owner return result, nil } @@ -303,13 +311,13 @@ func (az *Azure) PutObject(ctx context.Context, po *s3.PutObjectInput) (s3respon opts.HTTPHeaders.BlobContentDisposition = po.ContentDisposition if strings.HasSuffix(*po.Key, "/") { // Hardcode "application/x-directory" for direcoty objects - opts.HTTPHeaders.BlobContentType = backend.GetStringPtr(backend.DirContentType) + opts.HTTPHeaders.BlobContentType = backend.GetPtrFromString(backend.DirContentType) } else { opts.HTTPHeaders.BlobContentType = po.ContentType } if opts.HTTPHeaders.BlobContentType == nil { - opts.HTTPHeaders.BlobContentType = backend.GetStringPtr(backend.DefaultContentType) + opts.HTTPHeaders.BlobContentType = backend.GetPtrFromString(backend.DefaultContentType) } uploadResp, err := az.client.UploadStream(ctx, *po.Bucket, *po.Key, po.Body, opts) @@ -408,7 +416,7 @@ func (az *Azure) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.G contentType := blobDownloadResponse.ContentType if contentType == nil { - contentType = backend.GetStringPtr(backend.DefaultContentType) + contentType = backend.GetPtrFromString(backend.DefaultContentType) } return &s3.GetObjectOutput{ @@ -712,8 +720,8 @@ func (az *Azure) DeleteObjects(ctx context.Context, input *s3.DeleteObjectsInput } else { errs = append(errs, types.Error{ Key: obj.Key, - Code: backend.GetStringPtr("InternalError"), - Message: backend.GetStringPtr(err.Error()), + Code: backend.GetPtrFromString("InternalError"), + Message: backend.GetPtrFromString(err.Error()), }) } } @@ -849,7 +857,7 @@ func (az *Azure) CreateMultipartUpload(ctx context.Context, input *s3.CreateMult // set blob legal hold status in metadata if input.ObjectLockLegalHoldStatus == types.ObjectLockLegalHoldStatusOn { - meta[string(keyObjLegalHold)] = backend.GetStringPtr("1") + meta[string(keyObjLegalHold)] = backend.GetPtrFromString("1") } // set blob retention date @@ -862,7 +870,7 @@ func (az *Azure) CreateMultipartUpload(ctx context.Context, input *s3.CreateMult if err != nil { return s3response.InitiateMultipartUploadResult{}, azureErrToS3Err(err) } - meta[string(keyObjRetention)] = backend.GetStringPtr(string(retParsed)) + meta[string(keyObjRetention)] = backend.GetPtrFromString(string(retParsed)) } uploadId := uuid.New().String() @@ -1318,12 +1326,12 @@ func (az *Azure) PutObjectRetention(ctx context.Context, bucket, object, version meta := blobProps.Metadata if meta == nil { meta = map[string]*string{ - string(keyObjRetention): backend.GetStringPtr(string(retention)), + string(keyObjRetention): backend.GetPtrFromString(string(retention)), } } else { objLockCfg, ok := meta[string(keyObjRetention)] if !ok { - meta[string(keyObjRetention)] = backend.GetStringPtr(string(retention)) + meta[string(keyObjRetention)] = backend.GetPtrFromString(string(retention)) } else { var lockCfg types.ObjectLockRetention if err := json.Unmarshal([]byte(*objLockCfg), &lockCfg); err != nil { @@ -1341,7 +1349,7 @@ func (az *Azure) PutObjectRetention(ctx context.Context, bucket, object, version } } - meta[string(keyObjRetention)] = backend.GetStringPtr(string(retention)) + meta[string(keyObjRetention)] = backend.GetPtrFromString(string(retention)) } } @@ -1689,7 +1697,7 @@ func (az *Azure) setContainerMetaData(ctx context.Context, bucket, key string, v } str := encodeBytes(value) - mdmap[key] = backend.GetStringPtr(str) + mdmap[key] = backend.GetPtrFromString(str) _, err = client.SetMetadata(ctx, &container.SetMetadataOptions{Metadata: mdmap}) if err != nil { diff --git a/backend/backend.go b/backend/backend.go index 7b463672..6a0ad737 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -32,7 +32,7 @@ type Backend interface { Shutdown() // bucket operations - ListBuckets(_ context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error) + ListBuckets(context.Context, s3response.ListBucketsInput) (s3response.ListAllMyBucketsResult, error) HeadBucket(context.Context, *s3.HeadBucketInput) (*s3.HeadBucketOutput, error) GetBucketAcl(context.Context, *s3.GetBucketAclInput) ([]byte, error) CreateBucket(_ context.Context, _ *s3.CreateBucketInput, defaultACL []byte) error @@ -108,7 +108,7 @@ func (BackendUnsupported) Shutdown() {} func (BackendUnsupported) String() string { return "Unsupported" } -func (BackendUnsupported) ListBuckets(context.Context, string, bool) (s3response.ListAllMyBucketsResult, error) { +func (BackendUnsupported) ListBuckets(context.Context, s3response.ListBucketsInput) (s3response.ListAllMyBucketsResult, error) { return s3response.ListAllMyBucketsResult{}, s3err.GetAPIError(s3err.ErrNotImplemented) } func (BackendUnsupported) HeadBucket(context.Context, *s3.HeadBucketInput) (*s3.HeadBucketOutput, error) { diff --git a/backend/common.go b/backend/common.go index a3a54a9b..60a144c9 100644 --- a/backend/common.go +++ b/backend/common.go @@ -50,10 +50,6 @@ func (d ByObjectName) Len() int { return len(d) } func (d ByObjectName) Swap(i, j int) { d[i], d[j] = d[j], d[i] } func (d ByObjectName) Less(i, j int) bool { return *d[i].Key < *d[j].Key } -func GetStringPtr(s string) *string { - return &s -} - func GetPtrFromString(str string) *string { if str == "" { return nil @@ -61,6 +57,13 @@ func GetPtrFromString(str string) *string { return &str } +func GetStringFromPtr(str *string) string { + if str == nil { + return "" + } + return *str +} + func GetTimePtr(t time.Time) *time.Time { return &t } diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 2195ff1f..78fe2d49 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -208,13 +208,15 @@ func (p *Posix) doesBucketAndObjectExist(bucket, object string) error { return nil } -func (p *Posix) ListBuckets(_ context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error) { +func (p *Posix) ListBuckets(_ context.Context, input s3response.ListBucketsInput) (s3response.ListAllMyBucketsResult, error) { entries, err := os.ReadDir(".") if err != nil { return s3response.ListAllMyBucketsResult{}, fmt.Errorf("readdir buckets: %w", err) } + var cToken string + var buckets []s3response.ListAllMyBucketsEntry for _, entry := range entries { fi, err := entry.Info() @@ -236,8 +238,21 @@ func (p *Posix) ListBuckets(_ context.Context, owner string, isAdmin bool) (s3re continue } + if !strings.HasPrefix(fi.Name(), input.Prefix) { + continue + } + + if len(buckets) == int(input.MaxBuckets) { + cToken = buckets[len(buckets)-1].Name + break + } + + if fi.Name() <= input.ContinuationToken { + continue + } + // return all the buckets for admin users - if isAdmin { + if input.IsAdmin { buckets = append(buckets, s3response.ListAllMyBucketsEntry{ Name: entry.Name(), CreationDate: fi.ModTime(), @@ -260,7 +275,7 @@ func (p *Posix) ListBuckets(_ context.Context, owner string, isAdmin bool) (s3re return s3response.ListAllMyBucketsResult{}, fmt.Errorf("parse acl tag: %w", err) } - if acl.Owner == owner { + if acl.Owner == input.Owner { buckets = append(buckets, s3response.ListAllMyBucketsEntry{ Name: entry.Name(), CreationDate: fi.ModTime(), @@ -268,15 +283,15 @@ func (p *Posix) ListBuckets(_ context.Context, owner string, isAdmin bool) (s3re } } - sort.Sort(backend.ByBucketName(buckets)) - return s3response.ListAllMyBucketsResult{ Buckets: s3response.ListAllMyBucketsList{ Bucket: buckets, }, Owner: s3response.CanonicalUser{ - ID: owner, + ID: input.Owner, }, + Prefix: input.Prefix, + ContinuationToken: cToken, }, nil } @@ -926,7 +941,7 @@ func (p *Posix) fileToObjVersions(bucket string) backend.GetVersionsFunc { // Check to see if the null versionId object is delete marker or not if isDel { nullObjDelMarker = &types.DeleteMarkerEntry{ - VersionId: backend.GetStringPtr("null"), + VersionId: backend.GetPtrFromString("null"), LastModified: backend.GetTimePtr(nf.ModTime()), Key: &path, IsLatest: getBoolPtr(false), @@ -948,7 +963,7 @@ func (p *Posix) fileToObjVersions(bucket string) backend.GetVersionsFunc { Key: &path, LastModified: backend.GetTimePtr(nf.ModTime()), Size: &size, - VersionId: backend.GetStringPtr("null"), + VersionId: backend.GetPtrFromString("null"), IsLatest: getBoolPtr(false), StorageClass: types.ObjectVersionStorageClassStandard, } @@ -3250,7 +3265,7 @@ func (p *Posix) CopyObject(ctx context.Context, input *s3.CopyObjectInput) (*s3. if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } - version = backend.GetStringPtr(string(vId)) + version = backend.GetPtrFromString(string(vId)) } else { contentLength := fi.Size() res, err := p.PutObject(ctx, diff --git a/backend/s3proxy/s3.go b/backend/s3proxy/s3.go index fea085d3..9f976119 100644 --- a/backend/s3proxy/s3.go +++ b/backend/s3proxy/s3.go @@ -75,8 +75,12 @@ func New(access, secret, endpoint, region string, disableChecksum, sslSkipVerify return s, nil } -func (s *S3Proxy) ListBuckets(ctx context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error) { - output, err := s.client.ListBuckets(ctx, &s3.ListBucketsInput{}) +func (s *S3Proxy) ListBuckets(ctx context.Context, input s3response.ListBucketsInput) (s3response.ListAllMyBucketsResult, error) { + output, err := s.client.ListBuckets(ctx, &s3.ListBucketsInput{ + ContinuationToken: &input.ContinuationToken, + MaxBuckets: &input.MaxBuckets, + Prefix: &input.Prefix, + }) if err != nil { return s3response.ListAllMyBucketsResult{}, handleError(err) } @@ -96,6 +100,8 @@ func (s *S3Proxy) ListBuckets(ctx context.Context, owner string, isAdmin bool) ( Buckets: s3response.ListAllMyBucketsList{ Bucket: buckets, }, + ContinuationToken: backend.GetStringFromPtr(output.ContinuationToken), + Prefix: backend.GetStringFromPtr(output.Prefix), }, nil } @@ -112,8 +118,8 @@ func (s *S3Proxy) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, var tagSet []types.Tag tagSet = append(tagSet, types.Tag{ - Key: backend.GetStringPtr(aclKey), - Value: backend.GetStringPtr(base64Encode(acl)), + Key: backend.GetPtrFromString(aclKey), + Value: backend.GetPtrFromString(base64Encode(acl)), }) _, err = s.client.PutBucketTagging(ctx, &s3.PutBucketTaggingInput{ @@ -525,8 +531,8 @@ func (s *S3Proxy) PutBucketAcl(ctx context.Context, bucket string, data []byte) for i, tag := range tagout.TagSet { if *tag.Key == aclKey { tagout.TagSet[i] = types.Tag{ - Key: backend.GetStringPtr(aclKey), - Value: backend.GetStringPtr(base64Encode(data)), + Key: backend.GetPtrFromString(aclKey), + Value: backend.GetPtrFromString(base64Encode(data)), } found = true break @@ -534,8 +540,8 @@ func (s *S3Proxy) PutBucketAcl(ctx context.Context, bucket string, data []byte) } if !found { tagout.TagSet = append(tagout.TagSet, types.Tag{ - Key: backend.GetStringPtr(aclKey), - Value: backend.GetStringPtr(base64Encode(data)), + Key: backend.GetPtrFromString(aclKey), + Value: backend.GetPtrFromString(base64Encode(data)), }) } @@ -595,7 +601,7 @@ func (s *S3Proxy) DeleteObjectTagging(ctx context.Context, bucket, object string func (s *S3Proxy) PutBucketPolicy(ctx context.Context, bucket string, policy []byte) error { _, err := s.client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ Bucket: &bucket, - Policy: backend.GetStringPtr(string(policy)), + Policy: backend.GetPtrFromString(string(policy)), }) return handleError(err) } diff --git a/backend/walk_test.go b/backend/walk_test.go index e07061c7..a5aa5ac9 100644 --- a/backend/walk_test.go +++ b/backend/walk_test.go @@ -90,10 +90,10 @@ func TestWalk(t *testing.T) { }, expected: backend.WalkResults{ CommonPrefixes: []types.CommonPrefix{{ - Prefix: backend.GetStringPtr("photos/"), + Prefix: backend.GetPtrFromString("photos/"), }}, Objects: []s3response.Object{{ - Key: backend.GetStringPtr("sample.jpg"), + Key: backend.GetPtrFromString("sample.jpg"), }}, }, getobj: getObj, @@ -105,7 +105,7 @@ func TestWalk(t *testing.T) { }, expected: backend.WalkResults{ CommonPrefixes: []types.CommonPrefix{{ - Prefix: backend.GetStringPtr("test/"), + Prefix: backend.GetPtrFromString("test/"), }}, Objects: []s3response.Object{}, }, diff --git a/s3api/controllers/backend_moq_test.go b/s3api/controllers/backend_moq_test.go index febeec6e..d9e76e8a 100644 --- a/s3api/controllers/backend_moq_test.go +++ b/s3api/controllers/backend_moq_test.go @@ -104,7 +104,7 @@ var _ backend.Backend = &BackendMock{} // HeadObjectFunc: func(contextMoqParam context.Context, headObjectInput *s3.HeadObjectInput) (*s3.HeadObjectOutput, error) { // panic("mock out the HeadObject method") // }, -// ListBucketsFunc: func(contextMoqParam context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error) { +// ListBucketsFunc: func(contextMoqParam context.Context, listBucketsInput s3response.ListBucketsInput) (s3response.ListAllMyBucketsResult, error) { // panic("mock out the ListBuckets method") // }, // ListBucketsAndOwnersFunc: func(contextMoqParam context.Context) ([]s3response.Bucket, error) { @@ -265,7 +265,7 @@ type BackendMock struct { HeadObjectFunc func(contextMoqParam context.Context, headObjectInput *s3.HeadObjectInput) (*s3.HeadObjectOutput, error) // ListBucketsFunc mocks the ListBuckets method. - ListBucketsFunc func(contextMoqParam context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error) + ListBucketsFunc func(contextMoqParam context.Context, listBucketsInput s3response.ListBucketsInput) (s3response.ListAllMyBucketsResult, error) // ListBucketsAndOwnersFunc mocks the ListBucketsAndOwners method. ListBucketsAndOwnersFunc func(contextMoqParam context.Context) ([]s3response.Bucket, error) @@ -547,10 +547,8 @@ type BackendMock struct { ListBuckets []struct { // ContextMoqParam is the contextMoqParam argument value. ContextMoqParam context.Context - // Owner is the owner argument value. - Owner string - // IsAdmin is the isAdmin argument value. - IsAdmin bool + // ListBucketsInput is the listBucketsInput argument value. + ListBucketsInput s3response.ListBucketsInput } // ListBucketsAndOwners holds details about calls to the ListBucketsAndOwners method. ListBucketsAndOwners []struct { @@ -1792,23 +1790,21 @@ func (mock *BackendMock) HeadObjectCalls() []struct { } // ListBuckets calls ListBucketsFunc. -func (mock *BackendMock) ListBuckets(contextMoqParam context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error) { +func (mock *BackendMock) ListBuckets(contextMoqParam context.Context, listBucketsInput s3response.ListBucketsInput) (s3response.ListAllMyBucketsResult, error) { if mock.ListBucketsFunc == nil { panic("BackendMock.ListBucketsFunc: method is nil but Backend.ListBuckets was just called") } callInfo := struct { - ContextMoqParam context.Context - Owner string - IsAdmin bool + ContextMoqParam context.Context + ListBucketsInput s3response.ListBucketsInput }{ - ContextMoqParam: contextMoqParam, - Owner: owner, - IsAdmin: isAdmin, + ContextMoqParam: contextMoqParam, + ListBucketsInput: listBucketsInput, } mock.lockListBuckets.Lock() mock.calls.ListBuckets = append(mock.calls.ListBuckets, callInfo) mock.lockListBuckets.Unlock() - return mock.ListBucketsFunc(contextMoqParam, owner, isAdmin) + return mock.ListBucketsFunc(contextMoqParam, listBucketsInput) } // ListBucketsCalls gets all the calls that were made to ListBuckets. @@ -1816,14 +1812,12 @@ func (mock *BackendMock) ListBuckets(contextMoqParam context.Context, owner stri // // len(mockedBackend.ListBucketsCalls()) func (mock *BackendMock) ListBucketsCalls() []struct { - ContextMoqParam context.Context - Owner string - IsAdmin bool + ContextMoqParam context.Context + ListBucketsInput s3response.ListBucketsInput } { var calls []struct { - ContextMoqParam context.Context - Owner string - IsAdmin bool + ContextMoqParam context.Context + ListBucketsInput s3response.ListBucketsInput } mock.lockListBuckets.RLock() calls = mock.calls.ListBuckets diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 89139cc8..46381a30 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -68,8 +68,32 @@ func New(be backend.Backend, iam auth.IAMService, logger s3log.AuditLogger, evs } func (c S3ApiController) ListBuckets(ctx *fiber.Ctx) error { + cToken := ctx.Query("continuation-token") + prefix := ctx.Query("prefix") + maxBucketsStr := ctx.Query("max-buckets") acct := ctx.Locals("account").(auth.Account) - res, err := c.be.ListBuckets(ctx.Context(), acct.Access, acct.Role == "admin") + + maxBuckets, err := utils.ParseUint(maxBucketsStr) + if err != nil || maxBuckets > 10000 { + if c.debug { + log.Printf("error parsing max-buckets %q: %v\n", maxBucketsStr, err) + } + return SendXMLResponse(ctx, nil, s3err.GetAPIError(s3err.ErrInvalidMaxBuckets), + &MetaOpts{ + Logger: c.logger, + MetricsMng: c.mm, + Action: metrics.ActionListAllMyBuckets, + }) + } + + res, err := c.be.ListBuckets(ctx.Context(), + s3response.ListBucketsInput{ + Owner: acct.Access, + IsAdmin: acct.Role == auth.RoleAdmin, + MaxBuckets: maxBuckets, + ContinuationToken: cToken, + Prefix: prefix, + }) return SendXMLResponse(ctx, res, err, &MetaOpts{ Logger: c.logger, diff --git a/s3api/controllers/base_test.go b/s3api/controllers/base_test.go index 2964d76e..8a17d2d7 100644 --- a/s3api/controllers/base_test.go +++ b/s3api/controllers/base_test.go @@ -92,7 +92,7 @@ func TestS3ApiController_ListBuckets(t *testing.T) { app := fiber.New() s3ApiController := S3ApiController{ be: &BackendMock{ - ListBucketsFunc: func(context.Context, string, bool) (s3response.ListAllMyBucketsResult, error) { + ListBucketsFunc: func(contextMoqParam context.Context, listBucketsInput s3response.ListBucketsInput) (s3response.ListAllMyBucketsResult, error) { return s3response.ListAllMyBucketsResult{}, nil }, }, @@ -109,7 +109,7 @@ func TestS3ApiController_ListBuckets(t *testing.T) { appErr := fiber.New() s3ApiControllerErr := S3ApiController{ be: &BackendMock{ - ListBucketsFunc: func(context.Context, string, bool) (s3response.ListAllMyBucketsResult, error) { + ListBucketsFunc: func(contextMoqParam context.Context, listBucketsInput s3response.ListBucketsInput) (s3response.ListAllMyBucketsResult, error) { return s3response.ListAllMyBucketsResult{}, s3err.GetAPIError(s3err.ErrMethodNotAllowed) }, }, @@ -1628,7 +1628,7 @@ func TestS3ApiController_HeadObject(t *testing.T) { return acldata, nil }, HeadObjectFunc: func(context.Context, *s3.HeadObjectInput) (*s3.HeadObjectOutput, error) { - return nil, s3err.GetAPIError(42) + return nil, s3err.GetAPIError(s3err.ErrInvalidRequest) }, }, } diff --git a/s3err/s3err.go b/s3err/s3err.go index 8c62ea2e..8bea6b48 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -66,6 +66,7 @@ const ( ErrInvalidBucketName ErrInvalidDigest ErrInvalidMaxKeys + ErrInvalidMaxBuckets ErrInvalidMaxUploads ErrInvalidMaxParts ErrInvalidPartNumberMarker @@ -193,6 +194,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "The Content-Md5 you specified is not valid.", HTTPStatusCode: http.StatusBadRequest, }, + ErrInvalidMaxBuckets: { + Code: "InvalidArgument", + Description: "Argument max-buckets must be an integer between 1 and 10000.", + HTTPStatusCode: http.StatusBadRequest, + }, ErrInvalidMaxUploads: { Code: "InvalidArgument", Description: "Argument max-uploads must be an integer between 0 and 2147483647.", diff --git a/s3response/s3response.go b/s3response/s3response.go index c84ddb1d..4880ae82 100644 --- a/s3response/s3response.go +++ b/s3response/s3response.go @@ -277,10 +277,20 @@ type Bucket struct { Owner string `json:"owner"` } +type ListBucketsInput struct { + Owner string + IsAdmin bool + ContinuationToken string + Prefix string + MaxBuckets int32 +} + type ListAllMyBucketsResult struct { - XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ ListAllMyBucketsResult" json:"-"` - Owner CanonicalUser - Buckets ListAllMyBucketsList + XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ ListAllMyBucketsResult" json:"-"` + Owner CanonicalUser + Buckets ListAllMyBucketsList + ContinuationToken string `xml:"ContinuationToken,omitempty"` + Prefix string `xml:"Prefix,omitempty"` } type ListAllMyBucketsEntry struct { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 8c5a083a..5331567b 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -82,6 +82,9 @@ func TestHeadBucket(s *S3Conf) { func TestListBuckets(s *S3Conf) { ListBuckets_as_user(s) ListBuckets_as_admin(s) + ListBuckets_with_prefix(s) + ListBuckets_invalid_max_buckets(s) + ListBuckets_truncated(s) ListBuckets_success(s) } @@ -676,6 +679,9 @@ func GetIntTests() IntTests { "HeadBucket_success": HeadBucket_success, "ListBuckets_as_user": ListBuckets_as_user, "ListBuckets_as_admin": ListBuckets_as_admin, + "ListBuckets_with_prefix": ListBuckets_with_prefix, + "ListBuckets_invalid_max_buckets": ListBuckets_invalid_max_buckets, + "ListBuckets_truncated": ListBuckets_truncated, "ListBuckets_success": ListBuckets_success, "DeleteBucket_non_existing_bucket": DeleteBucket_non_existing_bucket, "DeleteBucket_non_empty_bucket": DeleteBucket_non_empty_bucket, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 7f293ff0..c8413ad0 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -35,7 +35,6 @@ import ( "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/versity/versitygw/backend" "github.com/versity/versitygw/s3err" - "github.com/versity/versitygw/s3response" "golang.org/x/sync/errgroup" ) @@ -1967,7 +1966,7 @@ func HeadBucket_success(s *S3Conf) error { func ListBuckets_as_user(s *S3Conf) error { testName := "ListBuckets_as_user" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - buckets := []s3response.ListAllMyBucketsEntry{{Name: bucket}} + buckets := []types.Bucket{{Name: &bucket}} for i := 0; i < 6; i++ { bckt := getBucketName() @@ -1976,8 +1975,8 @@ func ListBuckets_as_user(s *S3Conf) error { return err } - buckets = append(buckets, s3response.ListAllMyBucketsEntry{ - Name: bckt, + buckets = append(buckets, types.Bucket{ + Name: &bckt, }) } usr := user{ @@ -1997,7 +1996,7 @@ func ListBuckets_as_user(s *S3Conf) error { bckts := []string{} for i := 0; i < 3; i++ { - bckts = append(bckts, buckets[i].Name) + bckts = append(bckts, *buckets[i].Name) } err = changeBucketsOwner(s, bckts, usr.access) @@ -2017,12 +2016,12 @@ func ListBuckets_as_user(s *S3Conf) error { if *out.Owner.ID != usr.access { return fmt.Errorf("expected buckets owner to be %v, instead got %v", usr.access, *out.Owner.ID) } - if ok := compareBuckets(out.Buckets, buckets[:3]); !ok { + if !compareBuckets(out.Buckets, buckets[:3]) { return fmt.Errorf("expected list buckets result to be %v, instead got %v", buckets[:3], out.Buckets) } for _, elem := range buckets[1:] { - err = teardown(s, elem.Name) + err = teardown(s, *elem.Name) if err != nil { return err } @@ -2035,7 +2034,7 @@ func ListBuckets_as_user(s *S3Conf) error { func ListBuckets_as_admin(s *S3Conf) error { testName := "ListBuckets_as_admin" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - buckets := []s3response.ListAllMyBucketsEntry{{Name: bucket}} + buckets := []types.Bucket{{Name: &bucket}} for i := 0; i < 6; i++ { bckt := getBucketName() @@ -2044,8 +2043,8 @@ func ListBuckets_as_admin(s *S3Conf) error { return err } - buckets = append(buckets, s3response.ListAllMyBucketsEntry{ - Name: bckt, + buckets = append(buckets, types.Bucket{ + Name: &bckt, }) } usr := user{ @@ -2070,7 +2069,7 @@ func ListBuckets_as_admin(s *S3Conf) error { bckts := []string{} for i := 0; i < 3; i++ { - bckts = append(bckts, buckets[i].Name) + bckts = append(bckts, *buckets[i].Name) } err = changeBucketsOwner(s, bckts, usr.access) @@ -2090,12 +2089,163 @@ func ListBuckets_as_admin(s *S3Conf) error { if *out.Owner.ID != admin.access { return fmt.Errorf("expected buckets owner to be %v, instead got %v", admin.access, *out.Owner.ID) } - if ok := compareBuckets(out.Buckets, buckets); !ok { + if !compareBuckets(out.Buckets, buckets) { return fmt.Errorf("expected list buckets result to be %v, instead got %v", buckets, out.Buckets) } for _, elem := range buckets[1:] { - err = teardown(s, elem.Name) + err = teardown(s, *elem.Name) + if err != nil { + return err + } + } + + return nil + }) +} + +func ListBuckets_with_prefix(s *S3Conf) error { + testName := "ListBuckets_with_prefix" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + prefix := "my-prefix-" + allBuckets, prefixedBuckets := []types.Bucket{{Name: &bucket}}, []types.Bucket{} + for i := 0; i < 5; i++ { + bckt := getBucketName() + if i%2 == 0 { + bckt = prefix + bckt + } + + err := setup(s, bckt) + if err != nil { + return err + } + + allBuckets = append(allBuckets, types.Bucket{ + Name: &bckt, + }) + + if i%2 == 0 { + prefixedBuckets = append(prefixedBuckets, types.Bucket{ + Name: &bckt, + }) + } + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.ListBuckets(ctx, &s3.ListBucketsInput{ + Prefix: &prefix, + }) + cancel() + if err != nil { + return err + } + + if *out.Owner.ID != s.awsID { + return fmt.Errorf("expected owner to be %v, instead got %v", s.awsID, *out.Owner.ID) + } + if getString(out.Prefix) != prefix { + return fmt.Errorf("expected prefix to be %v, instead got %v", prefix, getString(out.Prefix)) + } + if !compareBuckets(out.Buckets, prefixedBuckets) { + return fmt.Errorf("expected list buckets result to be %v, instead got %v", prefixedBuckets, out.Buckets) + } + + for _, elem := range allBuckets[1:] { + err = teardown(s, *elem.Name) + if err != nil { + return err + } + } + + return nil + }) +} +func ListBuckets_invalid_max_buckets(s *S3Conf) error { + testName := "ListBuckets_invalid_max_buckets" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + listBuckets := func(maxBuckets int32) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.ListBuckets(ctx, &s3.ListBucketsInput{ + MaxBuckets: &maxBuckets, + }) + cancel() + return err + } + + invMaxBuckets := int32(-3) + err := listBuckets(invMaxBuckets) + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidMaxBuckets)); err != nil { + return err + } + + invMaxBuckets = 2000000 + err = listBuckets(invMaxBuckets) + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidMaxBuckets)); err != nil { + return err + } + + return nil + }) +} + +func ListBuckets_truncated(s *S3Conf) error { + testName := "ListBuckets_truncated" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + buckets := []types.Bucket{{Name: &bucket}} + for i := 0; i < 5; i++ { + bckt := getBucketName() + + err := setup(s, bckt) + if err != nil { + return err + } + + buckets = append(buckets, types.Bucket{ + Name: &bckt, + }) + } + + maxBuckets := int32(3) + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.ListBuckets(ctx, &s3.ListBucketsInput{ + MaxBuckets: &maxBuckets, + }) + cancel() + if err != nil { + return err + } + + if *out.Owner.ID != s.awsID { + return fmt.Errorf("expected owner to be %v, instead got %v", s.awsID, *out.Owner.ID) + } + if !compareBuckets(out.Buckets, buckets[:maxBuckets]) { + return fmt.Errorf("expected list buckets result to be %v, instead got %v", buckets[:maxBuckets], out.Buckets) + } + if getString(out.ContinuationToken) != *buckets[maxBuckets-1].Name { + return fmt.Errorf("expected ContinuationToken to be %v, instead got %v", *buckets[maxBuckets-1].Name, getString(out.ContinuationToken)) + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err = s3client.ListBuckets(ctx, &s3.ListBucketsInput{ + ContinuationToken: out.ContinuationToken, + }) + cancel() + if err != nil { + return err + } + + if !compareBuckets(out.Buckets, buckets[maxBuckets:]) { + return fmt.Errorf("expected list buckets result to be %v, instead got %v", buckets[maxBuckets:], out.Buckets) + } + if out.ContinuationToken != nil { + return fmt.Errorf("expected nil continuation token, instead got %v", *out.ContinuationToken) + } + if out.Prefix != nil { + return fmt.Errorf("expected nil prefix, instead got %v", *out.Prefix) + } + + for _, elem := range buckets[1:] { + err = teardown(s, *elem.Name) if err != nil { return err } @@ -2108,7 +2258,7 @@ func ListBuckets_as_admin(s *S3Conf) error { func ListBuckets_success(s *S3Conf) error { testName := "ListBuckets_success" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { - buckets := []s3response.ListAllMyBucketsEntry{{Name: bucket}} + buckets := []types.Bucket{{Name: &bucket}} for i := 0; i < 5; i++ { bckt := getBucketName() @@ -2117,8 +2267,8 @@ func ListBuckets_success(s *S3Conf) error { return err } - buckets = append(buckets, s3response.ListAllMyBucketsEntry{ - Name: bckt, + buckets = append(buckets, types.Bucket{ + Name: &bckt, }) } @@ -2132,12 +2282,12 @@ func ListBuckets_success(s *S3Conf) error { if *out.Owner.ID != s.awsID { return fmt.Errorf("expected owner to be %v, instead got %v", s.awsID, *out.Owner.ID) } - if ok := compareBuckets(out.Buckets, buckets); !ok { + if !compareBuckets(out.Buckets, buckets) { return fmt.Errorf("expected list buckets result to be %v, instead got %v", buckets, out.Buckets) } for _, elem := range buckets[1:] { - err = teardown(s, elem.Name) + err = teardown(s, *elem.Name) if err != nil { return err } diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 24301105..71c42e2a 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -39,7 +39,6 @@ import ( "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/aws/smithy-go" "github.com/versity/versitygw/s3err" - "github.com/versity/versitygw/s3response" ) var ( @@ -593,19 +592,13 @@ func areMapsSame(mp1, mp2 map[string]string) bool { return true } -func compareBuckets(list1 []types.Bucket, list2 []s3response.ListAllMyBucketsEntry) bool { +func compareBuckets(list1 []types.Bucket, list2 []types.Bucket) bool { if len(list1) != len(list2) { return false } - elementMap := make(map[string]bool) - - for _, elem := range list1 { - elementMap[*elem.Name] = true - } - - for _, elem := range list2 { - if _, found := elementMap[elem.Name]; !found { + for i, elem := range list1 { + if *elem.Name != *list2[i].Name { return false } }