diff --git a/internal/credential/rotate.go b/internal/credential/rotate.go index 640b839..6c4b31b 100644 --- a/internal/credential/rotate.go +++ b/internal/credential/rotate.go @@ -143,7 +143,7 @@ func (c *Config) DeletePrivateKey(ctx context.Context, opts ...option.ClientOpti Name: fmt.Sprintf("projects/-/serviceAccounts/%s/keys/%s", c.ClientEmail, c.PrivateKeyId), }) if err != nil { - return status.Errorf(codes.Internal, "error deleting service account key: %v", err) + return status.Errorf(codes.Unknown, "error deleting service account key: %v", err) } return nil diff --git a/internal/credential/rotate_test.go b/internal/credential/rotate_test.go index ffd30b2..a13c3d1 100644 --- a/internal/credential/rotate_test.go +++ b/internal/credential/rotate_test.go @@ -317,7 +317,7 @@ func TestDeletePrivateKey(t *testing.T) { setup: func() { testIAMAdminServer.testDeleteServiceAccountKeyError = errors.New("delete key error") }, - expectedError: status.Errorf(codes.Internal, "error deleting service account key"), + expectedError: status.Errorf(codes.Unknown, "error deleting service account key"), }, { name: "Successful deletion", diff --git a/plugin/service/host/plugin.go b/plugin/service/host/plugin.go index d31e607..5188a6f 100644 --- a/plugin/service/host/plugin.go +++ b/plugin/service/host/plugin.go @@ -234,10 +234,39 @@ func (p *HostPlugin) OnDeleteCatalog(ctx context.Context, req *pb.OnDeleteCatalo return nil, status.Error(codes.InvalidArgument, "new catalog missing attributes") } - if _, err := getCatalogAttributes(attrs); err != nil { + catalogAttributes, err := getCatalogAttributes(attrs) + if err != nil { return nil, err } + credState, err := credential.PersistedStateFromProto( + req.GetPersisted().GetSecrets(), + catalogAttributes.CredentialAttributes) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "error getting persisted state from proto: %s", err) + } + + _, err = newGCPCatalogPersistedState( + append([]gcpCatalogPersistedStateOption{ + withCredentials(credState), + }, p.testCatalogStateOpts...)..., + ) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err) + } + + // try to delete static credentials for dynamic and static credentials + if credState.CredentialsConfig.IsRotatable() { + if !credState.CredsLastRotatedTime.IsZero() { + // Delete old/existing credentials. This is done with the same + // credentials to ensure that it has the proper permissions to do + // it. + if err := credState.DeleteCreds(ctx, p.testGCPClientOpts...); err != nil { + return nil, err + } + } + } + return &pb.OnDeleteCatalogResponse{}, nil } diff --git a/plugin/service/host/plugin_test.go b/plugin/service/host/plugin_test.go index 67aa62a..8311b31 100644 --- a/plugin/service/host/plugin_test.go +++ b/plugin/service/host/plugin_test.go @@ -5,6 +5,7 @@ package host import ( "context" + "fmt" "os" "path/filepath" "testing" @@ -23,7 +24,9 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/api/option" "google.golang.org/grpc" + "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" "google.golang.org/protobuf/testing/protocmp" "google.golang.org/protobuf/types/known/structpb" @@ -844,6 +847,106 @@ func TestUpdateCatalog(t *testing.T) { } } +func TestDeleteCatalog(t *testing.T) { + cases := []struct { + name string + req *pb.OnDeleteCatalogRequest + catalogOpts []gcpCatalogPersistedStateOption + deleteCredError error + expectedRsp *pb.OnDeleteCatalogResponse + expectedErr string + expectedErrCode codes.Code + }{ + { + name: "persisted state setup error", + req: &pb.OnDeleteCatalogRequest{ + Catalog: &hostcatalogs.HostCatalog{ + Attrs: &hostcatalogs.HostCatalog_Attributes{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstProjectId: structpb.NewStringValue("test-project-id"), + cred.ConstZone: structpb.NewStringValue("us-central1-a"), + }, + }, + }, + }, + Persisted: &pb.HostCatalogPersisted{ + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstPrivateKeyId: structpb.NewStringValue("test-private-key-id"), + cred.ConstPrivateKey: structpb.NewStringValue("test-private-key"), + cred.ConstCredsLastRotatedTime: structpb.NewStringValue("2006-01-02T15:04:05+07:00"), + }, + }, + }, + }, + catalogOpts: []gcpCatalogPersistedStateOption{ + func(s *gcpCatalogPersistedState) error { + return fmt.Errorf("error loading persisted state") + }, + }, + expectedErr: "error loading persisted state", + expectedErrCode: codes.InvalidArgument, + }, + { + name: "delete error", + req: &pb.OnDeleteCatalogRequest{ + Catalog: &hostcatalogs.HostCatalog{ + Attrs: &hostcatalogs.HostCatalog_Attributes{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstClientEmail: structpb.NewStringValue("test@test.com"), + cred.ConstProjectId: structpb.NewStringValue("test-project-id"), + cred.ConstZone: structpb.NewStringValue("us-central1-a"), + }, + }, + }, + }, + Persisted: &pb.HostCatalogPersisted{ + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstPrivateKeyId: structpb.NewStringValue("test-private-key-id"), + cred.ConstPrivateKey: structpb.NewStringValue("test-private-key"), + cred.ConstCredsLastRotatedTime: structpb.NewStringValue("2006-01-02T15:04:05+07:00"), + }, + }, + }, + }, + deleteCredError: fmt.Errorf("failed to delete access key"), + expectedErr: "failed to delete access key", + expectedErrCode: codes.Unknown, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + require := require.New(t) + + testIAMAdminServer := cred.NewTestIAMAdminServer(nil, tc.deleteCredError) + + gsrv := cred.NewGRPCServer() + adminpb.RegisterIAMServer(gsrv.Server, testIAMAdminServer) + addr, err := gsrv.Start() + require.NoError(err) + + p := &HostPlugin{ + testGCPClientOpts: []option.ClientOption{ + option.WithEndpoint(addr), + option.WithoutAuthentication(), + option.WithGRPCDialOption(grpc.WithTransportCredentials(insecure.NewCredentials())), + option.WithTokenSource(nil), + }, + testCatalogStateOpts: tc.catalogOpts, + } + + _, err = p.OnDeleteCatalog(context.Background(), tc.req) + require.Error(err) + require.Contains(err.Error(), tc.expectedErr) + require.Equal(status.Code(err).String(), tc.expectedErrCode.String()) + }) + } +} + func TestCreateSet(t *testing.T) { ctx := context.Background() p := &HostPlugin{}