move service account validation to only check for service account on credential rotation. should allow other S3-compatible providers (#16)

plugin/service/storage/plugin.go

@@ -60,11 +60,6 @@ func (sp *StoragePlugin) OnCreateStorageBucket(ctx context.Context, req *pb.OnCr
 		return nil, err
 	}
 
-	err = ensureServiceAccount(ctx, sa, sec)
-	if err != nil {
-		return nil, status.Errorf(codes.InvalidArgument, "failed to ensure service account: %v", err)
-	}
-
 	deleteOldCredsFn := func() error { return nil }
 	if !sa.DisableCredentialRotation {
 		newSec, fn, err := rotateCredentials(ctx, bucket.GetBucketName(), sa, sec)
@@ -178,11 +173,6 @@ func (sp *StoragePlugin) OnUpdateStorageBucket(ctx context.Context, req *pb.OnUp
 			return nil, err
 		}
 
-		err = ensureServiceAccount(ctx, nsa, newSec)
-		if err != nil {
-			return nil, status.Errorf(codes.InvalidArgument, "failed to ensure service account: %s", err)
-		}
-
 		if !nsa.DisableCredentialRotation {
 			// We have new incoming credentials and we're also rotating,
 			// therefore we use incoming credentials to create a new service
@@ -292,9 +282,13 @@ func (sp *StoragePlugin) ValidatePermissions(ctx context.Context, req *pb.Valida
 		return nil, err
 	}
 
-	err = ensureServiceAccount(ctx, sa, sec)
-	if err != nil {
-		return nil, status.Errorf(codes.InvalidArgument, "failed to ensure service account: %v", err)
+	if !sec.LastRotatedTime.IsZero() {
+		// since these are existing secrets, we only want to validate the
+		// service account if the credentials have been rotated previously
+		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{

plugin/service/storage/plugin_test.go

@@ -234,26 +234,6 @@ func TestOnCreateStorageBucket(t *testing.T) {
 			},
 			expErrMsg: "empty secrets input",
 		},
-		{
-			name: "usingNonServiceAccount",
-			req: &plugin.OnCreateStorageBucketRequest{
-				Bucket: &storagebuckets.StorageBucket{
-					BucketName: bucketName,
-					Attributes: &structpb.Struct{
-						Fields: map[string]*structpb.Value{
-							ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr),
-						},
-					},
-					Secrets: &structpb.Struct{
-						Fields: map[string]*structpb.Value{
-							ConstAccessKeyId:     structpb.NewStringValue(server.RootUsername),
-							ConstSecretAccessKey: structpb.NewStringValue(server.RootPassword),
-						},
-					},
-				},
-			},
-			expErrMsg: "The specified service account is not found (Specified service account does not exist)",
-		},
 		{
 			name: "dryRunFail",
 			req: &plugin.OnCreateStorageBucketRequest{
@@ -323,6 +303,59 @@ func TestOnCreateStorageBucket(t *testing.T) {
 			}(),
 			expErrMsg: "failed to rotate minio credentials: failed to create new minio service account: Access Denied",
 		},
+		{
+			name: "credRotationFailNoSvcAcct",
+			req: func() *plugin.OnCreateStorageBucketRequest {
+				return &plugin.OnCreateStorageBucketRequest{
+					Bucket: &storagebuckets.StorageBucket{
+						BucketName: bucketName,
+						Attributes: &structpb.Struct{
+							Fields: map[string]*structpb.Value{
+								ConstEndpointUrl:               structpb.NewStringValue("http://" + server.ApiAddr),
+								ConstDisableCredentialRotation: structpb.NewBoolValue(false),
+							},
+						},
+						Secrets: &structpb.Struct{
+							Fields: map[string]*structpb.Value{
+								ConstAccessKeyId:     structpb.NewStringValue(server.RootUsername),
+								ConstSecretAccessKey: structpb.NewStringValue(server.RootPassword),
+							},
+						},
+					},
+				}
+			}(),
+			expErrMsg: "The specified service account is not found (Specified service account does not exist)",
+		},
+		{
+			name: "successNonServiceAccount",
+			req: &plugin.OnCreateStorageBucketRequest{
+				Bucket: &storagebuckets.StorageBucket{
+					BucketName: bucketName,
+					Attributes: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstEndpointUrl:               structpb.NewStringValue("http://" + server.ApiAddr),
+							ConstDisableCredentialRotation: structpb.NewBoolValue(true),
+						},
+					},
+					Secrets: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstAccessKeyId:     structpb.NewStringValue(server.RootUsername),
+							ConstSecretAccessKey: structpb.NewStringValue(server.RootPassword),
+						},
+					},
+				},
+			},
+			expRsp: &plugin.OnCreateStorageBucketResponse{
+				Persisted: &storagebuckets.StorageBucketPersisted{
+					Data: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstAccessKeyId:     structpb.NewStringValue(server.RootUsername),
+							ConstSecretAccessKey: structpb.NewStringValue(server.RootPassword),
+						},
+					},
+				},
+			},
+		},
 		{
 			name: "successNoRotation",
 			req: &plugin.OnCreateStorageBucketRequest{
@@ -764,43 +797,6 @@ func TestOnUpdateStorageBucket(t *testing.T) {
 			err:     "cannot disable rotation for already-rotated credentials",
 			errCode: codes.InvalidArgument,
 		},
-		{
-			name: "usingNonServiceAccount",
-			req: &plugin.OnUpdateStorageBucketRequest{
-				NewBucket: &storagebuckets.StorageBucket{
-					BucketName: bucketName,
-					Attributes: &structpb.Struct{
-						Fields: map[string]*structpb.Value{
-							ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr),
-						},
-					},
-					Secrets: &structpb.Struct{
-						Fields: map[string]*structpb.Value{
-							ConstAccessKeyId:     structpb.NewStringValue(server.RootUsername),
-							ConstSecretAccessKey: structpb.NewStringValue(server.RootPassword),
-						},
-					},
-				},
-				CurrentBucket: &storagebuckets.StorageBucket{
-					BucketName: bucketName,
-					Attributes: &structpb.Struct{
-						Fields: map[string]*structpb.Value{
-							ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr),
-						},
-					},
-				},
-				Persisted: &storagebuckets.StorageBucketPersisted{
-					Data: &structpb.Struct{
-						Fields: map[string]*structpb.Value{
-							ConstAccessKeyId:     structpb.NewStringValue(server.RootUsername),
-							ConstSecretAccessKey: structpb.NewStringValue(server.RootPassword),
-						},
-					},
-				},
-			},
-			err:     "The specified service account is not found (Specified service account does not exist)",
-			errCode: codes.InvalidArgument,
-		},
 		{
 			name: "dryRunFail",
 			req: &plugin.OnUpdateStorageBucketRequest{
@@ -939,6 +935,52 @@ func TestOnUpdateStorageBucket(t *testing.T) {
 			err:     "credential rotation enabled with no new credentials provided",
 			errCode: codes.InvalidArgument,
 		},
+		{
+			name: "successUsingNonServiceAccount",
+			req: &plugin.OnUpdateStorageBucketRequest{
+				NewBucket: &storagebuckets.StorageBucket{
+					BucketName: bucketName,
+					Attributes: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstEndpointUrl:               structpb.NewStringValue("http://" + server.ApiAddr),
+							ConstDisableCredentialRotation: structpb.NewBoolValue(true),
+						},
+					},
+					Secrets: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstAccessKeyId:     structpb.NewStringValue(server.RootUsername),
+							ConstSecretAccessKey: structpb.NewStringValue(server.RootPassword),
+						},
+					},
+				},
+				CurrentBucket: &storagebuckets.StorageBucket{
+					BucketName: bucketName,
+					Attributes: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr),
+						},
+					},
+				},
+				Persisted: &storagebuckets.StorageBucketPersisted{
+					Data: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstAccessKeyId:     structpb.NewStringValue(server.RootUsername),
+							ConstSecretAccessKey: structpb.NewStringValue(server.RootPassword),
+						},
+					},
+				},
+			},
+			expected: &plugin.OnUpdateStorageBucketResponse{
+				Persisted: &storagebuckets.StorageBucketPersisted{
+					Data: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstAccessKeyId:     structpb.NewStringValue(server.RootUsername),
+							ConstSecretAccessKey: structpb.NewStringValue(server.RootPassword),
+						},
+					},
+				},
+			},
+		},
 		{
 			name: "successNoRotation",
 			req: &plugin.OnUpdateStorageBucketRequest{
@@ -1644,26 +1686,6 @@ func TestValidatePermissions(t *testing.T) {
 			},
 			expErrMsg: "empty secrets input",
 		},
-		{
-			name: "usingNonServiceAccount",
-			req: &plugin.ValidatePermissionsRequest{
-				Bucket: &storagebuckets.StorageBucket{
-					BucketName: bucketName,
-					Attributes: &structpb.Struct{
-						Fields: map[string]*structpb.Value{
-							ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr),
-						},
-					},
-					Secrets: &structpb.Struct{
-						Fields: map[string]*structpb.Value{
-							ConstAccessKeyId:     structpb.NewStringValue(server.RootUsername),
-							ConstSecretAccessKey: structpb.NewStringValue(server.RootPassword),
-						},
-					},
-				},
-			},
-			expErrMsg: "The specified service account is not found (Specified service account does not exist)",
-		},
 		{
 			name: "dryRunFail",
 			req: &plugin.ValidatePermissionsRequest{
@@ -1691,6 +1713,46 @@ func TestValidatePermissions(t *testing.T) {
 			},
 			expErrMsg: "failed to verify provided minio environment: failed to put object",
 		},
+		{
+			name: "rotatedNonServiceAccount",
+			req: &plugin.ValidatePermissionsRequest{
+				Bucket: &storagebuckets.StorageBucket{
+					BucketName: bucketName,
+					Attributes: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr),
+						},
+					},
+					Secrets: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstAccessKeyId:     structpb.NewStringValue(server.RootUsername),
+							ConstSecretAccessKey: structpb.NewStringValue(server.RootPassword),
+							ConstLastRotatedTime: structpb.NewStringValue(time.Now().Format(time.RFC3339Nano)),
+						},
+					},
+				},
+			},
+			expErrMsg: "The specified service account is not found (Specified service account does not exist)",
+		},
+		{
+			name: "successNonServiceAccount",
+			req: &plugin.ValidatePermissionsRequest{
+				Bucket: &storagebuckets.StorageBucket{
+					BucketName: bucketName,
+					Attributes: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr),
+						},
+					},
+					Secrets: &structpb.Struct{
+						Fields: map[string]*structpb.Value{
+							ConstAccessKeyId:     structpb.NewStringValue(server.RootUsername),
+							ConstSecretAccessKey: structpb.NewStringValue(server.RootPassword),
+						},
+					},
+				},
+			},
+		},
 		{
 			name: "success",
 			req: &plugin.ValidatePermissionsRequest{