refactor(plugin): Split getStorageAttributes into attributes and secrets

plugin/service/storage/attributes.go

@@ -22,12 +22,6 @@ const (
 )
 
 type StorageAttributes struct {
-	// AccessKeyId is the MinIO Access Key Id. This field is required and comes
-	// from user input.
-	AccessKeyId string
-	// SecretAccessKey is the MinIO Secret Access Key. This field is required
-	// and comes from user input.
-	SecretAccessKey string
 	// EndpointUrl is the MinIO server URL. This field is required and comes
 	// from user input.
 	EndpointUrl string
@@ -39,20 +33,27 @@ type StorageAttributes struct {
 	UseSSL bool
 }
 
-// ToMap returns a map StorageAttributes's secret fields as a map.
-func (sa *StorageAttributes) SecretsToMap() map[string]any {
+type StorageSecrets struct {
+	// AccessKeyId is the MinIO Access Key Id. This field is required and comes
+	// from user input.
+	AccessKeyId string
+	// SecretAccessKey is the MinIO Secret Access Key. This field is required
+	// and comes from user input.
+	SecretAccessKey string
+}
+
+// AsMap returns a map StorageAttributes's secret fields as a map.
+func (sa *StorageSecrets) AsMap() map[string]any {
 	return map[string]any{
 		ConstAccessKeyId:     sa.AccessKeyId,
 		ConstSecretAccessKey: sa.SecretAccessKey,
 	}
 }
 
-func getStorageAttributes(inAttributes *structpb.Struct, inSecrets *structpb.Struct) (*StorageAttributes, error) {
+// getStorageAttributes accepts a *structpb.Struct and returns a valid StorageAttributes struct or a status error
+func getStorageAttributes(inAttributes *structpb.Struct) (*StorageAttributes, error) {
 	if inAttributes == nil || inAttributes.GetFields() == nil || len(inAttributes.GetFields()) == 0 {
-		return nil, status.Error(codes.InvalidArgument, "empty attributes input")
-	}
-	if inSecrets == nil || inSecrets.GetFields() == nil || len(inSecrets.GetFields()) == 0 {
-		return nil, status.Error(codes.InvalidArgument, "empty secrets input")
+		return nil, status.Errorf(codes.InvalidArgument, "empty attributes input")
 	}
 
 	badFields := make(map[string]string)
@@ -86,6 +87,24 @@ func getStorageAttributes(inAttributes *structpb.Struct, inSecrets *structpb.Str
 		badFields[fmt.Sprintf("attributes.%s", s)] = "unrecognized field"
 	}
 
+	if len(badFields) > 0 {
+		return nil, errors.InvalidArgumentError("invalid or unrecognized attributes found", badFields)
+	}
+	return &StorageAttributes{
+		EndpointUrl: endpointUrl,
+		Region:      region,
+		UseSSL:      useSSL,
+	}, nil
+}
+
+// getStorageSecrets accepts a *structpb.Struct and returns a valid StorageSecrets struct or a status error
+func getStorageSecrets(inSecrets *structpb.Struct) (*StorageSecrets, error) {
+	if inSecrets == nil || inSecrets.GetFields() == nil || len(inSecrets.GetFields()) == 0 {
+		return nil, status.Errorf(codes.InvalidArgument, "empty secrets input")
+	}
+
+	badFields := make(map[string]string)
+
 	unknownSecretFields := values.StructFields(inSecrets)
 	accessKeyId, err := values.GetStringValue(inSecrets, ConstAccessKeyId, true)
 	if err != nil {
@@ -104,13 +123,10 @@ func getStorageAttributes(inAttributes *structpb.Struct, inSecrets *structpb.Str
 	}
 
 	if len(badFields) > 0 {
-		return nil, errors.InvalidArgumentError("invalid or unrecognized attributes/secrets found", badFields)
+		return nil, errors.InvalidArgumentError("invalid or unrecognized secrets found", badFields)
 	}
-	return &StorageAttributes{
+	return &StorageSecrets{
 		AccessKeyId:     accessKeyId,
 		SecretAccessKey: secretAccessKey,
-		EndpointUrl:     endpointUrl,
-		Region:          region,
-		UseSSL:          useSSL,
 	}, nil
 }

plugin/service/storage/attributes_test.go

@@ -14,84 +14,23 @@ func TestGetStorageAttributes(t *testing.T) {
 	tests := []struct {
 		name                 string
 		inAttributes         *structpb.Struct
-		inSecrets            *structpb.Struct
 		expStorageAttributes *StorageAttributes
 		expErrMsg            string
 	}{
 		{
 			name:         "nilAttributes",
 			inAttributes: nil,
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					"foo": structpb.NewStringValue("bar"),
-				},
-			},
-			expErrMsg: "empty attributes input",
+			expErrMsg:    "empty attributes input",
 		},
 		{
 			name:         "nilAttributesFields",
 			inAttributes: &structpb.Struct{Fields: nil},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					"foo": structpb.NewStringValue("bar"),
-				},
-			},
-			expErrMsg: "empty attributes input",
+			expErrMsg:    "empty attributes input",
 		},
 		{
 			name:         "emptyAttributesFields",
 			inAttributes: &structpb.Struct{Fields: map[string]*structpb.Value{}},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					"foo": structpb.NewStringValue("bar"),
-				},
-			},
-			expErrMsg: "empty attributes input",
-		},
-		{
-			name: "nilSecrets",
-			inAttributes: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					"foo": structpb.NewStringValue("bar"),
-				},
-			},
-			inSecrets: nil,
-			expErrMsg: "empty secrets input",
-		},
-		{
-			name: "nilSecretsFields",
-			inAttributes: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					"foo": structpb.NewStringValue("bar"),
-				},
-			},
-			inSecrets: &structpb.Struct{Fields: nil},
-			expErrMsg: "empty secrets input",
-		},
-		{
-			name: "emptySecretsFields",
-			inAttributes: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					"foo": structpb.NewStringValue("bar"),
-				},
-			},
-			inSecrets: &structpb.Struct{Fields: map[string]*structpb.Value{}},
-			expErrMsg: "empty secrets input",
-		},
-		{
-			name: "noEndpointUrlAttribute",
-			inAttributes: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					"foo": structpb.NewStringValue("bar"),
-				},
-			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewStringValue("bar"),
-					ConstSecretAccessKey: structpb.NewStringValue("bar"),
-				},
-			},
-			expErrMsg: "attributes.endpoint_url: missing required value \"endpoint_url\"",
+			expErrMsg:    "empty attributes input",
 		},
 		{
 			name: "emptyEndpointUrlAttribute",
@@ -100,12 +39,6 @@ func TestGetStorageAttributes(t *testing.T) {
 					ConstEndpointUrl: structpb.NewStringValue(""),
 				},
 			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewStringValue("bar"),
-					ConstSecretAccessKey: structpb.NewStringValue("bar"),
-				},
-			},
 			expErrMsg: "attributes.endpoint_url: value \"endpoint_url\" cannot be empty",
 		},
 		{
@@ -115,12 +48,6 @@ func TestGetStorageAttributes(t *testing.T) {
 					ConstEndpointUrl: structpb.NewBoolValue(true),
 				},
 			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewStringValue("bar"),
-					ConstSecretAccessKey: structpb.NewStringValue("bar"),
-				},
-			},
 			expErrMsg: "attributes.endpoint_url: unexpected type for value \"endpoint_url\": want string, got bool",
 		},
 		{
@@ -130,12 +57,6 @@ func TestGetStorageAttributes(t *testing.T) {
 					ConstEndpointUrl: structpb.NewStringValue("bad://foo.bar"),
 				},
 			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewStringValue("bar"),
-					ConstSecretAccessKey: structpb.NewStringValue("bar"),
-				},
-			},
 			expErrMsg: "attributes.endpoint_url.format: unknown protocol, should be http:// or https://",
 		},
 		{
@@ -146,12 +67,6 @@ func TestGetStorageAttributes(t *testing.T) {
 					ConstRegion:      structpb.NewBoolValue(true),
 				},
 			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewStringValue("bar"),
-					ConstSecretAccessKey: structpb.NewStringValue("bar"),
-				},
-			},
 			expErrMsg: "attributes.region: unexpected type for value \"region\": want string, got bool",
 		},
 		{
@@ -163,124 +78,16 @@ func TestGetStorageAttributes(t *testing.T) {
 					"foo":            structpb.NewStringValue("bar"),
 				},
 			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewStringValue("bar"),
-					ConstSecretAccessKey: structpb.NewStringValue("bar"),
-				},
-			},
 			expErrMsg: "attributes.foo: unrecognized field",
 		},
 		{
-			name: "noAccessKeyIdSecret",
+			name: "noEndpointUrlAttribute",
 			inAttributes: &structpb.Struct{
 				Fields: map[string]*structpb.Value{
-					ConstEndpointUrl: structpb.NewStringValue("http://foo.bar"),
-					ConstRegion:      structpb.NewStringValue("us-east-1"),
+					"foo": structpb.NewStringValue("bar"),
 				},
 			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstSecretAccessKey: structpb.NewStringValue("bar"),
-				},
-			},
-			expErrMsg: "secrets.access_key_id: missing required value \"access_key_id\"",
-		},
-		{
-			name: "emptyAccessKeyIdSecret",
-			inAttributes: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstEndpointUrl: structpb.NewStringValue("http://foo.bar"),
-					ConstRegion:      structpb.NewStringValue("us-east-1"),
-				},
-			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewStringValue(""),
-					ConstSecretAccessKey: structpb.NewStringValue("bar"),
-				},
-			},
-			expErrMsg: "secrets.access_key_id: value \"access_key_id\" cannot be empty",
-		},
-		{
-			name: "accessKeyIdSecretBadType",
-			inAttributes: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstEndpointUrl: structpb.NewStringValue("http://foo.bar"),
-					ConstRegion:      structpb.NewStringValue("us-east-1"),
-				},
-			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewBoolValue(true),
-					ConstSecretAccessKey: structpb.NewStringValue("bar"),
-				},
-			},
-			expErrMsg: "secrets.access_key_id: unexpected type for value \"access_key_id\": want string, got bool",
-		},
-		{
-			name: "noSecretAccessKeySecret",
-			inAttributes: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstEndpointUrl: structpb.NewStringValue("http://foo.bar"),
-					ConstRegion:      structpb.NewStringValue("us-east-1"),
-				},
-			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId: structpb.NewStringValue("bar"),
-				},
-			},
-			expErrMsg: "secrets.secret_access_key: missing required value \"secret_access_key\"",
-		},
-		{
-			name: "emptySecretAccessKeySecret",
-			inAttributes: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstEndpointUrl: structpb.NewStringValue("http://foo.bar"),
-					ConstRegion:      structpb.NewStringValue("us-east-1"),
-				},
-			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewStringValue("bar"),
-					ConstSecretAccessKey: structpb.NewStringValue(""),
-				},
-			},
-			expErrMsg: "secrets.secret_access_key: value \"secret_access_key\" cannot be empty",
-		},
-		{
-			name: "secretAccessKeySecretBadType",
-			inAttributes: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstEndpointUrl: structpb.NewStringValue("http://foo.bar"),
-					ConstRegion:      structpb.NewStringValue("us-east-1"),
-				},
-			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewStringValue("bar"),
-					ConstSecretAccessKey: structpb.NewBoolValue(true),
-				},
-			},
-			expErrMsg: "secrets.secret_access_key: unexpected type for value \"secret_access_key\": want string, got bool",
-		},
-		{
-			name: "unknownSecret",
-			inAttributes: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstEndpointUrl: structpb.NewStringValue("http://foo.bar"),
-					ConstRegion:      structpb.NewStringValue("us-east-1"),
-				},
-			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewStringValue("bar"),
-					ConstSecretAccessKey: structpb.NewStringValue("bar"),
-					"foo":                structpb.NewStringValue("bar"),
-				},
-			},
-			expErrMsg: "secrets.foo: unrecognized field",
+			expErrMsg: "attributes.endpoint_url: missing required value \"endpoint_url\"",
 		},
 		{
 			name: "successWithNoSSL",
@@ -290,18 +97,10 @@ func TestGetStorageAttributes(t *testing.T) {
 					ConstRegion:      structpb.NewStringValue("us-east-1"),
 				},
 			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewStringValue("foo"),
-					ConstSecretAccessKey: structpb.NewStringValue("bar"),
-				},
-			},
 			expStorageAttributes: &StorageAttributes{
-				AccessKeyId:     "foo",
-				SecretAccessKey: "bar",
-				EndpointUrl:     "foo.bar",
-				Region:          "us-east-1",
-				UseSSL:          false,
+				EndpointUrl: "foo.bar",
+				Region:      "us-east-1",
+				UseSSL:      false,
 			},
 		},
 		{
@@ -312,25 +111,17 @@ func TestGetStorageAttributes(t *testing.T) {
 					ConstRegion:      structpb.NewStringValue("us-east-1"),
 				},
 			},
-			inSecrets: &structpb.Struct{
-				Fields: map[string]*structpb.Value{
-					ConstAccessKeyId:     structpb.NewStringValue("foo"),
-					ConstSecretAccessKey: structpb.NewStringValue("bar"),
-				},
-			},
 			expStorageAttributes: &StorageAttributes{
-				AccessKeyId:     "foo",
-				SecretAccessKey: "bar",
-				EndpointUrl:     "foo.bar",
-				Region:          "us-east-1",
-				UseSSL:          true,
+				EndpointUrl: "foo.bar",
+				Region:      "us-east-1",
+				UseSSL:      true,
 			},
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			sa, err := getStorageAttributes(tt.inAttributes, tt.inSecrets)
+			sa, err := getStorageAttributes(tt.inAttributes)
 			if tt.expErrMsg != "" {
 				require.ErrorContains(t, err, tt.expErrMsg)
 				require.Nil(t, sa)
@@ -344,20 +135,139 @@ func TestGetStorageAttributes(t *testing.T) {
 	}
 }
 
-func TestStorageAttributesSecretsToMap(t *testing.T) {
+func TestGetStorageSecrets(t *testing.T) {
+	tests := []struct {
+		name              string
+		inSecrets         *structpb.Struct
+		expStorageSecrets *StorageSecrets
+		expErrMsg         string
+	}{
+		{
+			name:      "nilSecrets",
+			inSecrets: nil,
+			expErrMsg: "empty secrets input",
+		},
+		{
+			name:      "nilSecretsFields",
+			inSecrets: &structpb.Struct{Fields: nil},
+			expErrMsg: "empty secrets input",
+		},
+		{
+			name:      "emptySecretsFields",
+			inSecrets: &structpb.Struct{Fields: map[string]*structpb.Value{}},
+			expErrMsg: "empty secrets input",
+		},
+		{
+			name: "noAccessKeyIdSecret",
+			inSecrets: &structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					ConstSecretAccessKey: structpb.NewStringValue("bar"),
+				},
+			},
+			expErrMsg: "secrets.access_key_id: missing required value \"access_key_id\"",
+		},
+		{
+			name: "emptyAccessKeyIdSecret",
+			inSecrets: &structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					ConstAccessKeyId:     structpb.NewStringValue(""),
+					ConstSecretAccessKey: structpb.NewStringValue("bar"),
+				},
+			},
+			expErrMsg: "secrets.access_key_id: value \"access_key_id\" cannot be empty",
+		},
+		{
+			name: "accessKeyIdSecretBadType",
+			inSecrets: &structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					ConstAccessKeyId:     structpb.NewBoolValue(true),
+					ConstSecretAccessKey: structpb.NewStringValue("bar"),
+				},
+			},
+			expErrMsg: "secrets.access_key_id: unexpected type for value \"access_key_id\": want string, got bool",
+		},
+		{
+			name: "noSecretAccessKeySecret",
+			inSecrets: &structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					ConstAccessKeyId: structpb.NewStringValue("bar"),
+				},
+			},
+			expErrMsg: "secrets.secret_access_key: missing required value \"secret_access_key\"",
+		},
+		{
+			name: "emptySecretAccessKeySecret",
+			inSecrets: &structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					ConstAccessKeyId:     structpb.NewStringValue("bar"),
+					ConstSecretAccessKey: structpb.NewStringValue(""),
+				},
+			},
+			expErrMsg: "secrets.secret_access_key: value \"secret_access_key\" cannot be empty",
+		},
+		{
+			name: "secretAccessKeySecretBadType",
+			inSecrets: &structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					ConstAccessKeyId:     structpb.NewStringValue("bar"),
+					ConstSecretAccessKey: structpb.NewBoolValue(true),
+				},
+			},
+			expErrMsg: "secrets.secret_access_key: unexpected type for value \"secret_access_key\": want string, got bool",
+		},
+		{
+			name: "unknownSecret",
+			inSecrets: &structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					ConstAccessKeyId:     structpb.NewStringValue("bar"),
+					ConstSecretAccessKey: structpb.NewStringValue("bar"),
+					"foo":                structpb.NewStringValue("bar"),
+				},
+			},
+			expErrMsg: "secrets.foo: unrecognized field",
+		},
+		{
+			name: "success",
+			inSecrets: &structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					ConstAccessKeyId:     structpb.NewStringValue("foo"),
+					ConstSecretAccessKey: structpb.NewStringValue("bar"),
+				},
+			},
+			expStorageSecrets: &StorageSecrets{
+				AccessKeyId:     "foo",
+				SecretAccessKey: "bar",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			sa, err := getStorageSecrets(tt.inSecrets)
+			if tt.expErrMsg != "" {
+				require.ErrorContains(t, err, tt.expErrMsg)
+				require.Nil(t, sa)
+				return
+			}
+
+			require.NoError(t, err)
+			require.NotNil(t, sa)
+			require.EqualValues(t, tt.expStorageSecrets, sa)
+		})
+	}
+}
+
+func TestStorageSecretsAsMap(t *testing.T) {
 	tests := []struct {
 		name   string
-		in     *StorageAttributes
+		in     *StorageSecrets
 		expMap map[string]any
 	}{
 		{
 			name: "completeObject",
-			in: &StorageAttributes{
+			in: &StorageSecrets{
 				AccessKeyId:     "access_key_id_value",
 				SecretAccessKey: "secret_access_key_value",
-				EndpointUrl:     "endpoint_url_value",
-				Region:          "region_value",
-				UseSSL:          true,
 			},
 			expMap: map[string]any{
 				"access_key_id":     "access_key_id_value",
@@ -368,7 +278,7 @@ func TestStorageAttributesSecretsToMap(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			m := tt.in.SecretsToMap()
+			m := tt.in.AsMap()
 			require.EqualValues(t, tt.expMap, m)
 		})
 	}

plugin/service/storage/plugin.go

@@ -38,18 +38,23 @@ func (sp *StoragePlugin) OnCreateStorageBucket(ctx context.Context, req *pb.OnCr
 		return nil, status.Error(codes.InvalidArgument, "storage bucket name is required")
 	}
 
-	sa, err := getStorageAttributes(bucket.GetAttributes(), bucket.GetSecrets())
+	sa, err := getStorageAttributes(bucket.GetAttributes())
 	if err != nil {
 		return nil, err // getStorageAttributes already returns a "status"-type error.
 	}
 
-	err = ensureServiceAccount(ctx, sa)
+	sec, err := getStorageSecrets(bucket.GetSecrets())
+	if err != nil {
+		return nil, err
+	}
+
+	err = ensureServiceAccount(ctx, sa, sec)
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "failed to ensure service account: %v", err)
 	}
 
 	cl, err := minio.New(sa.EndpointUrl, &minio.Options{
-		Creds:  credentials.NewStaticV4(sa.AccessKeyId, sa.SecretAccessKey, ""),
+		Creds:  credentials.NewStaticV4(sec.AccessKeyId, sec.SecretAccessKey, ""),
 		Secure: sa.UseSSL,
 		Region: sa.Region,
 	})
@@ -62,7 +67,7 @@ func (sp *StoragePlugin) OnCreateStorageBucket(ctx context.Context, req *pb.OnCr
 		return nil, status.Errorf(codes.Unknown, "failed to verify provided minio environment: %v", err)
 	}
 
-	persistedData, err := structpb.NewStruct(sa.SecretsToMap())
+	persistedData, err := structpb.NewStruct(sec.AsMap())
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to convert storage attributes to protobuf struct: %v", err)
 	}
@@ -92,18 +97,23 @@ func (sp *StoragePlugin) ValidatePermissions(ctx context.Context, req *pb.Valida
 		return nil, status.Error(codes.InvalidArgument, "storage bucket name is required")
 	}
 
-	sa, err := getStorageAttributes(bucket.GetAttributes(), bucket.GetSecrets())
+	sa, err := getStorageAttributes(bucket.GetAttributes())
 	if err != nil {
 		return nil, err // getStorageAttributes already returns a "status"-type error.
 	}
 
-	err = ensureServiceAccount(ctx, sa)
+	sec, err := getStorageSecrets(bucket.GetSecrets())
+	if err != nil {
+		return nil, err
+	}
+
+	err = ensureServiceAccount(ctx, sa, sec)
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "failed to ensure service account: %v", err)
 	}
 
 	cl, err := minio.New(sa.EndpointUrl, &minio.Options{
-		Creds:  credentials.NewStaticV4(sa.AccessKeyId, sa.SecretAccessKey, ""),
+		Creds:  credentials.NewStaticV4(sec.AccessKeyId, sec.SecretAccessKey, ""),
 		Secure: sa.UseSSL,
 		Region: sa.Region,
 	})
@@ -174,20 +184,23 @@ func dryRun(ctx context.Context, cl *minio.Client, bucket *storagebuckets.Storag
 	return nil
 }
 
+func newMadminClient(sa *StorageAttributes, sec *StorageSecrets) (*madmin.AdminClient, error) {
+	return madmin.NewWithOptions(sa.EndpointUrl, &madmin.Options{
+		Creds:  credentials.NewStaticV4(sec.AccessKeyId, sec.SecretAccessKey, ""),
+		Secure: sa.UseSSL,
+	})
+}
+
 // ensureServiceAccount ensures the credentials we received belong to a MinIO
 // service account. This plugin does not support using user credentials for its
 // configuration.
-func ensureServiceAccount(ctx context.Context, sa *StorageAttributes) error {
-	cl, err := madmin.NewWithOptions(sa.EndpointUrl, &madmin.Options{
-		Creds:  credentials.NewStaticV4(sa.AccessKeyId, sa.SecretAccessKey, ""),
-		Secure: sa.UseSSL,
-	})
+func ensureServiceAccount(ctx context.Context, sa *StorageAttributes, sec *StorageSecrets) error {
+	cl, err := newMadminClient(sa, sec)
 	if err != nil {
 		return fmt.Errorf("failed to create madmin client: %w", err)
 	}
 
-	_, err = cl.InfoServiceAccount(ctx, sa.AccessKeyId)
-	if err != nil {
+	if _, err = cl.InfoServiceAccount(ctx, sec.AccessKeyId); err != nil {
 		return fmt.Errorf("failed to obtain service account info: %w", err)
 	}
 

plugin/service/storage/plugin_test.go

@@ -153,6 +153,20 @@ func TestOnCreateStorageBucket(t *testing.T) {
 			},
 			expErrMsg: "empty attributes input",
 		},
+		{
+			name: "badStorageSecrets",
+			req: &plugin.OnCreateStorageBucketRequest{
+				Bucket: &storagebuckets.StorageBucket{
+					BucketName: bucketName,
+					Attributes: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstEndpointUrl: structpb.NewStringValue("http://foo"),
+						},
+					},
+				},
+			},
+			expErrMsg: "empty secrets input",
+		},
 		{
 			name: "usingNonServiceAccount",
 			req: &plugin.OnCreateStorageBucketRequest{
@@ -289,6 +303,20 @@ func TestValidatePermissions(t *testing.T) {
 			},
 			expErrMsg: "empty attributes input",
 		},
+		{
+			name: "badStorageSecrets",
+			req: &plugin.ValidatePermissionsRequest{
+				Bucket: &storagebuckets.StorageBucket{
+					BucketName: bucketName,
+					Attributes: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstEndpointUrl: structpb.NewStringValue("http://foo"),
+						},
+					},
+				},
+			},
+			expErrMsg: "empty secrets input",
+		},
 		{
 			name: "usingNonServiceAccount",
 			req: &plugin.ValidatePermissionsRequest{
@@ -547,30 +575,32 @@ func TestDryRun(t *testing.T) {
 func TestEnsureServiceAccount(t *testing.T) {
 	tests := []struct {
 		name      string
-		saFunc    func(s *internaltest.MinioServer) *StorageAttributes
+		saFunc    func(s *internaltest.MinioServer) (*StorageAttributes, *StorageSecrets)
 		expErrMsg string
 	}{
 		{
 			name: "nonServiceAccount",
-			saFunc: func(s *internaltest.MinioServer) *StorageAttributes {
+			saFunc: func(s *internaltest.MinioServer) (*StorageAttributes, *StorageSecrets) {
 				return &StorageAttributes{
-					AccessKeyId:     s.RootUsername,
-					SecretAccessKey: s.RootPassword,
-					EndpointUrl:     s.ApiAddr,
-					UseSSL:          false,
-				}
+						EndpointUrl: s.ApiAddr,
+						UseSSL:      false,
+					}, &StorageSecrets{
+						AccessKeyId:     s.RootUsername,
+						SecretAccessKey: s.RootPassword,
+					}
 			},
 			expErrMsg: "The specified service account is not found (Specified service account does not exist)",
 		},
 		{
 			name: "serviceAccount",
-			saFunc: func(s *internaltest.MinioServer) *StorageAttributes {
+			saFunc: func(s *internaltest.MinioServer) (*StorageAttributes, *StorageSecrets) {
 				return &StorageAttributes{
-					AccessKeyId:     s.ServiceAccountAccessKeyId,
-					SecretAccessKey: s.ServiceAccountSecretAccessKey,
-					EndpointUrl:     s.ApiAddr,
-					UseSSL:          false,
-				}
+						EndpointUrl: s.ApiAddr,
+						UseSSL:      false,
+					}, &StorageSecrets{
+						AccessKeyId:     s.ServiceAccountAccessKeyId,
+						SecretAccessKey: s.ServiceAccountSecretAccessKey,
+					}
 			},
 		},
 	}
@@ -578,7 +608,8 @@ func TestEnsureServiceAccount(t *testing.T) {
 	server := internaltest.NewMinioServer(t)
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			err := ensureServiceAccount(context.Background(), tt.saFunc(server))
+			sa, sec := tt.saFunc(server)
+			err := ensureServiceAccount(context.Background(), sa, sec)
 			if tt.expErrMsg != "" {
 				require.ErrorContains(t, err, tt.expErrMsg)
 				return