diff --git a/plugin/service/storage/plugin.go b/plugin/service/storage/plugin.go index 333d0b6..911c06a 100644 --- a/plugin/service/storage/plugin.go +++ b/plugin/service/storage/plugin.go @@ -78,7 +78,71 @@ func (sp *StoragePlugin) OnCreateStorageBucket(ctx context.Context, req *pb.OnCr // OnUpdateStorageBucket is a hook that runs when a storage bucket is updated. func (sp *StoragePlugin) OnUpdateStorageBucket(ctx context.Context, req *pb.OnUpdateStorageBucketRequest) (*pb.OnUpdateStorageBucketResponse, error) { - return nil, fmt.Errorf("unimplemented") + newBucket := req.GetNewBucket() + if newBucket == nil { + return nil, status.Error(codes.InvalidArgument, "new bucket is required") + } + if newBucket.BucketName == "" { + return nil, status.Error(codes.InvalidArgument, "new bucketName is required") + } + oldBucket := req.GetCurrentBucket() + if oldBucket == nil { + return nil, status.Error(codes.InvalidArgument, "current bucket is required") + } + + // current attributes + osa, err := getStorageAttributes(oldBucket.GetAttributes()) + if err != nil { + return nil, err + } + + // new attributes and secrets + nsa, err := getStorageAttributes(newBucket.GetAttributes()) + if err != nil { + return nil, err + } + + sec, err := getStorageSecrets(newBucket.GetSecrets()) + if err != nil { + return nil, err + } + + switch { + case newBucket.GetBucketName() != oldBucket.GetBucketName(): + return nil, status.Errorf(codes.InvalidArgument, "cannot update attribute BucketName") + case newBucket.GetBucketPrefix() != oldBucket.GetBucketPrefix(): + return nil, status.Errorf(codes.InvalidArgument, "cannot update attribute BucketPrefix") + case nsa.Region != osa.Region: + return nil, status.Errorf(codes.InvalidArgument, "cannot update attribute Region") + case nsa.EndpointUrl != osa.EndpointUrl: + return nil, status.Errorf(codes.InvalidArgument, "cannot update attribute EndpointUrl") + } + + if err = ensureServiceAccount(ctx, nsa, sec); err != nil { + return nil, status.Errorf(codes.InvalidArgument, "failed to ensure service account: %s", err) + } + + cl, err := minio.New(nsa.EndpointUrl, &minio.Options{ + Creds: credentials.NewStaticV4(sec.AccessKeyId, sec.SecretAccessKey, ""), + Secure: nsa.UseSSL, + Region: nsa.Region, + }) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "failed to create minio sdk client: %s", err) + } + + if err = dryRun(ctx, cl, newBucket); err != nil { + return nil, status.Errorf(codes.InvalidArgument, "failed to verify provided minio environment: %s", err) + } + + persisted, err := structpb.NewStruct(sec.AsMap()) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "failed to convert storage attributes to protobuf struct: %s", err) + } + + return &pb.OnUpdateStorageBucketResponse{ + Persisted: &storagebuckets.StorageBucketPersisted{Data: persisted}, + }, nil } // OnDeleteStorageBucket is a hook that runs when a storage bucket is deleted. diff --git a/plugin/service/storage/plugin_test.go b/plugin/service/storage/plugin_test.go index 5107513..686b5db 100644 --- a/plugin/service/storage/plugin_test.go +++ b/plugin/service/storage/plugin_test.go @@ -15,6 +15,8 @@ import ( "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/testing/protocmp" "google.golang.org/protobuf/types/known/structpb" ) @@ -262,7 +264,401 @@ func TestOnCreateStorageBucket(t *testing.T) { } } -func TestOnUpdateStorageBucket(t *testing.T) {} +func TestOnUpdateStorageBucket(t *testing.T) { + ctx := context.Background() + server := internaltest.NewMinioServer(t) + + bucketName := "test-bucket" + require.NoError(t, server.Client.MakeBucket(ctx, bucketName, minio.MakeBucketOptions{})) + + tests := []struct { + name string + req *plugin.OnUpdateStorageBucketRequest + expected *plugin.OnUpdateStorageBucketResponse + err string + errCode codes.Code + }{ + { + name: "nilNewBucket", + req: &plugin.OnUpdateStorageBucketRequest{}, + err: "new bucket is required", + errCode: codes.InvalidArgument, + }, + { + name: "emptyNewBucketName", + req: &plugin.OnUpdateStorageBucketRequest{ + NewBucket: &storagebuckets.StorageBucket{}, + }, + err: "new bucketName is required", + errCode: codes.InvalidArgument, + }, + { + name: "nilCurrentBucket", + req: &plugin.OnUpdateStorageBucketRequest{ + NewBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + }, + }, + err: "current bucket is required", + errCode: codes.InvalidArgument, + }, + { + name: "nilCurrentBucketAttributes", + req: &plugin.OnUpdateStorageBucketRequest{ + NewBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + }, + CurrentBucket: &storagebuckets.StorageBucket{}, + }, + err: "empty attributes input", + errCode: codes.InvalidArgument, + }, + { + name: "nilNewBucketAttributes", + req: &plugin.OnUpdateStorageBucketRequest{ + NewBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + Secrets: new(structpb.Struct), + }, + CurrentBucket: &storagebuckets.StorageBucket{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr), + }, + }, + }, + }, + err: "empty attributes input", + errCode: codes.InvalidArgument, + }, + { + name: "nilNewBucketSecrets", + req: &plugin.OnUpdateStorageBucketRequest{ + NewBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr), + }, + }, + }, + CurrentBucket: &storagebuckets.StorageBucket{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr), + }, + }, + }, + }, + err: "empty secrets input", + errCode: codes.InvalidArgument, + }, + { + name: "errorReadingAttributes", + req: &plugin.OnUpdateStorageBucketRequest{ + NewBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + Secrets: new(structpb.Struct), + Attributes: new(structpb.Struct), + }, + CurrentBucket: &storagebuckets.StorageBucket{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr), + }, + }, + }, + }, + err: "empty attributes input", + errCode: codes.InvalidArgument, + }, + { + name: "errorReadingSecrets", + req: &plugin.OnUpdateStorageBucketRequest{ + NewBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr), + }, + }, + Secrets: new(structpb.Struct), + }, + CurrentBucket: &storagebuckets.StorageBucket{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr), + }, + }, + }, + }, + err: "empty secrets input", + errCode: codes.InvalidArgument, + }, + { + name: "invalidBucketNameUpdate", + req: &plugin.OnUpdateStorageBucketRequest{ + NewBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + 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.ServiceAccountAccessKeyId), + ConstSecretAccessKey: structpb.NewStringValue(server.ServiceAccountSecretAccessKey), + }, + }, + }, + CurrentBucket: &storagebuckets.StorageBucket{ + BucketName: "foo2", + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr), + }, + }, + }, + }, + err: "cannot update attribute BucketName", + errCode: codes.InvalidArgument, + }, + { + name: "invalidBucketPrefixUpdate", + req: &plugin.OnUpdateStorageBucketRequest{ + NewBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + BucketPrefix: "foo", + 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.ServiceAccountAccessKeyId), + ConstSecretAccessKey: structpb.NewStringValue(server.ServiceAccountSecretAccessKey), + }, + }, + }, + CurrentBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr), + }, + }, + }, + }, + err: "cannot update attribute BucketPrefix", + errCode: codes.InvalidArgument, + }, + { + name: "invalidRegionUpdate", + req: &plugin.OnUpdateStorageBucketRequest{ + NewBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr), + "region": structpb.NewStringValue("a region"), + }, + }, + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstAccessKeyId: structpb.NewStringValue(server.ServiceAccountAccessKeyId), + ConstSecretAccessKey: structpb.NewStringValue(server.ServiceAccountSecretAccessKey), + }, + }, + }, + CurrentBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr), + }, + }, + }, + }, + err: "cannot update attribute Region", + errCode: codes.InvalidArgument, + }, + { + name: "invalidEndpointUrlUpdate", + req: &plugin.OnUpdateStorageBucketRequest{ + NewBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://localhost:9000"), + }, + }, + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstAccessKeyId: structpb.NewStringValue(server.ServiceAccountAccessKeyId), + ConstSecretAccessKey: structpb.NewStringValue(server.ServiceAccountSecretAccessKey), + }, + }, + }, + CurrentBucket: &storagebuckets.StorageBucket{ + BucketName: "foo", + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr), + }, + }, + }, + }, + err: "cannot update attribute EndpointUrl", + 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{ + NewBucket: &storagebuckets.StorageBucket{ + BucketName: bucketName, + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstEndpointUrl: structpb.NewStringValue("http://" + server.ApiAddr), + }, + }, + Secrets: &structpb.Struct{ + Fields: func() map[string]*structpb.Value { + creds, err := server.AdminClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{ + Policy: policyDenyPutObject, + }) + require.NoError(t, err) + + return map[string]*structpb.Value{ + ConstAccessKeyId: structpb.NewStringValue(creds.AccessKey), + ConstSecretAccessKey: structpb.NewStringValue(creds.SecretKey), + } + }(), + }, + }, + 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: "failed to verify provided minio environment: failed to put object", + errCode: codes.InvalidArgument, + }, + { + name: "success", + 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.ServiceAccountAccessKeyId), + ConstSecretAccessKey: structpb.NewStringValue(server.ServiceAccountSecretAccessKey), + }, + }, + }, + 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.ServiceAccountAccessKeyId), + ConstSecretAccessKey: structpb.NewStringValue(server.ServiceAccountSecretAccessKey), + }, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + sp := new(StoragePlugin) + res, err := sp.OnUpdateStorageBucket(ctx, tt.req) + if tt.err != "" { + require.ErrorContains(err, tt.err) + require.Equal(tt.errCode.String(), status.Code(err).String()) + require.Nil(res) + return + } + + require.NoError(err) + require.NotNil(res) + require.EqualValues(tt.expected, res) + }) + } +} func TestOnDeleteStorageBucket(t *testing.T) { sp := new(StoragePlugin)