fixup! update timeout for tests to avoid ctx deadline errors

- 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

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,
 			},
 		},
 	}

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,
 	}
 }
 

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,
 		},

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 {

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{}
 

plugin/service/host/attributes.go

@@ -108,7 +108,7 @@ func buildListInstancesRequest(attributes *SetAttributes, catalog *CatalogAttrib
 		return nil, fmt.Errorf("error building filters: %w", err)
 	}
 
-	if len(filters) > 1 {
+	if len(filters) > 0 {
 		filters := strings.Join(filters, " AND ")
 		request.Filter = &filters
 	}

plugin/service/host/attributes_test.go

@@ -183,6 +183,23 @@ func TestBuildListInstancesRequest(t *testing.T) {
 			},
 			expectedErrContains: "error building filters",
 		},
+		{
+			name: "valid request with no filters",
+			attributes: &SetAttributes{
+				Filters: []string{},
+			},
+			catalog: &CatalogAttributes{
+				CredentialAttributes: &cred.CredentialAttributes{
+					ProjectId: "test-12345",
+					Zone:      "us-central-1a",
+				},
+			},
+			expected: &computepb.ListInstancesRequest{
+				Project: "test-12345",
+				Zone:    "us-central-1a",
+				Filter:  stringPtr("status = running"),
+			},
+		},
 	}
 
 	for _, tc := range cases {

plugin/service/host/plugin.go

@@ -87,6 +87,10 @@ func (p *HostPlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalo
 		credential.IAMServiceAccountKeysDeletePermission,
 	}
 
+	if !credState.CredentialsConfig.IsRotatable() && !catalogAttributes.DisableCredentialRotation {
+		return nil, status.Error(codes.InvalidArgument, "cannot rotate credentials for non-rotatable credentials")
+	}
+
 	if credState.CredentialsConfig.IsRotatable() && !catalogAttributes.DisableCredentialRotation {
 		state, err := newGCPCatalogPersistedState(
 			append([]gcpCatalogPersistedStateOption{

plugin/service/host/plugin_test.go

@@ -878,6 +878,53 @@ func TestCreateCatalog(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "create with application default credentials and credential rotation disabled",
+			req: &pb.OnCreateCatalogRequest{
+				Catalog: &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.ConstDisableCredentialRotation: structpb.NewBoolValue(true),
+							},
+						},
+					},
+				},
+			},
+			catalogOpts: []gcpCatalogPersistedStateOption{
+				withTestInstancesAPIFunc(newTestMockInstances(ctx,
+					nil,
+					testMockInstancesWithListInstancesOutput(&computepb.InstanceList{}),
+					testMockInstancesWithListInstancesError(nil),
+				)),
+			},
+			expectedRsp: &pb.OnCreateCatalogResponse{
+				Persisted: &pb.HostCatalogPersisted{
+					Secrets: &structpb.Struct{
+						Fields: map[string]*structpb.Value{},
+					},
+				},
+			},
+		},
+		{
+			name: "create with application default credentials and credential rotation enabled",
+			req: &pb.OnCreateCatalogRequest{
+				Catalog: &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.ConstDisableCredentialRotation: structpb.NewBoolValue(false),
+							},
+						},
+					},
+				},
+			},
+			expectedErr: "cannot rotate credentials for non-rotatable credentials",
+		},
 	}
 
 	for _, tc := range cases {