From d70ea61830481f3b6e34991e5effef686915ceda Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Fri, 26 Jan 2024 10:57:31 -0500 Subject: [PATCH 1/3] feat: Added the following actions support in posix backend: PutBucketTagging, GetBucketTagging, DeleteBucketTagging --- backend/backend.go | 15 +++ backend/posix/posix.go | 74 ++++++++++++ s3api/controllers/backend_moq_test.go | 156 ++++++++++++++++++++++++++ s3api/controllers/base.go | 53 +++++++++ s3api/middlewares/acl-parser.go | 2 +- 5 files changed, 299 insertions(+), 1 deletion(-) diff --git a/backend/backend.go b/backend/backend.go index ce397ab7..83534714 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -65,6 +65,11 @@ type Backend interface { RestoreObject(context.Context, *s3.RestoreObjectInput) error SelectObjectContent(ctx context.Context, input *s3.SelectObjectContentInput) func(w *bufio.Writer) + // bucket tagging operations + GetBucketTagging(_ context.Context, bucket string) (map[string]string, error) + PutBucketTagging(_ context.Context, bucket string, tags map[string]string) error + DeleteBucketTagging(_ context.Context, bucket string) error + // object tags operations GetObjectTagging(_ context.Context, bucket, object string) (map[string]string, error) PutObjectTagging(_ context.Context, bucket, object string, tags map[string]string) error @@ -179,6 +184,16 @@ func (BackendUnsupported) SelectObjectContent(ctx context.Context, input *s3.Sel } } +func (BackendUnsupported) GetBucketTagging(_ context.Context, bucket string) (map[string]string, error) { + return nil, s3err.GetAPIError(s3err.ErrNotImplemented) +} +func (BackendUnsupported) PutBucketTagging(_ context.Context, bucket string, tags map[string]string) error { + return s3err.GetAPIError(s3err.ErrNotImplemented) +} +func (BackendUnsupported) DeleteBucketTagging(_ context.Context, bucket string) error { + return s3err.GetAPIError(s3err.ErrNotImplemented) +} + func (BackendUnsupported) GetObjectTagging(_ context.Context, bucket, object string) (map[string]string, error) { return nil, s3err.GetAPIError(s3err.ErrNotImplemented) } diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 784d5872..1584f598 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -60,6 +60,8 @@ const ( contentEncHdr = "content-encoding" emptyMD5 = "d41d8cd98f00b204e9800998ecf8427e" aclkey = "user.acl" + proxyAclKey = "versitygwAcl" + proxyBucketAclKey = "user.proxy.acl" etagkey = "user.etag" ) @@ -1720,6 +1722,78 @@ func (p *Posix) GetBucketAcl(_ context.Context, input *s3.GetBucketAclInput) ([] return b, nil } +func (p *Posix) PutBucketTagging(_ context.Context, bucket string, tags map[string]string) error { + _, err := os.Stat(bucket) + if errors.Is(err, fs.ErrNotExist) { + return s3err.GetAPIError(s3err.ErrNoSuchBucket) + } + if err != nil { + return fmt.Errorf("stat bucket: %w", err) + } + + acl, ok := tags[proxyAclKey] + if ok { + // set proxy acl in a separate xattr key + err = xattr.Set(bucket, proxyBucketAclKey, []byte(acl)) + if err != nil { + return fmt.Errorf("set proxy acl: %w", err) + } + + return nil + } + + if tags == nil { + err = xattr.Remove(bucket, "user."+tagHdr) + if err != nil { + return fmt.Errorf("remove tags: %w", err) + } + return nil + } + + b, err := json.Marshal(tags) + if err != nil { + return fmt.Errorf("marshal tags: %w", err) + } + + err = xattr.Set(bucket, "user."+tagHdr, b) + if err != nil { + return fmt.Errorf("set tags: %w", err) + } + + return nil +} + +func (p *Posix) GetBucketTagging(_ context.Context, bucket string) (map[string]string, error) { + _, err := os.Stat(bucket) + if errors.Is(err, fs.ErrNotExist) { + return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) + } + if err != nil { + return nil, fmt.Errorf("stat bucket: %w", err) + } + + tags, err := p.getXattrTags(bucket, "") + if err != nil { + return nil, err + } + + acl, err := xattr.Get(bucket, "user."+tagHdr) + if isNoAttr(err) { + return tags, nil + } + if err != nil { + return nil, fmt.Errorf("get tags: %w", err) + } + + tags[aclkey] = string(acl) + + return tags, nil +} + +func (p *Posix) DeleteBucketTagging(ctx context.Context, bucket string) error { + return p.PutBucketTagging(ctx, bucket, nil) +} + func (p *Posix) GetObjectTagging(_ context.Context, bucket, object string) (map[string]string, error) { _, err := os.Stat(bucket) if errors.Is(err, fs.ErrNotExist) { diff --git a/s3api/controllers/backend_moq_test.go b/s3api/controllers/backend_moq_test.go index b8a8b263..41cfc154 100644 --- a/s3api/controllers/backend_moq_test.go +++ b/s3api/controllers/backend_moq_test.go @@ -44,6 +44,9 @@ var _ backend.Backend = &BackendMock{} // DeleteBucketFunc: func(contextMoqParam context.Context, deleteBucketInput *s3.DeleteBucketInput) error { // panic("mock out the DeleteBucket method") // }, +// DeleteBucketTaggingFunc: func(contextMoqParam context.Context, bucket string) error { +// panic("mock out the DeleteBucketTagging method") +// }, // DeleteObjectFunc: func(contextMoqParam context.Context, deleteObjectInput *s3.DeleteObjectInput) error { // panic("mock out the DeleteObject method") // }, @@ -56,6 +59,9 @@ var _ backend.Backend = &BackendMock{} // GetBucketAclFunc: func(contextMoqParam context.Context, getBucketAclInput *s3.GetBucketAclInput) ([]byte, error) { // panic("mock out the GetBucketAcl method") // }, +// GetBucketTaggingFunc: func(contextMoqParam context.Context, bucket string) (map[string]string, error) { +// panic("mock out the GetBucketTagging method") +// }, // GetObjectFunc: func(contextMoqParam context.Context, getObjectInput *s3.GetObjectInput, writer io.Writer) (*s3.GetObjectOutput, error) { // panic("mock out the GetObject method") // }, @@ -95,6 +101,9 @@ var _ backend.Backend = &BackendMock{} // PutBucketAclFunc: func(contextMoqParam context.Context, bucket string, data []byte) error { // panic("mock out the PutBucketAcl method") // }, +// PutBucketTaggingFunc: func(contextMoqParam context.Context, bucket string, tags map[string]string) error { +// panic("mock out the PutBucketTagging method") +// }, // PutObjectFunc: func(contextMoqParam context.Context, putObjectInput *s3.PutObjectInput) (string, error) { // panic("mock out the PutObject method") // }, @@ -150,6 +159,9 @@ type BackendMock struct { // DeleteBucketFunc mocks the DeleteBucket method. DeleteBucketFunc func(contextMoqParam context.Context, deleteBucketInput *s3.DeleteBucketInput) error + // DeleteBucketTaggingFunc mocks the DeleteBucketTagging method. + DeleteBucketTaggingFunc func(contextMoqParam context.Context, bucket string) error + // DeleteObjectFunc mocks the DeleteObject method. DeleteObjectFunc func(contextMoqParam context.Context, deleteObjectInput *s3.DeleteObjectInput) error @@ -162,6 +174,9 @@ type BackendMock struct { // GetBucketAclFunc mocks the GetBucketAcl method. GetBucketAclFunc func(contextMoqParam context.Context, getBucketAclInput *s3.GetBucketAclInput) ([]byte, error) + // GetBucketTaggingFunc mocks the GetBucketTagging method. + GetBucketTaggingFunc func(contextMoqParam context.Context, bucket string) (map[string]string, error) + // GetObjectFunc mocks the GetObject method. GetObjectFunc func(contextMoqParam context.Context, getObjectInput *s3.GetObjectInput, writer io.Writer) (*s3.GetObjectOutput, error) @@ -201,6 +216,9 @@ type BackendMock struct { // PutBucketAclFunc mocks the PutBucketAcl method. PutBucketAclFunc func(contextMoqParam context.Context, bucket string, data []byte) error + // PutBucketTaggingFunc mocks the PutBucketTagging method. + PutBucketTaggingFunc func(contextMoqParam context.Context, bucket string, tags map[string]string) error + // PutObjectFunc mocks the PutObject method. PutObjectFunc func(contextMoqParam context.Context, putObjectInput *s3.PutObjectInput) (string, error) @@ -283,6 +301,13 @@ type BackendMock struct { // DeleteBucketInput is the deleteBucketInput argument value. DeleteBucketInput *s3.DeleteBucketInput } + // DeleteBucketTagging holds details about calls to the DeleteBucketTagging method. + DeleteBucketTagging []struct { + // ContextMoqParam is the contextMoqParam argument value. + ContextMoqParam context.Context + // Bucket is the bucket argument value. + Bucket string + } // DeleteObject holds details about calls to the DeleteObject method. DeleteObject []struct { // ContextMoqParam is the contextMoqParam argument value. @@ -313,6 +338,13 @@ type BackendMock struct { // GetBucketAclInput is the getBucketAclInput argument value. GetBucketAclInput *s3.GetBucketAclInput } + // GetBucketTagging holds details about calls to the GetBucketTagging method. + GetBucketTagging []struct { + // ContextMoqParam is the contextMoqParam argument value. + ContextMoqParam context.Context + // Bucket is the bucket argument value. + Bucket string + } // GetObject holds details about calls to the GetObject method. GetObject []struct { // ContextMoqParam is the contextMoqParam argument value. @@ -410,6 +442,15 @@ type BackendMock struct { // Data is the data argument value. Data []byte } + // PutBucketTagging holds details about calls to the PutBucketTagging method. + PutBucketTagging []struct { + // ContextMoqParam is the contextMoqParam argument value. + ContextMoqParam context.Context + // Bucket is the bucket argument value. + Bucket string + // Tags is the tags argument value. + Tags map[string]string + } // PutObject holds details about calls to the PutObject method. PutObject []struct { // ContextMoqParam is the contextMoqParam argument value. @@ -477,10 +518,12 @@ type BackendMock struct { lockCreateBucket sync.RWMutex lockCreateMultipartUpload sync.RWMutex lockDeleteBucket sync.RWMutex + lockDeleteBucketTagging sync.RWMutex lockDeleteObject sync.RWMutex lockDeleteObjectTagging sync.RWMutex lockDeleteObjects sync.RWMutex lockGetBucketAcl sync.RWMutex + lockGetBucketTagging sync.RWMutex lockGetObject sync.RWMutex lockGetObjectAcl sync.RWMutex lockGetObjectAttributes sync.RWMutex @@ -494,6 +537,7 @@ type BackendMock struct { lockListObjectsV2 sync.RWMutex lockListParts sync.RWMutex lockPutBucketAcl sync.RWMutex + lockPutBucketTagging sync.RWMutex lockPutObject sync.RWMutex lockPutObjectAcl sync.RWMutex lockPutObjectTagging sync.RWMutex @@ -765,6 +809,42 @@ func (mock *BackendMock) DeleteBucketCalls() []struct { return calls } +// DeleteBucketTagging calls DeleteBucketTaggingFunc. +func (mock *BackendMock) DeleteBucketTagging(contextMoqParam context.Context, bucket string) error { + if mock.DeleteBucketTaggingFunc == nil { + panic("BackendMock.DeleteBucketTaggingFunc: method is nil but Backend.DeleteBucketTagging was just called") + } + callInfo := struct { + ContextMoqParam context.Context + Bucket string + }{ + ContextMoqParam: contextMoqParam, + Bucket: bucket, + } + mock.lockDeleteBucketTagging.Lock() + mock.calls.DeleteBucketTagging = append(mock.calls.DeleteBucketTagging, callInfo) + mock.lockDeleteBucketTagging.Unlock() + return mock.DeleteBucketTaggingFunc(contextMoqParam, bucket) +} + +// DeleteBucketTaggingCalls gets all the calls that were made to DeleteBucketTagging. +// Check the length with: +// +// len(mockedBackend.DeleteBucketTaggingCalls()) +func (mock *BackendMock) DeleteBucketTaggingCalls() []struct { + ContextMoqParam context.Context + Bucket string +} { + var calls []struct { + ContextMoqParam context.Context + Bucket string + } + mock.lockDeleteBucketTagging.RLock() + calls = mock.calls.DeleteBucketTagging + mock.lockDeleteBucketTagging.RUnlock() + return calls +} + // DeleteObject calls DeleteObjectFunc. func (mock *BackendMock) DeleteObject(contextMoqParam context.Context, deleteObjectInput *s3.DeleteObjectInput) error { if mock.DeleteObjectFunc == nil { @@ -913,6 +993,42 @@ func (mock *BackendMock) GetBucketAclCalls() []struct { return calls } +// GetBucketTagging calls GetBucketTaggingFunc. +func (mock *BackendMock) GetBucketTagging(contextMoqParam context.Context, bucket string) (map[string]string, error) { + if mock.GetBucketTaggingFunc == nil { + panic("BackendMock.GetBucketTaggingFunc: method is nil but Backend.GetBucketTagging was just called") + } + callInfo := struct { + ContextMoqParam context.Context + Bucket string + }{ + ContextMoqParam: contextMoqParam, + Bucket: bucket, + } + mock.lockGetBucketTagging.Lock() + mock.calls.GetBucketTagging = append(mock.calls.GetBucketTagging, callInfo) + mock.lockGetBucketTagging.Unlock() + return mock.GetBucketTaggingFunc(contextMoqParam, bucket) +} + +// GetBucketTaggingCalls gets all the calls that were made to GetBucketTagging. +// Check the length with: +// +// len(mockedBackend.GetBucketTaggingCalls()) +func (mock *BackendMock) GetBucketTaggingCalls() []struct { + ContextMoqParam context.Context + Bucket string +} { + var calls []struct { + ContextMoqParam context.Context + Bucket string + } + mock.lockGetBucketTagging.RLock() + calls = mock.calls.GetBucketTagging + mock.lockGetBucketTagging.RUnlock() + return calls +} + // GetObject calls GetObjectFunc. func (mock *BackendMock) GetObject(contextMoqParam context.Context, getObjectInput *s3.GetObjectInput, writer io.Writer) (*s3.GetObjectOutput, error) { if mock.GetObjectFunc == nil { @@ -1393,6 +1509,46 @@ func (mock *BackendMock) PutBucketAclCalls() []struct { return calls } +// PutBucketTagging calls PutBucketTaggingFunc. +func (mock *BackendMock) PutBucketTagging(contextMoqParam context.Context, bucket string, tags map[string]string) error { + if mock.PutBucketTaggingFunc == nil { + panic("BackendMock.PutBucketTaggingFunc: method is nil but Backend.PutBucketTagging was just called") + } + callInfo := struct { + ContextMoqParam context.Context + Bucket string + Tags map[string]string + }{ + ContextMoqParam: contextMoqParam, + Bucket: bucket, + Tags: tags, + } + mock.lockPutBucketTagging.Lock() + mock.calls.PutBucketTagging = append(mock.calls.PutBucketTagging, callInfo) + mock.lockPutBucketTagging.Unlock() + return mock.PutBucketTaggingFunc(contextMoqParam, bucket, tags) +} + +// PutBucketTaggingCalls gets all the calls that were made to PutBucketTagging. +// Check the length with: +// +// len(mockedBackend.PutBucketTaggingCalls()) +func (mock *BackendMock) PutBucketTaggingCalls() []struct { + ContextMoqParam context.Context + Bucket string + Tags map[string]string +} { + var calls []struct { + ContextMoqParam context.Context + Bucket string + Tags map[string]string + } + mock.lockPutBucketTagging.RLock() + calls = mock.calls.PutBucketTagging + mock.lockPutBucketTagging.RUnlock() + return calls +} + // PutObject calls PutObjectFunc. func (mock *BackendMock) PutObject(contextMoqParam context.Context, putObjectInput *s3.PutObjectInput) (string, error) { if mock.PutObjectFunc == nil { diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 6f64d771..d9ab202d 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -246,6 +246,24 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error { isRoot := ctx.Locals("isRoot").(bool) parsedAcl := ctx.Locals("parsedAcl").(auth.ACL) + if ctx.Request().URI().QueryArgs().Has("tagging") { + if err := auth.VerifyACL(parsedAcl, acct.Access, "READ", isRoot); err != nil { + return SendXMLResponse(ctx, nil, err, &MetaOpts{Logger: c.logger, Action: "GetBucketTagging", BucketOwner: parsedAcl.Owner}) + } + + tags, err := c.be.GetBucketTagging(ctx.Context(), bucket) + if err != nil { + return SendXMLResponse(ctx, nil, err, &MetaOpts{Logger: c.logger, Action: "GetBucketTagging", BucketOwner: parsedAcl.Owner}) + } + resp := s3response.Tagging{TagSet: s3response.TagSet{Tags: []s3response.Tag{}}} + + for key, val := range tags { + resp.TagSet.Tags = append(resp.TagSet.Tags, s3response.Tag{Key: key, Value: val}) + } + + return SendXMLResponse(ctx, resp, nil, &MetaOpts{Logger: c.logger, Action: "GetBucketTagging", BucketOwner: parsedAcl.Owner}) + } + if ctx.Request().URI().QueryArgs().Has("acl") { if err := auth.VerifyACL(parsedAcl, acct.Access, "READ_ACP", isRoot); err != nil { return SendXMLResponse(ctx, nil, err, &MetaOpts{Logger: c.logger, Action: "GetBucketAcl", BucketOwner: parsedAcl.Owner}) @@ -340,6 +358,32 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { ctx.Locals("account").(auth.Account), ctx.Locals("isRoot").(bool) + if ctx.Request().URI().QueryArgs().Has("tagging") { + parsedAcl := ctx.Locals("parsedAcl").(auth.ACL) + + var bucketTagging s3response.Tagging + err := xml.Unmarshal(ctx.Body(), &bucketTagging) + if err != nil { + return SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidRequest), &MetaOpts{Logger: c.logger, Action: "PutBucketTagging", BucketOwner: parsedAcl.Owner}) + } + + tags := make(map[string]string, len(bucketTagging.TagSet.Tags)) + + for _, tag := range bucketTagging.TagSet.Tags { + if len(tag.Key) > 128 || len(tag.Value) > 256 { + return SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidTag), &MetaOpts{Logger: c.logger, Action: "PutBucketTagging", BucketOwner: parsedAcl.Owner}) + } + tags[tag.Key] = tag.Value + } + + if err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot); err != nil { + return SendResponse(ctx, err, &MetaOpts{Logger: c.logger, Action: "PutBucketTagging", BucketOwner: parsedAcl.Owner}) + } + + err = c.be.PutBucketTagging(ctx.Context(), bucket, tags) + return SendResponse(ctx, err, &MetaOpts{Logger: c.logger, Action: "PutBucketTagging", BucketOwner: parsedAcl.Owner}) + } + grants := grantFullControl + grantRead + grantReadACP + granWrite + grantWriteACP if ctx.Request().URI().QueryArgs().Has("acl") { @@ -714,6 +758,15 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error { func (c S3ApiController) DeleteBucket(ctx *fiber.Ctx) error { bucket, acct, isRoot, parsedAcl := ctx.Params("bucket"), ctx.Locals("account").(auth.Account), ctx.Locals("isRoot").(bool), ctx.Locals("parsedAcl").(auth.ACL) + if ctx.Request().URI().QueryArgs().Has("tagging") { + if err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot); err != nil { + return SendResponse(ctx, err, &MetaOpts{Logger: c.logger, Action: "DeleteBucketTagging", BucketOwner: parsedAcl.Owner}) + } + + err := c.be.DeleteBucketTagging(ctx.Context(), bucket) + return SendResponse(ctx, err, &MetaOpts{Logger: c.logger, Action: "DeleteBucketTagging", BucketOwner: parsedAcl.Owner}) + } + if err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot); err != nil { return SendResponse(ctx, err, &MetaOpts{Logger: c.logger, Action: "DeleteBucket", BucketOwner: parsedAcl.Owner}) } diff --git a/s3api/middlewares/acl-parser.go b/s3api/middlewares/acl-parser.go index c6b58d00..f9957083 100644 --- a/s3api/middlewares/acl-parser.go +++ b/s3api/middlewares/acl-parser.go @@ -38,7 +38,7 @@ func AclParser(be backend.Backend, logger s3log.AuditLogger) fiber.Handler { if ctx.Method() == http.MethodPatch { return ctx.Next() } - if len(pathParts) == 2 && pathParts[1] != "" && ctx.Method() == http.MethodPut && !ctx.Request().URI().QueryArgs().Has("acl") { + if len(pathParts) == 2 && pathParts[1] != "" && ctx.Method() == http.MethodPut && !ctx.Request().URI().QueryArgs().Has("acl") && !ctx.Request().URI().QueryArgs().Has("tagging") { if err := auth.IsAdmin(acct, isRoot); err != nil { return controllers.SendXMLResponse(ctx, nil, err, &controllers.MetaOpts{Logger: logger, Action: "CreateBucket"}) } From 3cce3a52019629e6d5c8442045eb06e438a45aeb Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Mon, 29 Jan 2024 14:35:02 -0500 Subject: [PATCH 2/3] feat: Added unit and integration test cases for posix bucket tagging related actions --- backend/posix/posix.go | 4 +- cmd/versitygw/test.go | 8 +- integration/group-tests.go | 28 +++++ integration/tests.go | 209 +++++++++++++++++++++++++++++++++ s3api/controllers/base.go | 4 +- s3api/controllers/base_test.go | 71 ++++++++++- 6 files changed, 314 insertions(+), 10 deletions(-) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 1584f598..cba70957 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1777,7 +1777,7 @@ func (p *Posix) GetBucketTagging(_ context.Context, bucket string) (map[string]s return nil, err } - acl, err := xattr.Get(bucket, "user."+tagHdr) + acl, err := xattr.Get(bucket, proxyBucketAclKey) if isNoAttr(err) { return tags, nil } @@ -1785,7 +1785,7 @@ func (p *Posix) GetBucketTagging(_ context.Context, bucket string) (map[string]s return nil, fmt.Errorf("get tags: %w", err) } - tags[aclkey] = string(acl) + tags[proxyAclKey] = string(acl) return tags, nil } diff --git a/cmd/versitygw/test.go b/cmd/versitygw/test.go index c1c52178..822612c9 100644 --- a/cmd/versitygw/test.go +++ b/cmd/versitygw/test.go @@ -268,9 +268,11 @@ func getAction(tf testFunc) func(*cli.Context) error { func extractIntTests() (commands []*cli.Command) { tests := integration.GetIntTests() for key, val := range tests { + testKey := key + testFunc := val commands = append(commands, &cli.Command{ - Name: key, - Usage: fmt.Sprintf("Runs %v integration test", key), + Name: testKey, + Usage: fmt.Sprintf("Runs %v integration test", testKey), Action: func(ctx *cli.Context) error { opts := []integration.Option{ integration.WithAccess(awsID), @@ -283,7 +285,7 @@ func extractIntTests() (commands []*cli.Command) { } s := integration.NewS3Conf(opts...) - err := val(s) + err := testFunc(s) return err }, }) diff --git a/integration/group-tests.go b/integration/group-tests.go index 24d84881..b63a9044 100644 --- a/integration/group-tests.go +++ b/integration/group-tests.go @@ -48,6 +48,23 @@ func TestDeleteBucket(s *S3Conf) { DeleteBucket_success_status_code(s) } +func TestPutBucketTagging(s *S3Conf) { + PutBucketTagging_non_existing_bucket(s) + PutBucketTagging_long_tags(s) + PutBucketTagging_success(s) +} + +func TestGetBucketTagging(s *S3Conf) { + GetBucketTagging_non_existing_bucket(s) + GetBucketTagging_success(s) +} + +func TestDeleteBucketTagging(s *S3Conf) { + DeleteBucketTagging_non_existing_object(s) + DeleteBucketTagging_success_status(s) + DeleteBucketTagging_success(s) +} + func TestPutObject(s *S3Conf) { PutObject_non_existing_bucket(s) PutObject_special_chars(s) @@ -199,6 +216,9 @@ func TestFullFlow(s *S3Conf) { TestHeadBucket(s) TestListBuckets(s) TestDeleteBucket(s) + TestPutBucketTagging(s) + TestGetBucketTagging(s) + TestDeleteBucketTagging(s) TestPutObject(s) TestHeadObject(s) TestGetObject(s) @@ -263,6 +283,14 @@ func GetIntTests() IntTests { "DeleteBucket_non_existing_bucket": DeleteBucket_non_existing_bucket, "DeleteBucket_non_empty_bucket": DeleteBucket_non_empty_bucket, "DeleteBucket_success_status_code": DeleteBucket_success_status_code, + "PutBucketTagging_non_existing_bucket": PutBucketTagging_non_existing_bucket, + "PutBucketTagging_long_tags": PutBucketTagging_long_tags, + "PutBucketTagging_success": PutBucketTagging_success, + "GetBucketTagging_non_existing_bucket": GetBucketTagging_non_existing_bucket, + "GetBucketTagging_success": GetBucketTagging_success, + "DeleteBucketTagging_non_existing_object": DeleteBucketTagging_non_existing_object, + "DeleteBucketTagging_success_status": DeleteBucketTagging_success_status, + "DeleteBucketTagging_success": DeleteBucketTagging_success, "PutObject_non_existing_bucket": PutObject_non_existing_bucket, "PutObject_special_chars": PutObject_special_chars, "PutObject_invalid_long_tags": PutObject_invalid_long_tags, diff --git a/integration/tests.go b/integration/tests.go index c014a455..1775e9cf 100644 --- a/integration/tests.go +++ b/integration/tests.go @@ -1128,6 +1128,215 @@ func DeleteBucket_success_status_code(s *S3Conf) error { return nil } +func PutBucketTagging_non_existing_bucket(s *S3Conf) error { + testName := "PutBucketTagging_non_existing_bucket" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketTagging(ctx, &s3.PutBucketTaggingInput{ + Bucket: getPtr(getBucketName()), + Tagging: &types.Tagging{TagSet: []types.Tag{}}, + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchBucket)); err != nil { + return err + } + + return nil + }) +} + +func PutBucketTagging_long_tags(s *S3Conf) error { + testName := "PutBucketTagging_long_tags" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + tagging := types.Tagging{TagSet: []types.Tag{{Key: getPtr(genRandString(200)), Value: getPtr("val")}}} + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketTagging(ctx, &s3.PutBucketTaggingInput{ + Bucket: &bucket, + Tagging: &tagging}) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidTag)); err != nil { + return err + } + + tagging = types.Tagging{TagSet: []types.Tag{{Key: getPtr("key"), Value: getPtr(genRandString(300))}}} + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.PutBucketTagging(ctx, &s3.PutBucketTaggingInput{ + Bucket: &bucket, + Tagging: &tagging}) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidTag)); err != nil { + return err + } + + return nil + }) +} + +func PutBucketTagging_success(s *S3Conf) error { + testName := "PutBucketTagging_success" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + tagging := types.Tagging{TagSet: []types.Tag{{Key: getPtr("key1"), Value: getPtr("val2")}, {Key: getPtr("key2"), Value: getPtr("val2")}}} + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketTagging(ctx, &s3.PutBucketTaggingInput{ + Bucket: &bucket, + Tagging: &tagging}) + cancel() + if err != nil { + return err + } + + return nil + }) +} + +func GetBucketTagging_non_existing_bucket(s *S3Conf) error { + testName := "GetBucketTagging_non_existing_object" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.GetBucketTagging(ctx, &s3.GetBucketTaggingInput{ + Bucket: getPtr(getBucketName()), + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchBucket)); err != nil { + return err + } + return nil + }) +} + +func GetBucketTagging_success(s *S3Conf) error { + testName := "GetBucketTagging_success" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + tagging := types.Tagging{TagSet: []types.Tag{{Key: getPtr("key1"), Value: getPtr("val2")}, {Key: getPtr("key2"), Value: getPtr("val2")}}} + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketTagging(ctx, &s3.PutBucketTaggingInput{ + Bucket: &bucket, + Tagging: &tagging}) + cancel() + if err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.GetBucketTagging(ctx, &s3.GetBucketTaggingInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return nil + } + + if !areTagsSame(out.TagSet, tagging.TagSet) { + return fmt.Errorf("expected %v instead got %v", tagging.TagSet, out.TagSet) + } + + return nil + }) +} + +func DeleteBucketTagging_non_existing_object(s *S3Conf) error { + testName := "DeleteBucketTagging_non_existing_object" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.DeleteBucketTagging(ctx, &s3.DeleteBucketTaggingInput{ + Bucket: getPtr(getBucketName()), + }) + cancel() + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchBucket)); err != nil { + return err + } + return nil + }) +} + +func DeleteBucketTagging_success_status(s *S3Conf) error { + testName := "DeleteBucketTagging_success_status" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + tagging := types.Tagging{ + TagSet: []types.Tag{ + { + Key: getPtr("Hello"), + Value: getPtr("World"), + }, + }, + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketTagging(ctx, &s3.PutBucketTaggingInput{ + Bucket: &bucket, + Tagging: &tagging, + }) + cancel() + if err != nil { + return err + } + + req, err := createSignedReq(http.MethodDelete, s.endpoint, fmt.Sprintf("%v?tagging", bucket), s.awsID, s.awsSecret, "s3", s.awsRegion, nil, time.Now()) + if err != nil { + return err + } + + client := http.Client{ + Timeout: shortTimeout, + } + + resp, err := client.Do(req) + if err != nil { + return err + } + + if resp.StatusCode != http.StatusNoContent { + return fmt.Errorf("expected response status to be %v, instead got %v", http.StatusNoContent, resp.StatusCode) + } + + return nil + }) +} + +func DeleteBucketTagging_success(s *S3Conf) error { + testName := "DeleteBucketTagging_success" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + tagging := types.Tagging{TagSet: []types.Tag{{Key: getPtr("key1"), Value: getPtr("val2")}, {Key: getPtr("key2"), Value: getPtr("val2")}}} + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketTagging(ctx, &s3.PutBucketTaggingInput{ + Bucket: &bucket, + Tagging: &tagging}) + cancel() + if err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.DeleteBucketTagging(ctx, &s3.DeleteBucketTaggingInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return nil + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.GetBucketTagging(ctx, &s3.GetBucketTaggingInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return nil + } + + if len(out.TagSet) > 0 { + return fmt.Errorf("expected empty tag set, instead got %v", out.TagSet) + } + + return nil + }) +} + func PutObject_non_existing_bucket(s *S3Conf) error { testName := "PutObject_non_existing_bucket" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index d9ab202d..e988118b 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -764,7 +764,7 @@ func (c S3ApiController) DeleteBucket(ctx *fiber.Ctx) error { } err := c.be.DeleteBucketTagging(ctx.Context(), bucket) - return SendResponse(ctx, err, &MetaOpts{Logger: c.logger, Action: "DeleteBucketTagging", BucketOwner: parsedAcl.Owner}) + return SendResponse(ctx, err, &MetaOpts{Logger: c.logger, Action: "DeleteBucketTagging", BucketOwner: parsedAcl.Owner, Status: http.StatusNoContent}) } if err := auth.VerifyACL(parsedAcl, acct.Access, "WRITE", isRoot); err != nil { @@ -774,7 +774,7 @@ func (c S3ApiController) DeleteBucket(ctx *fiber.Ctx) error { err := c.be.DeleteBucket(ctx.Context(), &s3.DeleteBucketInput{ Bucket: &bucket, }) - return SendResponse(ctx, err, &MetaOpts{Logger: c.logger, Action: "DeleteBucket", BucketOwner: parsedAcl.Owner, Status: 204}) + return SendResponse(ctx, err, &MetaOpts{Logger: c.logger, Action: "DeleteBucket", BucketOwner: parsedAcl.Owner, Status: http.StatusNoContent}) } func (c S3ApiController) DeleteObjects(ctx *fiber.Ctx) error { diff --git a/s3api/controllers/base_test.go b/s3api/controllers/base_test.go index 8facf76f..e694afd2 100644 --- a/s3api/controllers/base_test.go +++ b/s3api/controllers/base_test.go @@ -343,6 +343,9 @@ func TestS3ApiController_ListActions(t *testing.T) { ListObjectsFunc: func(context.Context, *s3.ListObjectsInput) (*s3.ListObjectsOutput, error) { return &s3.ListObjectsOutput{}, nil }, + GetBucketTaggingFunc: func(contextMoqParam context.Context, bucket string) (map[string]string, error) { + return map[string]string{}, nil + }, }, } @@ -365,6 +368,9 @@ func TestS3ApiController_ListActions(t *testing.T) { ListObjectsFunc: func(context.Context, *s3.ListObjectsInput) (*s3.ListObjectsOutput, error) { return nil, s3err.GetAPIError(s3err.ErrNotImplemented) }, + GetBucketTaggingFunc: func(contextMoqParam context.Context, bucket string) (map[string]string, error) { + return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket) + }, }, } appError := fiber.New() @@ -384,6 +390,24 @@ func TestS3ApiController_ListActions(t *testing.T) { wantErr bool statusCode int }{ + { + name: "Get-bucket-tagging-non-existing-bucket", + app: appError, + args: args{ + req: httptest.NewRequest(http.MethodGet, "/my-bucket?tagging", nil), + }, + wantErr: false, + statusCode: 404, + }, + { + name: "Get-bucket-tagging-success", + app: app, + args: args{ + req: httptest.NewRequest(http.MethodGet, "/my-bucket?tagging", nil), + }, + wantErr: false, + statusCode: 200, + }, { name: "Get-bucket-acl-success", app: app, @@ -492,6 +516,17 @@ func TestS3ApiController_PutBucketActions(t *testing.T) { ` + tagBody := ` + + + + organization + marketing + + + + ` + s3ApiController := S3ApiController{ be: &BackendMock{ GetBucketAclFunc: func(context.Context, *s3.GetBucketAclInput) ([]byte, error) { @@ -503,6 +538,9 @@ func TestS3ApiController_PutBucketActions(t *testing.T) { CreateBucketFunc: func(context.Context, *s3.CreateBucketInput, []byte) error { return nil }, + PutBucketTaggingFunc: func(contextMoqParam context.Context, bucket string, tags map[string]string) error { + return nil + }, }, } // Mock ctx.Locals @@ -543,6 +581,24 @@ func TestS3ApiController_PutBucketActions(t *testing.T) { wantErr bool statusCode int }{ + { + name: "Put-bucket-tagging-invalid-body", + app: app, + args: args{ + req: httptest.NewRequest(http.MethodPut, "/my-bucket?tagging", nil), + }, + wantErr: false, + statusCode: 400, + }, + { + name: "Put-bucket-tagging-success", + app: app, + args: args{ + req: httptest.NewRequest(http.MethodPut, "/my-bucket?tagging", strings.NewReader(tagBody)), + }, + wantErr: false, + statusCode: 200, + }, { name: "Put-bucket-acl-invalid-acl", app: app, @@ -869,12 +925,12 @@ func TestS3ApiController_DeleteBucket(t *testing.T) { app := fiber.New() s3ApiController := S3ApiController{ be: &BackendMock{ - GetBucketAclFunc: func(context.Context, *s3.GetBucketAclInput) ([]byte, error) { - return acldata, nil - }, DeleteBucketFunc: func(context.Context, *s3.DeleteBucketInput) error { return nil }, + DeleteBucketTaggingFunc: func(contextMoqParam context.Context, bucket string) error { + return nil + }, }, } @@ -904,6 +960,15 @@ func TestS3ApiController_DeleteBucket(t *testing.T) { wantErr: false, statusCode: 204, }, + { + name: "Delete-bucket-tagging-success", + app: app, + args: args{ + req: httptest.NewRequest(http.MethodDelete, "/my-bucket?tagging", nil), + }, + wantErr: false, + statusCode: 204, + }, } for _, tt := range tests { resp, err := tt.app.Test(tt.args.req) From 68794518af69c7aa084503ffccda4c1db853e932 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Wed, 31 Jan 2024 10:02:35 -0800 Subject: [PATCH 3/3] fix: remove special proxy handling for bucket acls in posix backend --- backend/posix/posix.go | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index cba70957..70bbe4ac 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -60,8 +60,6 @@ const ( contentEncHdr = "content-encoding" emptyMD5 = "d41d8cd98f00b204e9800998ecf8427e" aclkey = "user.acl" - proxyAclKey = "versitygwAcl" - proxyBucketAclKey = "user.proxy.acl" etagkey = "user.etag" ) @@ -1731,17 +1729,6 @@ func (p *Posix) PutBucketTagging(_ context.Context, bucket string, tags map[stri return fmt.Errorf("stat bucket: %w", err) } - acl, ok := tags[proxyAclKey] - if ok { - // set proxy acl in a separate xattr key - err = xattr.Set(bucket, proxyBucketAclKey, []byte(acl)) - if err != nil { - return fmt.Errorf("set proxy acl: %w", err) - } - - return nil - } - if tags == nil { err = xattr.Remove(bucket, "user."+tagHdr) if err != nil { @@ -1777,16 +1764,6 @@ func (p *Posix) GetBucketTagging(_ context.Context, bucket string) (map[string]s return nil, err } - acl, err := xattr.Get(bucket, proxyBucketAclKey) - if isNoAttr(err) { - return tags, nil - } - if err != nil { - return nil, fmt.Errorf("get tags: %w", err) - } - - tags[proxyAclKey] = string(acl) - return tags, nil }