From 08fd6e5b010baa5b4bc369a146657d3deb930533 Mon Sep 17 00:00:00 2001 From: Elim Tsiagbey Date: Mon, 28 Oct 2024 10:48:26 -0400 Subject: [PATCH] feat: OnUpdate Host Catalog (#9) * feat: OnUpdate Host Catalog Add support for onUpdate host catalog --- internal/credential/config.go | 20 ++ internal/credential/config_test.go | 62 ++++++ internal/credential/options.go | 78 ++++++- internal/credential/options_test.go | 90 +++++++- internal/credential/rotate.go | 37 ++++ internal/credential/rotate_test.go | 97 +++++++++ internal/credential/state.go | 121 +++++++++++ internal/credential/state_test.go | 107 ++++++++++ plugin/service/host/plugin.go | 122 ++++++++++- plugin/service/host/plugin_test.go | 306 ++++++++++++++++++++++++---- plugin/service/host/state_test.go | 1 + 11 files changed, 989 insertions(+), 52 deletions(-) diff --git a/internal/credential/config.go b/internal/credential/config.go index 996fefc..ff27d35 100644 --- a/internal/credential/config.go +++ b/internal/credential/config.go @@ -55,6 +55,26 @@ type credentials struct { PrivateKeyId string `json:"private_key_id"` } +// NewConfig creates a new GCP credential configuration +// based on the provided options. +// If the options are invalid, it will return an error. +func NewConfig(opt ...Option) (*Config, error) { + opts, err := getOpts(opt...) + if err != nil { + return nil, err + } + c := &Config{ + ProjectId: opts.WithProjectId, + PrivateKey: opts.WithPrivateKey, + PrivateKeyId: opts.WithPrivateKeyId, + ClientEmail: opts.WithClientEmail, + TargetServiceAccountId: opts.WithTargetServiceAccountId, + Zone: opts.WithZone, + Scopes: opts.WithScopes, + } + return c, nil +} + // toCredentials converts the config to credentials. func (c *Config) toCredentials() *credentials { return &credentials{ diff --git a/internal/credential/config_test.go b/internal/credential/config_test.go index 2225854..2febd78 100644 --- a/internal/credential/config_test.go +++ b/internal/credential/config_test.go @@ -383,3 +383,65 @@ func TestIsRotatable(t *testing.T) { }) } } + +func TestNewConfig(t *testing.T) { + tests := []struct { + name string + options []Option + expected *Config + expectError bool + }{ + { + name: "Valid Options", + options: []Option{ + WithProjectId("test-project-id"), + WithPrivateKey("test-private-key"), + WithPrivateKeyId("test-private-key-id"), + WithClientEmail("test-email@example.com"), + WithTargetServiceAccountId("target-service-account@example.com"), + WithZone("test-zone"), + WithScopes([]string{"scope1", "scope2"}), + }, + expected: &Config{ + ProjectId: "test-project-id", + PrivateKey: "test-private-key", + PrivateKeyId: "test-private-key-id", + ClientEmail: "test-email@example.com", + TargetServiceAccountId: "target-service-account@example.com", + Zone: "test-zone", + Scopes: []string{"scope1", "scope2"}, + }, + expectError: false, + }, + { + name: "No Options", + options: []Option{}, + expected: &Config{}, + expectError: false, + }, + { + name: "Invalid Option", + options: []Option{ + func(opts *Options) error { + return errors.New("invalid option") + }, + }, + expected: nil, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config, err := NewConfig(tt.options...) + if tt.expectError { + require.Error(t, err) + require.Nil(t, config) + return + } + require.NoError(t, err) + require.NotNil(t, config) + require.Equal(t, tt.expected, config) + }) + } +} diff --git a/internal/credential/options.go b/internal/credential/options.go index 7004ba8..183be53 100644 --- a/internal/credential/options.go +++ b/internal/credential/options.go @@ -9,8 +9,15 @@ import ( // options = how options are represented type Options struct { - WithCredentialsConfig *Config - WithCredsLastRotatedTime time.Time + WithCredentialsConfig *Config + WithCredsLastRotatedTime time.Time + WithClientEmail string + WithProjectId string + WithTargetServiceAccountId string + WithZone string + WithPrivateKeyId string + WithPrivateKey string + WithScopes []string } // getOpts - iterate the inbound Options and return a struct @@ -41,10 +48,75 @@ func WithCredentialsConfig(c *Config) Option { } } -// WithCredsLastRotatedTime - set the last rotated time +// WithCredsLastRotatedTime - The last time the service account key was +// rotated func WithCredsLastRotatedTime(t time.Time) Option { return func(o *Options) error { o.WithCredsLastRotatedTime = t return nil } } + +// WithClientEmail - The email address associated with the service account. +// The email address used to uniquely identify the service account +func WithClientEmail(email string) Option { + return func(o *Options) error { + o.WithClientEmail = email + return nil + } +} + +// WithProjectId - The project ID associated with the service account +func WithProjectId(id string) Option { + return func(o *Options) error { + o.WithProjectId = id + return nil + } +} + +// WithTargetServiceAccountId - The account that will be impersonated. +// This account has permission to perform actions that the base service +// account does not have. +func WithTargetServiceAccountId(id string) Option { + return func(o *Options) error { + o.WithTargetServiceAccountId = id + return nil + } +} + +// WithZone - The zone where the GCP resources are located +func WithZone(zone string) Option { + return func(o *Options) error { + o.WithZone = zone + return nil + } +} + +// WithPrivateKeyId - The private key ID +// of the GCP service account +func WithPrivateKeyId(id string) Option { + return func(o *Options) error { + o.WithPrivateKeyId = id + return nil + } +} + +// WithPrivateKey - set the GCP service account +// private key which is used to authentication +func WithPrivateKey(key string) Option { + return func(o *Options) error { + o.WithPrivateKey = key + return nil + } +} + +// WithScopes - set the GCP scope that defines the level +// of access that the requested access token will have. +// This option is required when authenticating with +// Service Account Impersonation. +func WithScopes(scopes []string) Option { + return func(o *Options) error { + o.WithScopes = scopes + return nil + } +} diff --git a/internal/credential/options_test.go b/internal/credential/options_test.go index 7b7a08d..1b7a19b 100644 --- a/internal/credential/options_test.go +++ b/internal/credential/options_test.go @@ -25,11 +25,99 @@ func Test_getOpts(t *testing.T) { require.Equal(t, opts, testOpts) }) t.Run("WithCredsLastRotatedTime", func(t *testing.T) { + opts, err := getOpts() + require.NoError(t, err) + require.Empty(t, opts.WithCredsLastRotatedTime) + tm := time.Now() - opts, err := getOpts(WithCredsLastRotatedTime(tm)) + opts, err = getOpts(WithCredsLastRotatedTime(tm)) require.NoError(t, err) testOpts := getDefaultOptions() testOpts.WithCredsLastRotatedTime = tm require.Equal(t, opts, testOpts) }) + t.Run("WithClientEmail", func(t *testing.T) { + opts, err := getOpts() + require.NoError(t, err) + require.Empty(t, opts.WithClientEmail) + + email := "test" + opts, err = getOpts(WithClientEmail(email)) + require.NoError(t, err) + testOpts := getDefaultOptions() + testOpts.WithClientEmail = email + require.Equal(t, opts, testOpts) + }) + t.Run("WithProjectId", func(t *testing.T) { + opts, err := getOpts() + require.NoError(t, err) + require.Empty(t, opts.WithProjectId) + + projectID := "test" + opts, err = getOpts(WithProjectId(projectID)) + require.NoError(t, err) + testOpts := getDefaultOptions() + testOpts.WithProjectId = projectID + require.Equal(t, opts, testOpts) + }) + t.Run("WithTargetServiceAccountId", func(t *testing.T) { + opts, err := getOpts() + require.NoError(t, err) + require.Empty(t, opts.WithTargetServiceAccountId) + + serviceAccountID := "test" + opts, err = getOpts(WithTargetServiceAccountId(serviceAccountID)) + require.NoError(t, err) + testOpts := getDefaultOptions() + testOpts.WithTargetServiceAccountId = serviceAccountID + require.Equal(t, opts, testOpts) + }) + t.Run("WithZone", func(t *testing.T) { + opts, err := getOpts() + require.NoError(t, err) + require.Empty(t, opts.WithZone) + + zone := "test" + opts, err = getOpts(WithZone(zone)) + require.NoError(t, err) + testOpts := getDefaultOptions() + testOpts.WithZone = zone + require.Equal(t, opts, testOpts) + }) + t.Run("WithPrivateKey", func(t *testing.T) { + opts, err := getOpts() + require.NoError(t, err) + require.Empty(t, opts.WithPrivateKey) + + privateKey := "test" + opts, err = getOpts(WithPrivateKey(privateKey)) + require.NoError(t, err) + testOpts := getDefaultOptions() + testOpts.WithPrivateKey = privateKey + require.Equal(t, opts, testOpts) + }) + t.Run("WithPrivateKeyId", func(t *testing.T) { + opts, err := getOpts() + require.NoError(t, err) + require.Empty(t, opts.WithPrivateKeyId) + + privateKeyID := "test" + opts, err = getOpts(WithPrivateKeyId(privateKeyID)) + require.NoError(t, err) + testOpts := getDefaultOptions() + testOpts.WithPrivateKeyId = privateKeyID + require.Equal(t, opts, testOpts) + }) + t.Run("WithScopes", func(t *testing.T) { + opts, err := getOpts() + require.NoError(t, err) + require.Nil(t, opts.WithScopes) + + scopes := []string{"test"} + opts, err = getOpts(WithScopes(scopes)) + require.NoError(t, err) + testOpts := getDefaultOptions() + testOpts.WithScopes = scopes + require.Equal(t, opts, testOpts) + }) } diff --git a/internal/credential/rotate.go b/internal/credential/rotate.go index 08d3ce1..640b839 100644 --- a/internal/credential/rotate.go +++ b/internal/credential/rotate.go @@ -112,6 +112,43 @@ func (c *Config) RotateServiceAccountKey(ctx context.Context, permissions []stri return nil } +func (c *Config) DeletePrivateKey(ctx context.Context, opts ...option.ClientOption) error { + if c.PrivateKey == "" { + return status.Error(codes.InvalidArgument, "cannot delete credentials when private key is not set") + } + if c.PrivateKeyId == "" { + return status.Error(codes.InvalidArgument, "cannot delete credentials when private key ID is not set") + } + if c.ClientEmail == "" { + return status.Error(codes.InvalidArgument, "cannot delete credentials when client email is not set") + } + + creds, err := c.GenerateCredentials(ctx) + if err != nil { + return status.Errorf(codes.Unauthenticated, "error generating credentials: %v", err) + } + if creds.TokenSource == nil { + return status.Error(codes.Unauthenticated, "error generating credentials: token source is nil") + } + + clientOptions := []option.ClientOption{option.WithTokenSource(creds.TokenSource)} + clientOptions = append(clientOptions, opts...) + + iamClient, err := admin.NewIamClient(ctx, clientOptions...) + if err != nil { + return status.Errorf(codes.Internal, "error creating IAM client: %v", err) + } + + err = iamClient.DeleteServiceAccountKey(ctx, &adminpb.DeleteServiceAccountKeyRequest{ + 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 nil +} + // ValidateIamPermissions tests the IAM permissions for the credentials. // It returns the granted permissions if successful. func (c *Config) ValidateIamPermissions(ctx context.Context, permissions []string, opts ...option.ClientOption) ([]string, error) { diff --git a/internal/credential/rotate_test.go b/internal/credential/rotate_test.go index 9526f91..ffd30b2 100644 --- a/internal/credential/rotate_test.go +++ b/internal/credential/rotate_test.go @@ -264,3 +264,100 @@ func TestValidateIamPermissions(t *testing.T) { }) } } +func TestDeletePrivateKey(t *testing.T) { + testIAMAdminServer := &testIAMAdminServer{} + + tests := []struct { + name string + config *Config + setup func() + expectedError error + }{ + { + name: "PrivateKey is empty", + config: &Config{ + PrivateKey: "", + }, + expectedError: status.Error(codes.InvalidArgument, "cannot delete credentials when private key is not set"), + }, + { + name: "PrivateKeyId is empty", + config: &Config{ + PrivateKey: "some-private-key", + PrivateKeyId: "", + }, + expectedError: status.Error(codes.InvalidArgument, "cannot delete credentials when private key ID is not set"), + }, + { + name: "ClientEmail is empty", + config: &Config{ + PrivateKey: "some-private-key", + PrivateKeyId: "some-private-key-id", + ClientEmail: "", + }, + expectedError: status.Error(codes.InvalidArgument, "cannot delete credentials when client email is not set"), + }, + { + name: "GenerateCredentials returns error", + config: &Config{ + PrivateKey: "some-private-key", + PrivateKeyId: "some-private-key-id", + ClientEmail: "client@example.com", + TargetServiceAccountId: "test-service-account-id", + }, + expectedError: status.Errorf(codes.Unauthenticated, "error generating credentials"), + }, + { + name: "DeleteServiceAccountKey returns error", + config: &Config{ + PrivateKey: "some-private-key", + PrivateKeyId: "some-private-key-id", + ClientEmail: "client@example.com", + }, + setup: func() { + testIAMAdminServer.testDeleteServiceAccountKeyError = errors.New("delete key error") + }, + expectedError: status.Errorf(codes.Internal, "error deleting service account key"), + }, + { + name: "Successful deletion", + config: &Config{ + PrivateKey: "some-private-key", + PrivateKeyId: "some-private-key-id", + ClientEmail: "client@example.com", + }, + setup: func() { + testIAMAdminServer.testDeleteServiceAccountKeyError = nil + }, + expectedError: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gsrv := NewGRPCServer() + adminpb.RegisterIAMServer(gsrv.Server, testIAMAdminServer) + addr, err := gsrv.Start() + require.NoError(t, err) + + ctx := context.Background() + if tt.setup != nil { + tt.setup() + } + testOptions := []googleOption.ClientOption{ + googleOption.WithEndpoint(addr), + googleOption.WithoutAuthentication(), + googleOption.WithGRPCDialOption(grpc.WithTransportCredentials(insecure.NewCredentials())), + googleOption.WithTokenSource(nil), + } + + err = tt.config.DeletePrivateKey(ctx, testOptions...) + if tt.expectedError != nil { + require.ErrorContains(t, err, tt.expectedError.Error()) + return + } + + require.NoError(t, err) + }) + } +} diff --git a/internal/credential/state.go b/internal/credential/state.go index bbae593..167392c 100644 --- a/internal/credential/state.go +++ b/internal/credential/state.go @@ -4,7 +4,15 @@ package credential import ( + "context" + "fmt" "time" + + "github.com/hashicorp/boundary-plugin-gcp/internal/values" + "google.golang.org/api/option" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/structpb" ) type CredentialType int @@ -29,13 +37,126 @@ func NewPersistedState(opt ...Option) (*PersistedState, error) { return s, nil } +// ReplaceCreds replaces the private key in the state with a new key. +// If the existing key was rotated at any point in time, it is +// deleted first, otherwise it's left alone. This method returns a +// status error with PluginError details. +func (s *PersistedState) ReplaceCreds(ctx context.Context, newCreds *Config, opts ...option.ClientOption) error { + if newCreds == nil { + return status.Errorf(codes.InvalidArgument, "missing new credentials") + } + if s.CredentialsConfig == nil { + return status.Errorf(codes.InvalidArgument, "missing existing credentials") + } + + // Delete old/existing credentials. + // This is done with the same credentials to ensure that it has the proper permissions to do it. + if !s.CredsLastRotatedTime.IsZero() && s.CredentialsConfig.IsRotatable() { + if err := s.DeleteCreds(ctx, opts...); err != nil { + return err + } + } + + // Set the new attributes and clear the rotated time. + s.CredentialsConfig = newCreds + s.CredsLastRotatedTime = time.Time{} + return nil +} + +// DeleteCreds deletes the credentials in the state. The access key +// ID, secret access key, and rotation time fields are zeroed out in +// the state just to ensure that they cannot be re-used after. This +// method returns a status error with PluginError details. +func (s *PersistedState) DeleteCreds(ctx context.Context, opts ...option.ClientOption) error { + if s.CredentialsConfig == nil { + return status.Errorf(codes.InvalidArgument, "missing credentials config") + } + if !s.CredentialsConfig.IsRotatable() { + return status.Errorf(codes.InvalidArgument, "cannot delete credentials for non-rotatable credentials") + } + + err := s.CredentialsConfig.DeletePrivateKey(ctx, opts...) + if err != nil { + return err + } + + s.CredentialsConfig = nil + s.CredsLastRotatedTime = time.Time{} + return nil +} + // ToMap returns a map of the credentials stored in the persisted state. // ToMap will return a map for long-term credentials with following keys: // private_key_id, private_key & creds_last_rotated_time func (s *PersistedState) ToMap() map[string]any { + if !s.CredentialsConfig.IsRotatable() { + return map[string]any{} + } return map[string]any{ ConstPrivateKey: s.CredentialsConfig.PrivateKey, ConstPrivateKeyId: s.CredentialsConfig.PrivateKeyId, ConstCredsLastRotatedTime: s.CredsLastRotatedTime.Format(time.RFC3339Nano), } } + +// PersistedStateFromProto parses values out of a protobuf struct input +// and returns a PersistedState used for GCP authentication. +func PersistedStateFromProto(secrets *structpb.Struct, attrs *CredentialAttributes, opts ...Option) (*PersistedState, error) { + if secrets == nil { + secrets = &structpb.Struct{ + Fields: map[string]*structpb.Value{}, + } + } + + if attrs == nil { + return nil, fmt.Errorf("missing credential attributes") + } + + privateKeyId, err := values.GetStringValue(secrets, ConstPrivateKeyId, false) + if err != nil { + return nil, fmt.Errorf("persisted state integrity error: %w", err) + } + + privateKey, err := values.GetStringValue(secrets, ConstPrivateKey, false) + if err != nil { + return nil, fmt.Errorf("persisted state integrity error: %w", err) + } + + credsLastRotatedTime, err := values.GetTimeValue(secrets, ConstCredsLastRotatedTime) + if err != nil { + return nil, fmt.Errorf("persisted state integrity error: %w", err) + } + + s, err := NewPersistedState(opts...) + if err != nil { + return nil, err + } + + cfgOpts := []Option{} + if privateKeyId != "" && privateKey != "" { + cfgOpts = append(cfgOpts, + WithPrivateKey(privateKey), + WithPrivateKeyId(privateKeyId)) + } + + if attrs.Zone != "" { + cfgOpts = append(cfgOpts, WithZone(attrs.Zone)) + } + if attrs.ProjectId != "" { + cfgOpts = append(cfgOpts, WithProjectId(attrs.ProjectId)) + } + if attrs.ClientEmail != "" { + cfgOpts = append(cfgOpts, WithClientEmail(attrs.ClientEmail)) + } + if attrs.TargetServiceAccountId != "" { + cfgOpts = append(cfgOpts, WithTargetServiceAccountId(attrs.TargetServiceAccountId)) + } + credentialsConfig, err := NewConfig(cfgOpts...) + if err != nil { + return nil, err + } + s.CredentialsConfig = credentialsConfig + s.CredsLastRotatedTime = credsLastRotatedTime + + return s, nil +} diff --git a/internal/credential/state_test.go b/internal/credential/state_test.go index a634da5..b8bbf6e 100644 --- a/internal/credential/state_test.go +++ b/internal/credential/state_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" ) func TestNewGCPCredentialPersistedState(t *testing.T) { @@ -77,3 +78,109 @@ func TestNewGCPCredentialPersistedState(t *testing.T) { }) } } + +func TestPersistedStateFromProto(t *testing.T) { + staticTime := time.Now() + + cases := []struct { + name string + secrets *structpb.Struct + attrs *CredentialAttributes + opts []Option + expected *PersistedState + expectedErr string + }{ + { + name: "missing credential attributes", + expectedErr: "missing credential attributes", + }, + { + name: "missing credential attributes", + secrets: &structpb.Struct{}, + attrs: nil, + expectedErr: "missing credential attributes", + }, + { + name: "with static credentials", + secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstPrivateKeyId: structpb.NewStringValue("test-private-key-id"), + ConstPrivateKey: structpb.NewStringValue("test-private-key"), + ConstCredsLastRotatedTime: structpb.NewStringValue(staticTime.Format(time.RFC3339Nano)), + }, + }, + attrs: &CredentialAttributes{ + Zone: "us-central1-a", + ProjectId: "test-project", + ClientEmail: "test-email", + }, + expected: &PersistedState{ + CredentialsConfig: &Config{ + PrivateKeyId: "test-private-key-id", + PrivateKey: "test-private-key", + Zone: "us-central1-a", + ProjectId: "test-project", + ClientEmail: "test-email", + }, + CredsLastRotatedTime: func() time.Time { + t, err := time.Parse(time.RFC3339Nano, staticTime.Format(time.RFC3339Nano)) + if err != nil { + panic(err) + } + return t + }(), + }, + }, + { + name: "with service account impersonation credentials", + secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + ConstPrivateKeyId: structpb.NewStringValue("test-private-key-id"), + ConstPrivateKey: structpb.NewStringValue("test-private-key"), + ConstCredsLastRotatedTime: structpb.NewStringValue((time.Time{}).Format(time.RFC3339Nano)), + }, + }, + attrs: &CredentialAttributes{ + Zone: "us-central1-a", + ProjectId: "test-project", + ClientEmail: "test-email", + TargetServiceAccountId: "test-service-account-id", + }, + expected: &PersistedState{ + CredentialsConfig: &Config{ + PrivateKeyId: "test-private-key-id", + PrivateKey: "test-private-key", + Zone: "us-central1-a", + ProjectId: "test-project", + ClientEmail: "test-email", + TargetServiceAccountId: "test-service-account-id", + }, + CredsLastRotatedTime: time.Time{}, + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + require := require.New(t) + actual, err := PersistedStateFromProto(tc.secrets, tc.attrs, tc.opts...) + if tc.expectedErr != "" { + require.EqualError(err, tc.expectedErr) + return + } + + require.NoError(err) + require.Equal(tc.expected.CredsLastRotatedTime, actual.CredsLastRotatedTime) + if tc.expected.CredentialsConfig != nil { + require.NotNil(actual.CredentialsConfig) + require.Equal(tc.expected.CredentialsConfig.PrivateKeyId, actual.CredentialsConfig.PrivateKeyId) + require.Equal(tc.expected.CredentialsConfig.PrivateKey, actual.CredentialsConfig.PrivateKey) + require.Equal(tc.expected.CredentialsConfig.Zone, actual.CredentialsConfig.Zone) + require.Equal(tc.expected.CredentialsConfig.ProjectId, actual.CredentialsConfig.ProjectId) + require.Equal(tc.expected.CredentialsConfig.ClientEmail, actual.CredentialsConfig.ClientEmail) + require.Equal(tc.expected.CredentialsConfig.TargetServiceAccountId, actual.CredentialsConfig.TargetServiceAccountId) + } + }) + } +} diff --git a/plugin/service/host/plugin.go b/plugin/service/host/plugin.go index c17cb05..d31e607 100644 --- a/plugin/service/host/plugin.go +++ b/plugin/service/host/plugin.go @@ -100,23 +100,125 @@ func (p *HostPlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalo } // OnUpdateCatalog is called when a dynamic host catalog is updated. -func (p *HostPlugin) OnUpdateCatalog(_ context.Context, req *pb.OnUpdateCatalogRequest) (*pb.OnUpdateCatalogResponse, error) { +func (p *HostPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalogRequest) (*pb.OnUpdateCatalogResponse, error) { currentCatalog := req.GetCurrentCatalog() if currentCatalog == nil { - return nil, status.Error(codes.FailedPrecondition, "current catalog is nil") + return nil, status.Errorf(codes.InvalidArgument, "current catalog is required") + } + newCatalog := req.GetNewCatalog() + if newCatalog == nil { + return nil, status.Errorf(codes.InvalidArgument, "new catalog is required") } - secrets := req.GetNewCatalog().GetSecrets() - if secrets == nil { - // If new secrets weren't passed in, don't rotate what we have on - // update. - return &pb.OnUpdateCatalogResponse{}, nil + oldAttrs := currentCatalog.GetAttributes() + if oldAttrs == nil { + return nil, status.Errorf(codes.InvalidArgument, "old catalog attributes are required") + } + oldCatalogAttributes, err := getCatalogAttributes(oldAttrs) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "error getting old catalog attributes: %s", err) + } + newAttrs := newCatalog.GetAttributes() + if newAttrs == nil { + return nil, status.Errorf(codes.InvalidArgument, "new catalog attributes are required") + } + newCatalogAttributes, err := getCatalogAttributes(newAttrs) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "error getting new catalog attributes: %s", err) + } + + credState, err := credential.PersistedStateFromProto( + req.GetPersisted().GetSecrets(), + oldCatalogAttributes.CredentialAttributes, + ) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "error getting persisted state from proto: %s", err) + } + + updatedCredentials, err := credential.GetCredentialsConfig(newCatalog.GetSecrets(), newCatalogAttributes.CredentialAttributes) + if err != nil { + return nil, err + } + + if newCatalog.GetSecrets() != nil { + newCredState, err := credential.NewPersistedState([]credential.Option{ + credential.WithCredentialsConfig(updatedCredentials), + }..., + ) + if err != nil { + return nil, status.Error(codes.Internal, fmt.Sprintf("error creating new persisted state: %s", err)) + } + newCatalogState, err := newGCPCatalogPersistedState( + append([]gcpCatalogPersistedStateOption{ + withCredentials(newCredState), + }, p.testCatalogStateOpts...)..., + ) + if err != nil { + return nil, status.Errorf(codes.Internal, "error setting up persisted state: %s", err) + } + if st := dryRunValidation(ctx, newCatalogState); st != nil { + return nil, st.Err() + } + + // Replace the existing credential state. + // This checks the timestamp on the last rotation time as well + // and deletes the credentials if we are managing them + // (ie: if we've rotated them before). + if err := credState.ReplaceCreds(ctx, updatedCredentials, p.testGCPClientOpts...); err != nil { + return nil, err + } + } + + if credState.CredentialsConfig.IsRotatable() { + if !updatedCredentials.IsRotatable() && !newCatalogAttributes.DisableCredentialRotation { + return nil, status.Error(codes.InvalidArgument, "cannot rotate credentials for non-rotatable credentials") + } + + // This is a validate check to make sure that we aren't disabling + // rotation for credentials currently being managed by rotation. + // This is not allowed. + if newCatalogAttributes.DisableCredentialRotation && newCatalog.GetSecrets() == nil { + if !credState.CredsLastRotatedTime.IsZero() { + return nil, status.Error(codes.InvalidArgument, "cannot disable credential rotation for credentials currently being rotated") + } + } + + permissions := []string{ + credential.ComputeInstancesListPermission, + credential.IAMServiceAccountKeysCreatePermission, + credential.IAMServiceAccountKeysDeletePermission, + } + + // If we're enabling rotation now but didn't before, or have + // freshly replaced credentials, we can rotate here. + if !newCatalogAttributes.DisableCredentialRotation && credState.CredsLastRotatedTime.IsZero() { + if err := credState.CredentialsConfig.RotateServiceAccountKey(ctx, permissions, p.testGCPClientOpts...); err != nil { + return nil, err + } + } + } + + catalogState, err := newGCPCatalogPersistedState( + append([]gcpCatalogPersistedStateOption{ + withCredentials(credState), + }, p.testCatalogStateOpts...)..., + ) + if err != nil { + return nil, status.Errorf(codes.Internal, "error setting up persisted state: %s", err) + } + + // perform dry run to ensure we can interact with GCP as expected. + if st := dryRunValidation(ctx, catalogState, p.testGCPClientOpts...); st != nil { + return nil, st.Err() + } + + persistedProto, err := catalogState.toProto() + if err != nil { + return nil, status.Errorf(codes.Internal, "error converting state to proto: %s", err) } return &pb.OnUpdateCatalogResponse{ - Persisted: &pb.HostCatalogPersisted{ - Secrets: nil, - }, + Persisted: persistedProto, }, nil } diff --git a/plugin/service/host/plugin_test.go b/plugin/service/host/plugin_test.go index e671271..67aa62a 100644 --- a/plugin/service/host/plugin_test.go +++ b/plugin/service/host/plugin_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "testing" + "time" "cloud.google.com/go/compute/apiv1/computepb" "cloud.google.com/go/iam/admin/apiv1/adminpb" @@ -535,66 +536,268 @@ func TestCreateCatalog(t *testing.T) { } func TestUpdateCatalog(t *testing.T) { - t.Skip("TODO: this needs a secrets file to run - maybe only manually?") ctx := context.Background() - p := &HostPlugin{} - - wd, err := os.Getwd() - require.NoError(t, err) - require.NotEmpty(t, wd) - project, err := parseutil.ParsePath("file://" + filepath.Join(wd, "secrets", "project")) - require.NoError(t, err) - zone, err := parseutil.ParsePath("file://" + filepath.Join(wd, "secrets", "zone")) - require.NoError(t, err) - - hostCatalogAttributes := &hostcatalogs.HostCatalog_Attributes{ - Attributes: wrapMap(t, map[string]interface{}{ - cred.ConstProjectId: project, - cred.ConstZone: zone, - }), - } - - require.NoError(t, err) cases := []struct { name string - req *pb.OnCreateCatalogRequest - expected *pb.HostCatalogPersisted + req *pb.OnUpdateCatalogRequest + catalogOpts []gcpCatalogPersistedStateOption + expectedRsp *pb.OnUpdateCatalogResponse expectedErr string }{ { - name: "nil catalog", - req: &pb.OnCreateCatalogRequest{}, - expectedErr: "catalog is nil", + name: "nil current catalog", + req: &pb.OnUpdateCatalogRequest{}, + expectedErr: "current catalog is required", + }, + { + name: "nil new catalog", + req: &pb.OnUpdateCatalogRequest{ + CurrentCatalog: &hostcatalogs.HostCatalog{}, + }, + expectedErr: "new catalog is required", }, { name: "nil attributes", - req: &pb.OnCreateCatalogRequest{ - Catalog: &hostcatalogs.HostCatalog{}, + req: &pb.OnUpdateCatalogRequest{ + CurrentCatalog: &hostcatalogs.HostCatalog{}, + NewCatalog: &hostcatalogs.HostCatalog{}, }, expectedErr: "attributes are required", }, { name: "error reading attributes", - req: &pb.OnCreateCatalogRequest{ - Catalog: &hostcatalogs.HostCatalog{ + req: &pb.OnUpdateCatalogRequest{ + CurrentCatalog: &hostcatalogs.HostCatalog{ + Attrs: &hostcatalogs.HostCatalog_Attributes{ + Attributes: new(structpb.Struct), + }, + }, + NewCatalog: &hostcatalogs.HostCatalog{ Attrs: &hostcatalogs.HostCatalog_Attributes{ Attributes: new(structpb.Struct), }, }, }, - expectedErr: "attributes.project: missing required value \"project\"", + expectedErr: "attributes.project_id: missing required value \"project_id\"", }, { - name: "do not persist secrets, use gcloud ADC", - req: &pb.OnCreateCatalogRequest{ - Catalog: &hostcatalogs.HostCatalog{ - Attrs: hostCatalogAttributes, + name: "staticCredentialToStaticRotated", + req: &pb.OnUpdateCatalogRequest{ + CurrentCatalog: &hostcatalogs.HostCatalog{ + Attrs: &hostcatalogs.HostCatalog_Attributes{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstProjectId: structpb.NewStringValue("test-project"), + cred.ConstClientEmail: structpb.NewStringValue("test@example.com"), + cred.ConstZone: structpb.NewStringValue("us-central1-a"), + cred.ConstDisableCredentialRotation: structpb.NewBoolValue(true), + }, + }, + }, + }, + NewCatalog: &hostcatalogs.HostCatalog{ + Attrs: &hostcatalogs.HostCatalog_Attributes{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstProjectId: structpb.NewStringValue("test-project"), + cred.ConstZone: structpb.NewStringValue("us-central1-a"), + cred.ConstClientEmail: structpb.NewStringValue("test@example.com"), + }, + }, + }, + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstPrivateKeyId: structpb.NewStringValue("new-private-key-id"), + cred.ConstPrivateKey: structpb.NewStringValue("new-private-key"), + }, + }, + }, + Persisted: &pb.HostCatalogPersisted{ + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstPrivateKeyId: structpb.NewStringValue("persisted-private-key-id"), + cred.ConstPrivateKey: structpb.NewStringValue("persisted-private-key"), + cred.ConstCredsLastRotatedTime: structpb.NewStringValue(time.Time{}.Format(time.RFC3339Nano)), + }, + }, }, }, - expected: &pb.HostCatalogPersisted{ - Secrets: nil, + catalogOpts: []gcpCatalogPersistedStateOption{ + withTestInstancesAPIFunc(newTestMockInstances(ctx, + nil, + testMockInstancesWithListInstancesOutput(&computepb.InstanceList{}), + testMockInstancesWithListInstancesError(nil), + )), }, + expectedRsp: &pb.OnUpdateCatalogResponse{ + Persisted: &pb.HostCatalogPersisted{ + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstPrivateKeyId: structpb.NewStringValue("updated-private-key-id"), + cred.ConstPrivateKey: structpb.NewStringValue("updated-private-key"), + }, + }, + }, + }, + }, + { + name: "staticRotatedCredentialToStatic", + req: &pb.OnUpdateCatalogRequest{ + CurrentCatalog: &hostcatalogs.HostCatalog{ + Attrs: &hostcatalogs.HostCatalog_Attributes{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstProjectId: structpb.NewStringValue("test-project"), + cred.ConstClientEmail: structpb.NewStringValue("test@example.com"), + cred.ConstZone: structpb.NewStringValue("us-central1-a"), + }, + }, + }, + }, + NewCatalog: &hostcatalogs.HostCatalog{ + Attrs: &hostcatalogs.HostCatalog_Attributes{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstProjectId: structpb.NewStringValue("test-project"), + cred.ConstZone: structpb.NewStringValue("us-central1-a"), + cred.ConstClientEmail: structpb.NewStringValue("test@example.com"), + cred.ConstDisableCredentialRotation: structpb.NewBoolValue(true), + }, + }, + }, + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstPrivateKeyId: structpb.NewStringValue("not-rotated-private-key-id"), + cred.ConstPrivateKey: structpb.NewStringValue("not-rotated-private-key"), + }, + }, + }, + Persisted: &pb.HostCatalogPersisted{ + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstPrivateKeyId: structpb.NewStringValue("persisted-private-key-id"), + cred.ConstPrivateKey: structpb.NewStringValue("persisted-private-key"), + cred.ConstCredsLastRotatedTime: structpb.NewStringValue(time.Now().Format(time.RFC3339Nano)), + }, + }, + }, + }, + catalogOpts: []gcpCatalogPersistedStateOption{ + withTestInstancesAPIFunc(newTestMockInstances(ctx, + nil, + testMockInstancesWithListInstancesOutput(&computepb.InstanceList{}), + testMockInstancesWithListInstancesError(nil), + )), + }, + expectedRsp: &pb.OnUpdateCatalogResponse{ + Persisted: &pb.HostCatalogPersisted{ + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstPrivateKeyId: structpb.NewStringValue("not-rotated-private-key-id"), + cred.ConstPrivateKey: structpb.NewStringValue("not-rotated-private-key"), + }, + }, + }, + }, + }, + { + name: "staticCredentialToImpersonateCredential", + req: &pb.OnUpdateCatalogRequest{ + CurrentCatalog: &hostcatalogs.HostCatalog{ + Attrs: &hostcatalogs.HostCatalog_Attributes{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstProjectId: structpb.NewStringValue("test-project"), + cred.ConstClientEmail: structpb.NewStringValue("test@example.com"), + cred.ConstZone: structpb.NewStringValue("us-central1-a"), + cred.ConstDisableCredentialRotation: structpb.NewBoolValue(true), + }, + }, + }, + }, + NewCatalog: &hostcatalogs.HostCatalog{ + Attrs: &hostcatalogs.HostCatalog_Attributes{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstProjectId: structpb.NewStringValue("test-project"), + cred.ConstZone: structpb.NewStringValue("us-central1-a"), + cred.ConstClientEmail: structpb.NewStringValue("test@example.com"), + cred.ConstTargetServiceAccountID: structpb.NewStringValue("test-target-service-account-id"), + cred.ConstDisableCredentialRotation: structpb.NewBoolValue(true), + }, + }, + }, + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstPrivateKeyId: structpb.NewStringValue("new-private-key-id"), + cred.ConstPrivateKey: structpb.NewStringValue("new-private-key"), + }, + }, + }, + Persisted: &pb.HostCatalogPersisted{ + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstPrivateKeyId: structpb.NewStringValue("persisted-private-key-id"), + cred.ConstPrivateKey: structpb.NewStringValue("persisted-private-key"), + cred.ConstCredsLastRotatedTime: structpb.NewStringValue(time.Time{}.Format(time.RFC3339Nano)), + }, + }, + }, + }, + catalogOpts: []gcpCatalogPersistedStateOption{ + withTestInstancesAPIFunc(newTestMockInstances(ctx, + nil, + testMockInstancesWithListInstancesOutput(&computepb.InstanceList{}), + testMockInstancesWithListInstancesError(nil), + )), + }, + expectedRsp: &pb.OnUpdateCatalogResponse{ + Persisted: &pb.HostCatalogPersisted{ + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstPrivateKeyId: structpb.NewStringValue("new-private-key-id"), + cred.ConstPrivateKey: structpb.NewStringValue("new-private-key"), + }, + }, + }, + }, + }, + { + name: "staticRotatedCredentialToAppDefaultCredential", + req: &pb.OnUpdateCatalogRequest{ + CurrentCatalog: &hostcatalogs.HostCatalog{ + Attrs: &hostcatalogs.HostCatalog_Attributes{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstProjectId: structpb.NewStringValue("test-project"), + cred.ConstClientEmail: structpb.NewStringValue("test@example.com"), + cred.ConstZone: structpb.NewStringValue("us-central1-a"), + }, + }, + }, + }, + NewCatalog: &hostcatalogs.HostCatalog{ + Attrs: &hostcatalogs.HostCatalog_Attributes{ + Attributes: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstProjectId: structpb.NewStringValue("test-project"), + cred.ConstZone: structpb.NewStringValue("us-central1-a"), + }, + }, + }, + }, + Persisted: &pb.HostCatalogPersisted{ + Secrets: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + cred.ConstPrivateKeyId: structpb.NewStringValue("persisted-private-key-id"), + cred.ConstPrivateKey: structpb.NewStringValue("persisted-private-key"), + cred.ConstCredsLastRotatedTime: structpb.NewStringValue(time.Time{}.Format(time.RFC3339Nano)), + }, + }, + }, + }, + expectedErr: "cannot rotate credentials for non-rotatable credentials", }, } @@ -602,14 +805,41 @@ func TestUpdateCatalog(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { require := require.New(t) - actual, err := p.OnCreateCatalog(ctx, tc.req) + + testIAMAdminServer := cred.NewTestIAMAdminServer(nil, nil) + testResourceServer := cred.NewTestResourceServer(&iampb.TestIamPermissionsResponse{ + Permissions: []string{ + cred.ComputeInstancesListPermission, + cred.IAMServiceAccountKeysCreatePermission, + cred.IAMServiceAccountKeysDeletePermission, + }, + }, nil) + + gsrv := cred.NewGRPCServer() + adminpb.RegisterIAMServer(gsrv.Server, testIAMAdminServer) + resourcemanagerpb.RegisterProjectsServer(gsrv.Server, testResourceServer) + 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, + } + + actual, err := p.OnUpdateCatalog(ctx, tc.req) if tc.expectedErr != "" { require.Contains(err.Error(), tc.expectedErr) return } - require.NoError(err) - require.Equal(actual.GetPersisted().Secrets, tc.expected.GetSecrets()) + + delete(actual.GetPersisted().GetSecrets().GetFields(), cred.ConstCredsLastRotatedTime) + require.Empty(cmp.Diff(tc.expectedRsp, actual, protocmp.Transform())) }) } } diff --git a/plugin/service/host/state_test.go b/plugin/service/host/state_test.go index 6a8445a..b5a3ef2 100644 --- a/plugin/service/host/state_test.go +++ b/plugin/service/host/state_test.go @@ -45,6 +45,7 @@ func TestToProto(t *testing.T) { CredentialsConfig: &credential.Config{ PrivateKey: "test-private-key", PrivateKeyId: "test-private-key-id", + ClientEmail: "test@test.com", }, CredsLastRotatedTime: lastRotatedTime, },