diff --git a/internal/credential/attributes_test.go b/internal/credential/attributes_test.go index e36bf73..bdffe33 100644 --- a/internal/credential/attributes_test.go +++ b/internal/credential/attributes_test.go @@ -119,15 +119,17 @@ func TestGetCredentialsConfig(t *testing.T) { Zone: "us-central-1a", }, expected: &Config{ - Zone: "us-central-1a", - Scopes: []string{defaultGCPScope}, + Zone: "us-central-1a", + Scopes: []string{defaultGCPScope}, + validateServiceAccountKeyTimeout: defaultValidateServiceAccountKeyTimeout, }, }, { name: "nil attributes", attrs: nil, expected: &Config{ - Scopes: []string{defaultGCPScope}, + Scopes: []string{defaultGCPScope}, + validateServiceAccountKeyTimeout: defaultValidateServiceAccountKeyTimeout, }, }, { @@ -154,12 +156,13 @@ func TestGetCredentialsConfig(t *testing.T) { ClientEmail: "test@test.com", }, expected: &Config{ - ProjectId: "test-project", - Zone: "us-central-1a", - ClientEmail: "test@test.com", - PrivateKey: "test-private-key", - PrivateKeyId: "test-private-key-id", - Scopes: []string{defaultGCPScope}, + ProjectId: "test-project", + Zone: "us-central-1a", + ClientEmail: "test@test.com", + PrivateKey: "test-private-key", + PrivateKeyId: "test-private-key-id", + Scopes: []string{defaultGCPScope}, + validateServiceAccountKeyTimeout: defaultValidateServiceAccountKeyTimeout, }, }, { @@ -220,12 +223,13 @@ func TestGetCredentialsConfig(t *testing.T) { Zone: "us-central-1a", }, expected: &Config{ - ProjectId: "test-project", - Zone: "us-central-1a", - PrivateKey: "test-private-key", - PrivateKeyId: "test-private-key-id", - ClientEmail: "test@test.com", - Scopes: []string{defaultGCPScope}, + ProjectId: "test-project", + Zone: "us-central-1a", + PrivateKey: "test-private-key", + PrivateKeyId: "test-private-key-id", + ClientEmail: "test@test.com", + Scopes: []string{defaultGCPScope}, + validateServiceAccountKeyTimeout: defaultValidateServiceAccountKeyTimeout, }, }, { @@ -241,13 +245,14 @@ func TestGetCredentialsConfig(t *testing.T) { TargetServiceAccountId: "test-target-service-account", }, expected: &Config{ - ProjectId: "test-project", - Zone: "us-central-1a", - PrivateKey: "test-private-key", - PrivateKeyId: "test-private-key-id", - ClientEmail: "test@test.com", - TargetServiceAccountId: "test-target-service-account", - Scopes: []string{defaultGCPScope}, + ProjectId: "test-project", + Zone: "us-central-1a", + PrivateKey: "test-private-key", + PrivateKeyId: "test-private-key-id", + ClientEmail: "test@test.com", + TargetServiceAccountId: "test-target-service-account", + Scopes: []string{defaultGCPScope}, + validateServiceAccountKeyTimeout: defaultValidateServiceAccountKeyTimeout, }, }, } diff --git a/internal/credential/config.go b/internal/credential/config.go index 07dc8aa..a05fa84 100644 --- a/internal/credential/config.go +++ b/internal/credential/config.go @@ -41,17 +41,21 @@ const ( // and then controlling the service account's access by granting it IAM roles. // https://cloud.google.com/compute/docs/access/service-accounts#scopes_best_practice defaultGCPScope = "https://www.googleapis.com/auth/cloud-platform" + // defaultValidateServiceAccountKeyTimeout is the timeout for validating + // a service account key. + defaultValidateServiceAccountKeyTimeout = 5 * time.Second ) // Config is the configuration for the GCP credential. type Config struct { - ProjectId string - PrivateKey string - PrivateKeyId string - Zone string - ClientEmail string - TargetServiceAccountId string - Scopes []string + ProjectId string + PrivateKey string + PrivateKeyId string + Zone string + ClientEmail string + TargetServiceAccountId string + Scopes []string + validateServiceAccountKeyTimeout time.Duration } // credentials represents a simplified version of the GCP credentials file format. @@ -72,13 +76,14 @@ func NewConfig(opt ...Option) (*Config, error) { 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, + ProjectId: opts.WithProjectId, + PrivateKey: opts.WithPrivateKey, + PrivateKeyId: opts.WithPrivateKeyId, + ClientEmail: opts.WithClientEmail, + TargetServiceAccountId: opts.WithTargetServiceAccountId, + Zone: opts.WithZone, + Scopes: opts.WithScopes, + validateServiceAccountKeyTimeout: defaultValidateServiceAccountKeyTimeout, } return c, nil } @@ -97,13 +102,14 @@ func (c *Config) toCredentials() *credentials { // clone returns a copy of the configuration. func (c *Config) clone() *Config { return &Config{ - ProjectId: c.ProjectId, - Zone: c.Zone, - PrivateKey: c.PrivateKey, - PrivateKeyId: c.PrivateKeyId, - ClientEmail: c.ClientEmail, - TargetServiceAccountId: c.TargetServiceAccountId, - Scopes: c.Scopes, + ProjectId: c.ProjectId, + Zone: c.Zone, + PrivateKey: c.PrivateKey, + PrivateKeyId: c.PrivateKeyId, + ClientEmail: c.ClientEmail, + TargetServiceAccountId: c.TargetServiceAccountId, + Scopes: c.Scopes, + validateServiceAccountKeyTimeout: c.validateServiceAccountKeyTimeout, } } diff --git a/internal/credential/config_test.go b/internal/credential/config_test.go index f7e9e22..400a6fa 100644 --- a/internal/credential/config_test.go +++ b/internal/credential/config_test.go @@ -403,13 +403,14 @@ func TestNewConfig(t *testing.T) { 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"}, + 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"}, + validateServiceAccountKeyTimeout: defaultValidateServiceAccountKeyTimeout, }, expectError: false, }, @@ -417,7 +418,8 @@ func TestNewConfig(t *testing.T) { name: "No Options", options: []Option{}, expected: &Config{ - Scopes: []string{defaultGCPScope}, + Scopes: []string{defaultGCPScope}, + validateServiceAccountKeyTimeout: defaultValidateServiceAccountKeyTimeout, }, expectError: false, }, @@ -442,13 +444,14 @@ func TestNewConfig(t *testing.T) { WithZone("test-zone"), }, 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{defaultGCPScope}, + 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{defaultGCPScope}, + validateServiceAccountKeyTimeout: defaultValidateServiceAccountKeyTimeout, }, expectError: false, }, diff --git a/internal/credential/rotate.go b/internal/credential/rotate.go index c83158e..7f1c518 100644 --- a/internal/credential/rotate.go +++ b/internal/credential/rotate.go @@ -32,9 +32,6 @@ const ( // required to delete a service account key. // This permission is required to rotate service account keys. IAMServiceAccountKeysDeletePermission = "iam.serviceAccountKeys.delete" - // validateServiceAccountKeyTimeout is the timeout for validating - // a service account key. - validateServiceAccountKeyTimeout = 5 * time.Second ) // ServiceAccountPrivateKey represents a decoded PrivateKeyData @@ -161,19 +158,31 @@ func (c *Config) ValidateServiceAccountKey( var validatePermissionsErr error var validationCallbackErr error - for start := time.Now(); time.Since(start) < validateServiceAccountKeyTimeout; time.Sleep(200 * time.Millisecond) { - _, validatePermissionsErr = c.ValidateIamPermissions(ctx, permissions, opts...) + ticker := time.NewTicker(200 * time.Millisecond) + defer ticker.Stop() - if validateCredsCallback != nil { - validationCallbackErr = validateCredsCallback(c, opts...) - } + ctx, cancel := context.WithTimeout(ctx, c.validateServiceAccountKeyTimeout) + defer cancel() - if validatePermissionsErr == nil && validationCallbackErr == nil { - return nil + for { + select { + case <-ctx.Done(): + if ctx.Err() == context.DeadlineExceeded { + return fmt.Errorf("failed to validate service account key before timeout: %w", errors.Join(validatePermissionsErr, validationCallbackErr)) + } + return ctx.Err() + case <-ticker.C: + _, validatePermissionsErr = c.ValidateIamPermissions(ctx, permissions, opts...) + + if validateCredsCallback != nil { + validationCallbackErr = validateCredsCallback(c, opts...) + } + + if validatePermissionsErr == nil && validationCallbackErr == nil { + return nil + } } } - - return errors.Join(validatePermissionsErr, validationCallbackErr) } func (c *Config) DeletePrivateKey(ctx context.Context, opts ...option.ClientOption) error { diff --git a/internal/credential/rotate_test.go b/internal/credential/rotate_test.go index 401630c..278f764 100644 --- a/internal/credential/rotate_test.go +++ b/internal/credential/rotate_test.go @@ -7,6 +7,7 @@ import ( "context" "errors" "testing" + "time" "cloud.google.com/go/iam/admin/apiv1/adminpb" "cloud.google.com/go/iam/apiv1/iampb" @@ -30,6 +31,7 @@ func TestRotateServiceAccountKey(t *testing.T) { }, }, } + testValidateTimeout := 1 * time.Second tests := []struct { name string @@ -76,9 +78,10 @@ func TestRotateServiceAccountKey(t *testing.T) { { name: "DeleteServiceAccountKey returns error", config: &Config{ - PrivateKey: "some-private-key", - PrivateKeyId: "some-private-key-id", - ClientEmail: "client@example.com", + PrivateKey: "some-private-key", + PrivateKeyId: "some-private-key-id", + ClientEmail: "client@example.com", + validateServiceAccountKeyTimeout: testValidateTimeout, }, setup: func() { testIAMAdminServer.testCreateServiceAccountKeyError = nil @@ -89,9 +92,10 @@ func TestRotateServiceAccountKey(t *testing.T) { { name: "Successful rotation", config: &Config{ - PrivateKey: "some-private-key", - PrivateKeyId: "some-private-key-id", - ClientEmail: "client@example.com", + PrivateKey: "some-private-key", + PrivateKeyId: "some-private-key-id", + ClientEmail: "client@example.com", + validateServiceAccountKeyTimeout: testValidateTimeout, }, setup: func() { testIAMAdminServer.testCreateServiceAccountKeyError = nil @@ -261,6 +265,104 @@ func TestValidateIamPermissions(t *testing.T) { }) } } + +func TestValidateServiceAccountKey(t *testing.T) { + ctx := context.Background() + testResourceServer := &testResourceServer{} + testValidateTimeout := 1 * time.Second + + tests := []struct { + name string + config *Config + permissions []string + validateCredsCallback ValidateCredsCallback + testIamPermissionsError error + expectedError error + }{ + { + name: "Successful validation", + config: &Config{ + ProjectId: "test-project-id", + PrivateKey: "test-private-key", + ClientEmail: "test@test.com", + validateServiceAccountKeyTimeout: testValidateTimeout, + }, + permissions: []string{ + ComputeInstancesListPermission, + IAMServiceAccountKeysCreatePermission, + IAMServiceAccountKeysDeletePermission, + }, + validateCredsCallback: func(c *Config, opts ...googleOption.ClientOption) error { + return nil + }, + expectedError: nil, + }, + { + name: "Validation callback returns error", + config: &Config{ + ProjectId: "test-project-id", + PrivateKey: "test-private-key", + ClientEmail: "test@test.com", + validateServiceAccountKeyTimeout: testValidateTimeout, + }, + permissions: []string{ + ComputeInstancesListPermission, + IAMServiceAccountKeysCreatePermission, + IAMServiceAccountKeysDeletePermission, + }, + validateCredsCallback: func(c *Config, opts ...googleOption.ClientOption) error { + return errors.New("validation callback error") + }, + expectedError: errors.New("validation callback error"), + }, + { + name: "IAM permissions validation returns error", + config: &Config{ + ProjectId: "test-project-id", + PrivateKey: "test-private-key", + ClientEmail: "test@test.com", + validateServiceAccountKeyTimeout: testValidateTimeout, + }, + permissions: []string{ + ComputeInstancesListPermission, + IAMServiceAccountKeysCreatePermission, + IAMServiceAccountKeysDeletePermission, + }, + testIamPermissionsError: errors.New("IAM permissions error"), + expectedError: errors.New("failed to validate IAM permissions"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testResourceServer.testIamPermissionsResponse = &iampb.TestIamPermissionsResponse{ + Permissions: tt.permissions, + } + testResourceServer.testIamPermissionsError = tt.testIamPermissionsError + + gsrv := NewGRPCServer() + resourcemanagerpb.RegisterProjectsServer(gsrv.Server, testResourceServer) + addr, err := gsrv.Start() + require.NoError(t, err) + + testOptions := []googleOption.ClientOption{ + googleOption.WithEndpoint(addr), + googleOption.WithoutAuthentication(), + googleOption.WithGRPCDialOption(grpc.WithTransportCredentials(insecure.NewCredentials())), + googleOption.WithTokenSource(nil), + } + + err = tt.config.ValidateServiceAccountKey(ctx, tt.permissions, tt.validateCredsCallback, testOptions...) + if tt.expectedError != nil { + require.ErrorContains(t, err, tt.expectedError.Error()) + return + } + + require.NoError(t, err) + }) + } +} + func TestDeletePrivateKey(t *testing.T) { testIAMAdminServer := &testIAMAdminServer{}