CyberCoveLLC/threshold-plugin-gcp
1
0 Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat: update retry logic for validation call (#26)

Browse Source 
* feat: update retry logic for validation call

- update validation for service account validation call to use `select` instead of `sleep`
- add `validateServiceAccountKeyTimeout` to config. This makes it easier to control the timeout from tests so tests do not have to run for long with the default timeout
This commit is contained in:
Elim Tsiagbey authored and GitHub committed 2025-01-07 23:23:46 -05:00
1 parent e17cbdb140
commit 877d285eef
5 files changed
+201 -76
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/credential/attributes_test.go
+27 -22
View File
@@ -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,
},
},
}
internal/credential/config.go
+27 -21
View File
@@ -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,
}
}
internal/credential/config_test.go
+18 -15
View File
@@ -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,
},
internal/credential/rotate.go
+21 -12
View File
@@ -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 {
internal/credential/rotate_test.go
+108 -6
View File
@@ -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{}