Merge pull request #123 from versity/unit-testing-cleanup

Unit testing cleanup

auth/iam.go

@@ -25,6 +25,8 @@ type Account struct {
 }
 
 // IAMService is the interface for all IAM service implementations
+//
+//go:generate moq -out ../s3api/controllers/iam_moq_test.go -pkg controllers . IAMService
 type IAMService interface {
 	CreateAccount(access string, account Account) error
 	GetUserAccount(access string) (Account, error)

backend/backend.go

@@ -24,7 +24,6 @@ import (
 	"github.com/versity/versitygw/s3response"
 )
 
-//go:generate moq -out backend_moq_test.go . Backend
 //go:generate moq -out ../s3api/controllers/backend_moq_test.go -pkg controllers . Backend
 type Backend interface {
 	fmt.Stringer

backend/backend_test.go

@@ -1,222 +0,0 @@
-// Copyright 2023 Versity Software
-// This file is licensed under the Apache License, Version 2.0
-// (the "License"); you may not use this file except in compliance
-// with the License.  You may obtain a copy of the License at
-//
-//   http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied.  See the License for the
-// specific language governing permissions and limitations
-// under the License.
-
-package backend
-
-import (
-	"context"
-	"testing"
-
-	"github.com/aws/aws-sdk-go-v2/service/s3"
-	"github.com/versity/versitygw/s3err"
-	"github.com/versity/versitygw/s3response"
-)
-
-func TestBackend_ListBuckets(t *testing.T) {
-	type args struct {
-		ctx context.Context
-	}
-	type test struct {
-		name    string
-		c       Backend
-		args    args
-		wantErr bool
-	}
-	var tests []test
-	tests = append(tests, test{
-		name: "list-Bucket",
-		c: &BackendMock{
-			ListBucketsFunc: func() (s3response.ListAllMyBucketsResult, error) {
-				return s3response.ListAllMyBucketsResult{
-					Buckets: s3response.ListAllMyBucketsList{
-						Bucket: []s3response.ListAllMyBucketsEntry{
-							{
-								Name: "t1",
-							},
-						},
-					},
-				}, s3err.GetAPIError(0)
-			},
-		},
-		args: args{
-			ctx: context.Background(),
-		},
-		wantErr: false,
-	}, test{
-		name: "list-Bucket-error",
-		c: &BackendMock{
-			ListBucketsFunc: func() (s3response.ListAllMyBucketsResult, error) {
-				return s3response.ListAllMyBucketsResult{}, s3err.GetAPIError(s3err.ErrNotImplemented)
-			},
-		},
-		args: args{
-			ctx: context.Background(),
-		},
-		wantErr: true,
-	})
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if _, err := tt.c.ListBuckets(); (err.(s3err.APIError).Code != "") != tt.wantErr {
-				t.Errorf("Backend.ListBuckets() error = %v, wantErr %v", err, tt.wantErr)
-			}
-		})
-	}
-}
-func TestBackend_HeadBucket(t *testing.T) {
-	type args struct {
-		ctx        context.Context
-		BucketName string
-	}
-	type test struct {
-		name    string
-		c       Backend
-		args    args
-		wantErr bool
-	}
-	var tests []test
-	tests = append(tests, test{
-		name: "head-buckets-error",
-		c: &BackendMock{
-			HeadBucketFunc: func(bucket string) (*s3.HeadBucketOutput, error) {
-				return nil, s3err.GetAPIError(s3err.ErrNotImplemented)
-			},
-		},
-		args: args{
-			ctx:        context.Background(),
-			BucketName: "b1",
-		},
-		wantErr: true,
-	})
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if _, err := tt.c.HeadBucket(tt.args.BucketName); (err.(s3err.APIError).Code != "") != tt.wantErr {
-				t.Errorf("Backend.HeadBucket() error = %v, wantErr %v", err, tt.wantErr)
-			}
-		})
-	}
-}
-func TestBackend_GetBucketAcl(t *testing.T) {
-	type args struct {
-		ctx        context.Context
-		bucketName string
-	}
-	type test struct {
-		name    string
-		c       Backend
-		args    args
-		wantErr bool
-	}
-	var tests []test
-	tests = append(tests, test{
-		name: "get bucket acl error",
-		c: &BackendMock{
-			GetBucketAclFunc: func(bucket string) ([]byte, error) {
-				return nil, s3err.GetAPIError(s3err.ErrNotImplemented)
-			},
-		},
-		args: args{
-			ctx:        context.Background(),
-			bucketName: "b1",
-		},
-		wantErr: true,
-	})
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if _, err := tt.c.GetBucketAcl(tt.args.bucketName); (err.(s3err.APIError).Code != "") != tt.wantErr {
-				t.Errorf("Backend.GetBucketAcl() error = %v, wantErr %v", err, tt.wantErr)
-			}
-		})
-	}
-}
-func TestBackend_PutBucket(t *testing.T) {
-	type args struct {
-		ctx         context.Context
-		bucketName  string
-		bucketOwner string
-	}
-	type test struct {
-		name    string
-		c       Backend
-		args    args
-		wantErr bool
-	}
-	var tests []test
-	tests = append(tests, test{
-		name: "put bucket ",
-		c: &BackendMock{
-			PutBucketFunc: func(bucket, owner string) error {
-				return s3err.GetAPIError(0)
-			},
-		},
-		args: args{
-			ctx:         context.Background(),
-			bucketName:  "b1",
-			bucketOwner: "owner",
-		},
-		wantErr: false,
-	}, test{
-		name: "put bucket error",
-		c: &BackendMock{
-			PutBucketFunc: func(bucket, owner string) error {
-				return s3err.GetAPIError(s3err.ErrNotImplemented)
-			},
-		},
-		args: args{
-			ctx:         context.Background(),
-			bucketName:  "b2",
-			bucketOwner: "owner",
-		},
-		wantErr: true,
-	})
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if err := tt.c.PutBucket(tt.args.bucketName, tt.args.bucketOwner); (err.(s3err.APIError).Code != "") != tt.wantErr {
-				t.Errorf("Backend.PutBucket() error = %v, wantErr %v", err, tt.wantErr)
-			}
-		})
-	}
-}
-func TestBackend_DeleteBucket(t *testing.T) {
-	type args struct {
-		ctx        context.Context
-		bucketName string
-	}
-	type test struct {
-		name    string
-		c       Backend
-		args    args
-		wantErr bool
-	}
-	var tests []test
-	tests = append(tests, test{
-		name: "Delete Bucket Error",
-		c: &BackendMock{
-			DeleteBucketFunc: func(bucket string) error {
-				return s3err.GetAPIError(s3err.ErrNotImplemented)
-			},
-		},
-		args: args{
-			ctx:        context.Background(),
-			bucketName: "b1",
-		},
-		wantErr: true,
-	})
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if err := tt.c.DeleteBucket(tt.args.bucketName); (err.(s3err.APIError).Code != "") != tt.wantErr {
-				t.Errorf("Backend.DeleteBucket() error = %v, wantErr %v", err, tt.wantErr)
-			}
-		})
-	}
-}

s3api/controllers/admin.go

@@ -32,6 +32,9 @@ func (c AdminController) CreateUser(ctx *fiber.Ctx) error {
 	if requesterRole != "admin" {
 		return fmt.Errorf("access denied: only admin users have access to this resource")
 	}
+	if role != "user" && role != "admin" {
+		return fmt.Errorf("invalid parameters: user role have to be one of the following: 'user', 'admin'")
+	}
 
 	user := auth.Account{Secret: secret, Role: role}
 
@@ -40,8 +43,7 @@ func (c AdminController) CreateUser(ctx *fiber.Ctx) error {
 		return fmt.Errorf("failed to create a user: %w", err)
 	}
 
-	ctx.SendString("The user has been created successfully")
-	return nil
+	return ctx.SendString("The user has been created successfully")
 }
 
 func (c AdminController) DeleteUser(ctx *fiber.Ctx) error {
@@ -56,6 +58,5 @@ func (c AdminController) DeleteUser(ctx *fiber.Ctx) error {
 		return err
 	}
 
-	ctx.SendString("The user has been created successfully")
-	return nil
+	return ctx.SendString("The user has been created successfully")
 }

s3api/controllers/admin_test.go

@@ -0,0 +1,173 @@
+// Copyright 2023 Versity Software
+// This file is licensed under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package controllers
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gofiber/fiber/v2"
+	"github.com/versity/versitygw/auth"
+)
+
+func TestAdminController_CreateUser(t *testing.T) {
+	type args struct {
+		req *http.Request
+	}
+
+	adminController := AdminController{
+		IAMService: &IAMServiceMock{
+			CreateAccountFunc: func(access string, account auth.Account) error {
+				return nil
+			},
+		},
+	}
+
+	app := fiber.New()
+
+	app.Use(func(ctx *fiber.Ctx) error {
+		ctx.Locals("role", "admin")
+		return ctx.Next()
+	})
+
+	app.Post("/create-user", adminController.CreateUser)
+
+	appErr := fiber.New()
+
+	appErr.Use(func(ctx *fiber.Ctx) error {
+		ctx.Locals("role", "user")
+		return ctx.Next()
+	})
+
+	appErr.Post("/create-user", adminController.CreateUser)
+
+	tests := []struct {
+		name       string
+		app        *fiber.App
+		args       args
+		wantErr    bool
+		statusCode int
+	}{
+		{
+			name: "Admin-create-user-success",
+			app:  app,
+			args: args{
+				req: httptest.NewRequest(http.MethodPost, "/create-user?access=test&secret=test&role=user", nil),
+			},
+			wantErr:    false,
+			statusCode: 200,
+		},
+		{
+			name: "Admin-create-user-invalid-user-role",
+			app:  app,
+			args: args{
+				req: httptest.NewRequest(http.MethodPost, "/create-user?access=test&secret=test&role=invalid", nil),
+			},
+			wantErr:    false,
+			statusCode: 500,
+		},
+		{
+			name: "Admin-create-user-invalid-requester-role",
+			app:  appErr,
+			args: args{
+				req: httptest.NewRequest(http.MethodPost, "/create-user?access=test&secret=test&role=admin", nil),
+			},
+			wantErr:    false,
+			statusCode: 500,
+		},
+	}
+	for _, tt := range tests {
+		resp, err := tt.app.Test(tt.args.req)
+
+		if (err != nil) != tt.wantErr {
+			t.Errorf("AdminController.CreateUser() error = %v, wantErr %v", err, tt.wantErr)
+		}
+
+		if resp.StatusCode != tt.statusCode {
+			t.Errorf("AdminController.CreateUser() statusCode = %v, wantStatusCode = %v", resp.StatusCode, tt.statusCode)
+		}
+	}
+}
+
+func TestAdminController_DeleteUser(t *testing.T) {
+	type args struct {
+		req *http.Request
+	}
+
+	adminController := AdminController{
+		IAMService: &IAMServiceMock{
+			DeleteUserAccountFunc: func(access string) error {
+				return nil
+			},
+		},
+	}
+
+	app := fiber.New()
+
+	app.Use(func(ctx *fiber.Ctx) error {
+		ctx.Locals("role", "admin")
+		return ctx.Next()
+	})
+
+	app.Delete("/delete-user", adminController.DeleteUser)
+
+	appErr := fiber.New()
+
+	appErr.Use(func(ctx *fiber.Ctx) error {
+		ctx.Locals("role", "user")
+		return ctx.Next()
+	})
+
+	appErr.Delete("/delete-user", adminController.DeleteUser)
+
+	tests := []struct {
+		name       string
+		app        *fiber.App
+		args       args
+		wantErr    bool
+		statusCode int
+	}{
+		{
+			name: "Admin-delete-user-success",
+			app:  app,
+			args: args{
+				req: httptest.NewRequest(http.MethodDelete, "/delete-user?access=test", nil),
+			},
+			wantErr:    false,
+			statusCode: 200,
+		},
+		{
+			name: "Admin-delete-user-invalid-requester-role",
+			app:  appErr,
+			args: args{
+				req: httptest.NewRequest(http.MethodDelete, "/delete-user?access=test", nil),
+			},
+			wantErr:    false,
+			statusCode: 500,
+		},
+	}
+	for _, tt := range tests {
+		resp, err := tt.app.Test(tt.args.req)
+
+		if (err != nil) != tt.wantErr {
+			t.Errorf("AdminController.DeleteUser() error = %v, wantErr %v", err, tt.wantErr)
+		}
+
+		if resp.StatusCode != tt.statusCode {
+			t.Errorf("AdminController.DeleteUser() statusCode = %v, wantStatusCode = %v", resp.StatusCode, tt.statusCode)
+		}
+	}
+}

s3api/controllers/base.go

@@ -550,7 +550,7 @@ func (c S3ApiController) DeleteObjects(ctx *fiber.Ctx) error {
 	var dObj types.Delete
 
 	if err := xml.Unmarshal(ctx.Body(), &dObj); err != nil {
-		return errors.New("wrong api call")
+		return SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidRequest))
 	}
 
 	data, err := c.be.GetBucketAcl(bucket)

s3api/controllers/base_test.go

@@ -16,6 +16,7 @@ package controllers
 
 import (
 	"encoding/json"
+	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -962,7 +963,7 @@ func TestS3ApiController_DeleteObjects(t *testing.T) {
 				req: httptest.NewRequest(http.MethodPost, "/my-bucket", nil),
 			},
 			wantErr:    false,
-			statusCode: 500,
+			statusCode: 400,
 		},
 	}
 	for _, tt := range tests {
@@ -995,6 +996,9 @@ func TestS3ApiController_DeleteActions(t *testing.T) {
 			AbortMultipartUploadFunc: func(*s3.AbortMultipartUploadInput) error {
 				return nil
 			},
+			RemoveTagsFunc: func(bucket, object string) error {
+				return nil
+			},
 		},
 	}
 
@@ -1006,7 +1010,7 @@ func TestS3ApiController_DeleteActions(t *testing.T) {
 	})
 	app.Delete("/:bucket/:key/*", s3ApiController.DeleteActions)
 
-	//Error case
+	// Error case
 	appErr := fiber.New()
 
 	s3ApiControllerErr := S3ApiController{be: &BackendMock{
@@ -1024,7 +1028,7 @@ func TestS3ApiController_DeleteActions(t *testing.T) {
 		ctx.Locals("isDebug", false)
 		return ctx.Next()
 	})
-	appErr.Delete("/:bucket", s3ApiControllerErr.DeleteBucket)
+	appErr.Delete("/:bucket/:key/*", s3ApiControllerErr.DeleteActions)
 
 	tests := []struct {
 		name       string
@@ -1042,6 +1046,15 @@ func TestS3ApiController_DeleteActions(t *testing.T) {
 			wantErr:    false,
 			statusCode: 200,
 		},
+		{
+			name: "Remove-object-tagging-success",
+			app:  app,
+			args: args{
+				req: httptest.NewRequest(http.MethodDelete, "/my-bucket/my-key/key2?tagging", nil),
+			},
+			wantErr:    false,
+			statusCode: 200,
+		},
 		{
 			name: "Delete-object-success",
 			app:  app,
@@ -1469,6 +1482,16 @@ func Test_response(t *testing.T) {
 			wantErr:    false,
 			statusCode: 500,
 		},
+		{
+			name: "Internal-server-error-not-api",
+			args: args{
+				ctx:  &ctx,
+				resp: nil,
+				err:  fmt.Errorf("custom error"),
+			},
+			wantErr:    false,
+			statusCode: 500,
+		},
 		{
 			name: "Error-not-implemented",
 			args: args{

s3api/controllers/iam_moq_test.go

@@ -0,0 +1,169 @@
+// Code generated by moq; DO NOT EDIT.
+// github.com/matryer/moq
+
+package controllers
+
+import (
+	"github.com/versity/versitygw/auth"
+	"sync"
+)
+
+// Ensure, that IAMServiceMock does implement auth.IAMService.
+// If this is not the case, regenerate this file with moq.
+var _ auth.IAMService = &IAMServiceMock{}
+
+// IAMServiceMock is a mock implementation of auth.IAMService.
+//
+//	func TestSomethingThatUsesIAMService(t *testing.T) {
+//
+//		// make and configure a mocked auth.IAMService
+//		mockedIAMService := &IAMServiceMock{
+//			CreateAccountFunc: func(access string, account auth.Account) error {
+//				panic("mock out the CreateAccount method")
+//			},
+//			DeleteUserAccountFunc: func(access string) error {
+//				panic("mock out the DeleteUserAccount method")
+//			},
+//			GetUserAccountFunc: func(access string) (auth.Account, error) {
+//				panic("mock out the GetUserAccount method")
+//			},
+//		}
+//
+//		// use mockedIAMService in code that requires auth.IAMService
+//		// and then make assertions.
+//
+//	}
+type IAMServiceMock struct {
+	// CreateAccountFunc mocks the CreateAccount method.
+	CreateAccountFunc func(access string, account auth.Account) error
+
+	// DeleteUserAccountFunc mocks the DeleteUserAccount method.
+	DeleteUserAccountFunc func(access string) error
+
+	// GetUserAccountFunc mocks the GetUserAccount method.
+	GetUserAccountFunc func(access string) (auth.Account, error)
+
+	// calls tracks calls to the methods.
+	calls struct {
+		// CreateAccount holds details about calls to the CreateAccount method.
+		CreateAccount []struct {
+			// Access is the access argument value.
+			Access string
+			// Account is the account argument value.
+			Account auth.Account
+		}
+		// DeleteUserAccount holds details about calls to the DeleteUserAccount method.
+		DeleteUserAccount []struct {
+			// Access is the access argument value.
+			Access string
+		}
+		// GetUserAccount holds details about calls to the GetUserAccount method.
+		GetUserAccount []struct {
+			// Access is the access argument value.
+			Access string
+		}
+	}
+	lockCreateAccount     sync.RWMutex
+	lockDeleteUserAccount sync.RWMutex
+	lockGetUserAccount    sync.RWMutex
+}
+
+// CreateAccount calls CreateAccountFunc.
+func (mock *IAMServiceMock) CreateAccount(access string, account auth.Account) error {
+	if mock.CreateAccountFunc == nil {
+		panic("IAMServiceMock.CreateAccountFunc: method is nil but IAMService.CreateAccount was just called")
+	}
+	callInfo := struct {
+		Access  string
+		Account auth.Account
+	}{
+		Access:  access,
+		Account: account,
+	}
+	mock.lockCreateAccount.Lock()
+	mock.calls.CreateAccount = append(mock.calls.CreateAccount, callInfo)
+	mock.lockCreateAccount.Unlock()
+	return mock.CreateAccountFunc(access, account)
+}
+
+// CreateAccountCalls gets all the calls that were made to CreateAccount.
+// Check the length with:
+//
+//	len(mockedIAMService.CreateAccountCalls())
+func (mock *IAMServiceMock) CreateAccountCalls() []struct {
+	Access  string
+	Account auth.Account
+} {
+	var calls []struct {
+		Access  string
+		Account auth.Account
+	}
+	mock.lockCreateAccount.RLock()
+	calls = mock.calls.CreateAccount
+	mock.lockCreateAccount.RUnlock()
+	return calls
+}
+
+// DeleteUserAccount calls DeleteUserAccountFunc.
+func (mock *IAMServiceMock) DeleteUserAccount(access string) error {
+	if mock.DeleteUserAccountFunc == nil {
+		panic("IAMServiceMock.DeleteUserAccountFunc: method is nil but IAMService.DeleteUserAccount was just called")
+	}
+	callInfo := struct {
+		Access string
+	}{
+		Access: access,
+	}
+	mock.lockDeleteUserAccount.Lock()
+	mock.calls.DeleteUserAccount = append(mock.calls.DeleteUserAccount, callInfo)
+	mock.lockDeleteUserAccount.Unlock()
+	return mock.DeleteUserAccountFunc(access)
+}
+
+// DeleteUserAccountCalls gets all the calls that were made to DeleteUserAccount.
+// Check the length with:
+//
+//	len(mockedIAMService.DeleteUserAccountCalls())
+func (mock *IAMServiceMock) DeleteUserAccountCalls() []struct {
+	Access string
+} {
+	var calls []struct {
+		Access string
+	}
+	mock.lockDeleteUserAccount.RLock()
+	calls = mock.calls.DeleteUserAccount
+	mock.lockDeleteUserAccount.RUnlock()
+	return calls
+}
+
+// GetUserAccount calls GetUserAccountFunc.
+func (mock *IAMServiceMock) GetUserAccount(access string) (auth.Account, error) {
+	if mock.GetUserAccountFunc == nil {
+		panic("IAMServiceMock.GetUserAccountFunc: method is nil but IAMService.GetUserAccount was just called")
+	}
+	callInfo := struct {
+		Access string
+	}{
+		Access: access,
+	}
+	mock.lockGetUserAccount.Lock()
+	mock.calls.GetUserAccount = append(mock.calls.GetUserAccount, callInfo)
+	mock.lockGetUserAccount.Unlock()
+	return mock.GetUserAccountFunc(access)
+}
+
+// GetUserAccountCalls gets all the calls that were made to GetUserAccount.
+// Check the length with:
+//
+//	len(mockedIAMService.GetUserAccountCalls())
+func (mock *IAMServiceMock) GetUserAccountCalls() []struct {
+	Access string
+} {
+	var calls []struct {
+		Access string
+	}
+	mock.lockGetUserAccount.RLock()
+	calls = mock.calls.GetUserAccount
+	mock.lockGetUserAccount.RUnlock()
+	return calls
+}