From e33615a9f6e8d25e1f39dd9f88b6d3497f590b15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar=20Nieto?= Date: Mon, 6 Apr 2020 09:59:19 -0700 Subject: [PATCH] mcs make bucket api remove setting access policy (#29) --- models/make_bucket_request.go | 23 --------------- restapi/client-admin.go | 7 +++-- restapi/client.go | 1 - restapi/embedded_spec.go | 6 ---- restapi/user_buckets.go | 53 +++++++++++++++++------------------ restapi/user_buckets_test.go | 36 +++--------------------- swagger.yml | 2 -- 7 files changed, 33 insertions(+), 95 deletions(-) diff --git a/models/make_bucket_request.go b/models/make_bucket_request.go index 1808694e8..0f04cb194 100644 --- a/models/make_bucket_request.go +++ b/models/make_bucket_request.go @@ -34,9 +34,6 @@ import ( // swagger:model makeBucketRequest type MakeBucketRequest struct { - // access - Access BucketAccess `json:"access,omitempty"` - // name // Required: true Name *string `json:"name"` @@ -46,10 +43,6 @@ type MakeBucketRequest struct { func (m *MakeBucketRequest) Validate(formats strfmt.Registry) error { var res []error - if err := m.validateAccess(formats); err != nil { - res = append(res, err) - } - if err := m.validateName(formats); err != nil { res = append(res, err) } @@ -60,22 +53,6 @@ func (m *MakeBucketRequest) Validate(formats strfmt.Registry) error { return nil } -func (m *MakeBucketRequest) validateAccess(formats strfmt.Registry) error { - - if swag.IsZero(m.Access) { // not required - return nil - } - - if err := m.Access.Validate(formats); err != nil { - if ve, ok := err.(*errors.Validation); ok { - return ve.ValidateName("access") - } - return err - } - - return nil -} - func (m *MakeBucketRequest) validateName(formats strfmt.Registry) error { if err := validate.Required("name", "body", m.Name); err != nil { diff --git a/restapi/client-admin.go b/restapi/client-admin.go index b75ba3ddb..567ae9428 100644 --- a/restapi/client-admin.go +++ b/restapi/client-admin.go @@ -18,12 +18,13 @@ package restapi import ( "context" - mcCmd "github.com/minio/mc/cmd" - "github.com/minio/mc/pkg/probe" - "github.com/minio/minio/pkg/madmin" "io" "path/filepath" "runtime" + + mcCmd "github.com/minio/mc/cmd" + "github.com/minio/mc/pkg/probe" + "github.com/minio/minio/pkg/madmin" ) const globalAppName = "mcs" diff --git a/restapi/client.go b/restapi/client.go index 0b063b95f..e317b50b5 100644 --- a/restapi/client.go +++ b/restapi/client.go @@ -31,7 +31,6 @@ func init() { minio.MaxRetry = 1 } - // Define MinioClient interface with all functions to be implemented // by mock when testing, it should include all MinioClient respective api calls // that are used within this project. diff --git a/restapi/embedded_spec.go b/restapi/embedded_spec.go index e4fa16bb1..ca6c10770 100644 --- a/restapi/embedded_spec.go +++ b/restapi/embedded_spec.go @@ -1317,9 +1317,6 @@ func init() { "name" ], "properties": { - "access": { - "$ref": "#/definitions/bucketAccess" - }, "name": { "type": "string" } @@ -2855,9 +2852,6 @@ func init() { "name" ], "properties": { - "access": { - "$ref": "#/definitions/bucketAccess" - }, "name": { "type": "string" } diff --git a/restapi/user_buckets.go b/restapi/user_buckets.go index 1961e4803..516d54cd5 100644 --- a/restapi/user_buckets.go +++ b/restapi/user_buckets.go @@ -124,13 +124,35 @@ func getListBucketsResponse() (*models.ListBucketsResponse, error) { } // makeBucket creates a bucket for an specific minio client -func makeBucket(ctx context.Context, client MinioClient, bucketName string, access models.BucketAccess) error { +func makeBucket(ctx context.Context, client MinioClient, bucketName string) error { // creates a new bucket with bucketName with a context to control cancellations and timeouts. if err := client.makeBucketWithContext(ctx, bucketName, "us-east-1"); err != nil { return err } - if err := setBucketAccessPolicy(ctx, client, bucketName, access); err != nil { - return fmt.Errorf("bucket created but error occurred while setting policy: %s", err.Error()) + return nil +} + +// getMakeBucketResponse performs makeBucket() to create a bucket with its access policy +func getMakeBucketResponse(br *models.MakeBucketRequest) error { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*20) + defer cancel() + // bucket request needed to proceed + if br == nil { + log.Println("error bucket body not in request") + return errors.New(500, "error bucket body not in request") + } + mClient, err := newMinioClient() + if err != nil { + log.Println("error creating MinIO Client:", err) + return err + } + // create a minioClient interface implementation + // defining the client to be used + minioClient := minioClient{client: mClient} + + if err := makeBucket(ctx, minioClient, *br.Name); err != nil { + log.Println("error making bucket:", err) + return err } return nil } @@ -163,31 +185,6 @@ func setBucketAccessPolicy(ctx context.Context, client MinioClient, bucketName s return client.setBucketPolicyWithContext(ctx, bucketName, string(policyJSON)) } -// getMakeBucketResponse performs makeBucket() to create a bucket with its access policy -func getMakeBucketResponse(br *models.MakeBucketRequest) error { - ctx, cancel := context.WithTimeout(context.Background(), time.Second*20) - defer cancel() - // bucket request needed to proceed - if br == nil { - log.Println("error bucket body not in request") - return errors.New(500, "error bucket body not in request") - } - mClient, err := newMinioClient() - if err != nil { - log.Println("error creating MinIO Client:", err) - return err - } - // create a minioClient interface implementation - // defining the client to be used - minioClient := minioClient{client: mClient} - - if err := makeBucket(ctx, minioClient, *br.Name, br.Access); err != nil { - log.Println("error making bucket:", err) - return err - } - return nil -} - // getBucketSetPolicyResponse calls setBucketAccessPolicy() to set a access policy to a bucket // and returns the serialized output. func getBucketSetPolicyResponse(bucketName string, req *models.SetBucketPolicyRequest) (*models.Bucket, error) { diff --git a/restapi/user_buckets_test.go b/restapi/user_buckets_test.go index 069edb185..a3baaa207 100644 --- a/restapi/user_buckets_test.go +++ b/restapi/user_buckets_test.go @@ -112,50 +112,22 @@ func TestMakeBucket(t *testing.T) { minClient := minioClientMock{} function := "makeBucket()" ctx := context.Background() - // Test-1: makeBucket() create a bucket with public access + // Test-1: makeBucket() create a bucket // mock function response from makeBucketWithContext(ctx) minioMakeBucketWithContextMock = func(ctx context.Context, bucketName, location string) error { return nil } - // mock function response from setBucketPolicyWithContext(ctx) - minioSetBucketPolicyWithContextMock = func(ctx context.Context, bucketName, policy string) error { - return nil - } - if err := makeBucket(ctx, minClient, "bucktest1", models.BucketAccessPUBLIC); err != nil { + if err := makeBucket(ctx, minClient, "bucktest1"); err != nil { t.Errorf("Failed on %s:, error occurred: %s", function, err.Error()) } - // Test-2: makeBucket() create a bucket with private access - if err := makeBucket(ctx, minClient, "bucktest1", models.BucketAccessPRIVATE); err != nil { - t.Errorf("Failed on %s:, error occurred: %s", function, err.Error()) - } - - // Test-3: makeBucket() create a bucket with an invalid access, expected error - if err := makeBucket(ctx, minClient, "bucktest1", "other"); assert.Error(err) { - assert.Equal("bucket created but error occurred while setting policy: access: `other` not supported", err.Error()) - } - - // Test-4 makeBucket() make sure errors are handled correctly when error on MakeBucketWithContext + // Test-2 makeBucket() make sure errors are handled correctly when error on MakeBucketWithContext minioMakeBucketWithContextMock = func(ctx context.Context, bucketName, location string) error { return errors.New("error") } - minioSetBucketPolicyWithContextMock = func(ctx context.Context, bucketName, policy string) error { - return nil - } - if err := makeBucket(ctx, minClient, "bucktest1", models.BucketAccessPUBLIC); assert.Error(err) { + if err := makeBucket(ctx, minClient, "bucktest1"); assert.Error(err) { assert.Equal("error", err.Error()) } - - // Test-5 makeBucket() make sure errors are handled correctly when error on SetBucketPolicyWithContext - minioMakeBucketWithContextMock = func(ctx context.Context, bucketName, location string) error { - return nil - } - minioSetBucketPolicyWithContextMock = func(ctx context.Context, bucketName, policy string) error { - return errors.New("error") - } - if err := makeBucket(ctx, minClient, "bucktest1", models.BucketAccessPUBLIC); assert.Error(err) { - assert.Equal("bucket created but error occurred while setting policy: error", err.Error()) - } } func TestDeleteBucket(t *testing.T) { diff --git a/swagger.yml b/swagger.yml index 43223ad5b..516a95523 100644 --- a/swagger.yml +++ b/swagger.yml @@ -684,8 +684,6 @@ definitions: properties: name: type: string - access: - $ref: "#/definitions/bucketAccess" error: type: object required: