diff --git a/plugin/service/storage/plugin.go b/plugin/service/storage/plugin.go index 18efb17..24ff6ef 100644 --- a/plugin/service/storage/plugin.go +++ b/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{ diff --git a/plugin/service/storage/plugin_test.go b/plugin/service/storage/plugin_test.go index a2ac7a6..c722e9b 100644 --- a/plugin/service/storage/plugin_test.go +++ b/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{