From d2a4e0d4e33bad71d5fe5b443db4f2759fcabadb Mon Sep 17 00:00:00 2001 From: Jiffin Tony Thottan Date: Thu, 20 Jul 2023 12:43:31 +0530 Subject: [PATCH] define parameters for initializeClients in unit testing Parameters are not defined for unit tests, variable is added but no explict check is defined. Signed-off-by: Jiffin Tony Thottan --- pkg/driver/provisioner.go | 35 ++++++++++++-------- pkg/driver/provisioner_test.go | 59 ++++++++++++++++++++++------------ 2 files changed, 60 insertions(+), 34 deletions(-) diff --git a/pkg/driver/provisioner.go b/pkg/driver/provisioner.go index 02a6faa..f1d5927 100644 --- a/pkg/driver/provisioner.go +++ b/pkg/driver/provisioner.go @@ -249,13 +249,10 @@ func fetchUserCredentials(user rgwadmin.User, endpoint string, region string) ma func InitializeClients(ctx context.Context, clientset *kubernetes.Clientset, parameters map[string]string) (*s3client.S3Agent, *rgwadmin.API, error) { klog.V(5).Infof("Initializing clients %v", parameters) - objectStoreUserSecretName := parameters["objectStoreUserSecretName"] - namespace := os.Getenv("POD_NAMESPACE") - if parameters["objectStoreUserSecretNamespace"] != "" { - namespace = parameters["objectStoreUserSecretNamespace"] - } - if objectStoreUserSecretName == "" || namespace == "" { - return nil, nil, status.Error(codes.InvalidArgument, "objectStoreUserSecretName and Namespace is required") + + objectStoreUserSecretName, namespace, err := fetchSecretNameAndNamespace(parameters) + if err != nil { + return nil, nil, err } objectStoreUserSecret, err := clientset.CoreV1().Secrets(namespace).Get(ctx, objectStoreUserSecretName, metav1.GetOptions{}) @@ -284,15 +281,27 @@ func InitializeClients(ctx context.Context, clientset *kubernetes.Clientset, par return s3Client, rgwAdminClient, nil } -func fetchParameters(parameters map[string][]byte) (string, string, string, string, error) { - - accessKey := string(parameters["AccessKey"]) - secretKey := string(parameters["SecretKey"]) - endPoint := string(parameters["Endpoint"]) +func fetchParameters(secretData map[string][]byte) (string, string, string, string, error) { + accessKey := string(secretData["AccessKey"]) + secretKey := string(secretData["SecretKey"]) + endPoint := string(secretData["Endpoint"]) if endPoint == "" || accessKey == "" || secretKey == "" { return "", "", "", "", status.Error(codes.InvalidArgument, "endpoint, accessKeyID and secretKey are required") } - tlsCert := string(parameters["SSLCertSecretName"]) + tlsCert := string(secretData["SSLCertSecretName"]) return accessKey, secretKey, endPoint, tlsCert, nil } + +func fetchSecretNameAndNamespace(parameters map[string]string) (string, string, error) { + secretName := parameters["objectStoreUserSecretName"] + namespace := os.Getenv("POD_NAMESPACE") + if parameters["objectStoreUserSecretNamespace"] != "" { + namespace = parameters["objectStoreUserSecretNamespace"] + } + if secretName == "" || namespace == "" { + return "", "", status.Error(codes.InvalidArgument, "objectStoreUserSecretName and Namespace is required") + } + + return secretName, namespace, nil +} diff --git a/pkg/driver/provisioner_test.go b/pkg/driver/provisioner_test.go index 7dd309d..97dfbbc 100644 --- a/pkg/driver/provisioner_test.go +++ b/pkg/driver/provisioner_test.go @@ -45,8 +45,8 @@ const ( "keys": [ { "user": "test-user", - "access_key": "EOE7FYCNOBZJ5VFV909G", - "secret_key": "qmIqpWm8HxCzmynCrD6U6vKWi4hnDBndOnmxXNsV" + "access_key": "AccessKey", + "secret_key": "SecretKey" } ], "swift_keys": [], @@ -80,6 +80,12 @@ const ( }` ) +func createParameters() map[string]string { + return map[string]string{ + "objectStoreUserSecretName": "test-user-secret", + "objectStoreUserSecretNamespace": "test-namespace", + } +} func Test_provisionerServer_DriverCreateBucket(t *testing.T) { type fields struct { provisioner string @@ -91,6 +97,10 @@ func Test_provisionerServer_DriverCreateBucket(t *testing.T) { } initializeClients = func(ctx context.Context, clientset *kubernetes.Clientset, parameters map[string]string) (*s3cli.S3Agent, *rgwadmin.API, error) { + _, _, err := fetchSecretNameAndNamespace(parameters) + if err != nil { + t.Fatalf("failed to fetch secret name and namespace: %v", err) + } s3Client := &s3cli.S3Agent{ Client: mockS3Client{}, } @@ -104,11 +114,11 @@ func Test_provisionerServer_DriverCreateBucket(t *testing.T) { want *cosispec.DriverCreateBucketResponse wantErr bool }{ - {"Empty Bucket Name", fields{"CreateBucket Empty Bucket Name"}, args{context.Background(), &cosispec.DriverCreateBucketRequest{Name: ""}}, nil, true}, - {"Create Bucket success", fields{"CreateBucket Success"}, args{context.Background(), &cosispec.DriverCreateBucketRequest{Name: "test-bucket"}}, &cosispec.DriverCreateBucketResponse{BucketId: "test-bucket"}, false}, - {"Create Bucket failure", fields{"CreateBucket Failure"}, args{context.Background(), &cosispec.DriverCreateBucketRequest{Name: "failed-bucket"}}, nil, true}, - {"Bucket already Exists", fields{"CreateBucket Already Exists"}, args{context.Background(), &cosispec.DriverCreateBucketRequest{Name: "test-bucket-already-exists"}}, nil, true}, - {"Bucket owned same user", fields{"CreateBucket Owned by same user"}, args{context.Background(), &cosispec.DriverCreateBucketRequest{Name: "test-bucket-owned-by-same-user"}}, nil, true}, + {"Empty Bucket Name", fields{"CreateBucket Empty Bucket Name"}, args{context.Background(), &cosispec.DriverCreateBucketRequest{Name: "", Parameters: createParameters()}}, nil, true}, + {"Create Bucket success", fields{"CreateBucket Success"}, args{context.Background(), &cosispec.DriverCreateBucketRequest{Name: "test-bucket", Parameters: createParameters()}}, &cosispec.DriverCreateBucketResponse{BucketId: "test-bucket"}, false}, + {"Create Bucket failure", fields{"CreateBucket Failure"}, args{context.Background(), &cosispec.DriverCreateBucketRequest{Name: "failed-bucket", Parameters: createParameters()}}, nil, true}, + {"Bucket already Exists", fields{"CreateBucket Already Exists"}, args{context.Background(), &cosispec.DriverCreateBucketRequest{Name: "test-bucket-already-exists", Parameters: createParameters()}}, nil, true}, + {"Bucket owned same user", fields{"CreateBucket Owned by same user"}, args{context.Background(), &cosispec.DriverCreateBucketRequest{Name: "test-bucket-owned-by-same-user", Parameters: createParameters()}}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -136,6 +146,11 @@ func Test_provisionerServer_DriverGrantBucketAccess(t *testing.T) { req *cosispec.DriverGrantBucketAccessRequest } initializeClients = func(ctx context.Context, clientset *kubernetes.Clientset, parameters map[string]string) (*s3cli.S3Agent, *rgwadmin.API, error) { + _, _, err := fetchSecretNameAndNamespace(parameters) + if err != nil { + t.Fatalf("failed to fetch secret name and namespace: %v", err) + } + s3Client := &s3cli.S3Agent{ Client: mockS3Client{}, } @@ -170,12 +185,12 @@ func Test_provisionerServer_DriverGrantBucketAccess(t *testing.T) { want *cosispec.DriverGrantBucketAccessResponse wantErr bool }{ - {"Empty Bucket Name", fields{"GrantBucketAccess Empty Bucket Name"}, args{context.Background(), &cosispec.DriverGrantBucketAccessRequest{BucketId: "", Name: "test-user"}}, nil, true}, - {"Empty User Name", fields{"GrantBucketAccess Empty User Name"}, args{context.Background(), &cosispec.DriverGrantBucketAccessRequest{BucketId: "test-bucket", Name: ""}}, nil, true}, - {"Grant Bucket Access success", fields{"GrantBucketAccess Success"}, args{context.Background(), &cosispec.DriverGrantBucketAccessRequest{BucketId: "test-bucket", Name: "test-user"}}, &cosispec.DriverGrantBucketAccessResponse{AccountId: "test-user", Credentials: fetchUserCredentials(u, "rgw-my-store:8000", "")}, false}, - {"Grant Bucket Access failure", fields{"GrantBucketAccess Failure"}, args{context.Background(), &cosispec.DriverGrantBucketAccessRequest{BucketId: "failed-bucket", Name: "test-user"}}, nil, true}, - {"Bucket does not exist", fields{"GrantBucketAccess Does not exist"}, args{context.Background(), &cosispec.DriverGrantBucketAccessRequest{BucketId: "test-bucket-does-not-exist", Name: "test-user"}}, nil, true}, - {"User does not exist", fields{"GrantBucketAccess User Does not exist"}, args{context.Background(), &cosispec.DriverGrantBucketAccessRequest{BucketId: "test-bucket", Name: "test-user-does-not-exist"}}, nil, true}, + {"Empty Bucket Name", fields{"GrantBucketAccess Empty Bucket Name"}, args{context.Background(), &cosispec.DriverGrantBucketAccessRequest{BucketId: "", Name: "test-user", Parameters: createParameters()}}, nil, true}, + {"Empty User Name", fields{"GrantBucketAccess Empty User Name"}, args{context.Background(), &cosispec.DriverGrantBucketAccessRequest{BucketId: "test-bucket", Name: "", Parameters: createParameters()}}, nil, true}, + {"Grant Bucket Access success", fields{"GrantBucketAccess Success"}, args{context.Background(), &cosispec.DriverGrantBucketAccessRequest{BucketId: "test-bucket", Name: "test-user", Parameters: createParameters()}}, &cosispec.DriverGrantBucketAccessResponse{AccountId: "test-user", Credentials: fetchUserCredentials(u, "rgw-my-store:8000", "")}, false}, + {"Grant Bucket Access failure", fields{"GrantBucketAccess Failure"}, args{context.Background(), &cosispec.DriverGrantBucketAccessRequest{BucketId: "failed-bucket", Name: "test-user", Parameters: createParameters()}}, nil, true}, + {"Bucket does not exist", fields{"GrantBucketAccess Does not exist"}, args{context.Background(), &cosispec.DriverGrantBucketAccessRequest{BucketId: "test-bucket-does-not-exist", Name: "test-user", Parameters: createParameters()}}, nil, true}, + {"User does not exist", fields{"GrantBucketAccess User Does not exist"}, args{context.Background(), &cosispec.DriverGrantBucketAccessRequest{BucketId: "test-bucket", Name: "test-user-does-not-exist", Parameters: createParameters()}}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -205,6 +220,10 @@ func Test_provisionerServer_DriverDeleteBucket(t *testing.T) { } initializeClients = func(ctx context.Context, clientset *kubernetes.Clientset, parameters map[string]string) (*s3cli.S3Agent, *rgwadmin.API, error) { + _, _, err := fetchSecretNameAndNamespace(parameters) + if err != nil { + t.Fatalf("failed to fetch secret name and namespace: %v", err) + } s3Client := &s3cli.S3Agent{ Client: mockS3Client{}, } @@ -233,10 +252,7 @@ func Test_provisionerServer_DriverDeleteBucket(t *testing.T) { }, Spec: v1alpha1.BucketSpec{ DriverName: tt.fields.provisioner, - Parameters: map[string]string{ - "ObjectStoreUserSecretName": "test-user-secret", - "ObjectStoreUserSecretNamespace": "test-namespace", - }, + Parameters: createParameters(), }, } bucketClient := fakebucketclientset.NewSimpleClientset(&b) @@ -266,6 +282,10 @@ func Test_provisonerServer_DriverRevokeBucketAccess(t *testing.T) { } initializeClients = func(ctx context.Context, clientset *kubernetes.Clientset, parameters map[string]string) (*s3cli.S3Agent, *rgwadmin.API, error) { + _, _, err := fetchSecretNameAndNamespace(parameters) + if err != nil { + t.Fatalf("failed to fetch secret name and namespace: %v", err) + } s3Client := &s3cli.S3Agent{ Client: mockS3Client{}, } @@ -310,10 +330,7 @@ func Test_provisonerServer_DriverRevokeBucketAccess(t *testing.T) { }, Spec: v1alpha1.BucketSpec{ DriverName: tt.fields.provisioner, - Parameters: map[string]string{ - "ObjectStoreUserSecretName": "test-user-secret", - "ObjectStoreUserSecretNamespace": "test-namespace", - }, + Parameters: createParameters(), }, } bucketClient := fakebucketclientset.NewSimpleClientset(&b)