Browse Source

fix(gcpsm): Improve SecretExists method in GCP secret manager provider (#5610)

Co-authored-by: Gergely Brautigam <skarlso777@gmail.com>
Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Sohail Ahmed 3 months ago
parent
commit
c4241b506c

+ 1 - 1
providers/v1/gcp/go.mod

@@ -17,6 +17,7 @@ require (
 	google.golang.org/api v0.253.0
 	google.golang.org/genproto v0.0.0-20251022142026-3a174f9686a8
 	google.golang.org/grpc v1.76.0
+	google.golang.org/protobuf v1.36.10
 	grpc.go4.org v0.0.0-20170609214715-11d0a25b4919
 	k8s.io/api v0.34.1
 	k8s.io/apiextensions-apiserver v0.34.1
@@ -100,7 +101,6 @@ require (
 	gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20251014184007-4626949a642f // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20251014184007-4626949a642f // indirect
-	google.golang.org/protobuf v1.36.10 // indirect
 	gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect

+ 39 - 36
providers/v1/gcp/secretmanager/client.go

@@ -146,42 +146,16 @@ func parseError(err error) error {
 	return err
 }
 
-// SecretExists checks if a secret exists in Google Cloud Secret Manager.
-// It verifies the existence of a secret in Google Cloud Secret Manager AND that it has at least one version.
+// SecretExists checks if a secret exists in Google Cloud Secret Manager and has at least one accessible version.
 func (c *Client) SecretExists(ctx context.Context, ref esv1.PushSecretRemoteRef) (bool, error) {
-	secretName := fmt.Sprintf(globalSecretPath, c.store.ProjectID, ref.GetRemoteKey())
-	if c.store.Location != "" {
-		secretName = fmt.Sprintf(regionalSecretPath, c.store.ProjectID, c.store.Location, ref.GetRemoteKey())
-	}
-	gcpSecret, err := c.smClient.GetSecret(ctx, &secretmanagerpb.GetSecretRequest{
-		Name: secretName,
-	})
-	if err != nil {
-		if status.Code(err) == codes.NotFound {
-			return false, nil
-		}
-
-		return false, err
-	}
-
-	if gcpSecret == nil {
-		return false, nil
-	}
-	// Check if the secret has at least one version
-	versionName := fmt.Sprintf("%s/versions/latest", secretName)
-	_, err = c.smClient.AccessSecretVersion(ctx, &secretmanagerpb.AccessSecretVersionRequest{
-		Name: versionName,
-	})
-	metrics.ObserveAPICall(constants.ProviderGCPSM, constants.CallGCPSMAccessSecretVersion, err)
-
+	secretName := getName(c.store.ProjectID, c.store.Location, ref.GetRemoteKey())
+	_, err := getLatestEnabledVersion(ctx, c.smClient, secretName)
 	if err != nil {
 		if status.Code(err) == codes.NotFound {
-			// Secret exists but has no versions
 			return false, nil
 		}
 		return false, err
 	}
-
 	return true, nil
 }
 
@@ -215,7 +189,7 @@ func (c *Client) PushSecret(ctx context.Context, secret *corev1.Secret, pushSecr
 			return err
 		}
 
-		var replication = &secretmanagerpb.Replication{
+		replication := &secretmanagerpb.Replication{
 			Replication: &secretmanagerpb.Replication_Automatic_{
 				Automatic: &secretmanagerpb.Replication_Automatic{},
 			},
@@ -713,6 +687,8 @@ func getDataByProperty(data []byte, property string) (gjson.Result, error) {
 	return gjson.Get(payload, property), nil
 }
 
+// getName constructs the full resource name for a GCP secret.
+// If location is provided, it returns a regional secret path; otherwise, it returns a global secret path.
 func getName(projectID, location, key string) string {
 	if location != "" {
 		return fmt.Sprintf(regionalSecretPath, projectID, location, key)
@@ -720,6 +696,8 @@ func getName(projectID, location, key string) string {
 	return fmt.Sprintf(globalSecretPath, projectID, key)
 }
 
+// getParentName constructs the parent resource name for listing secrets in GCP.
+// If location is provided, it returns a regional parent path; otherwise, it returns a global parent path.
 func getParentName(projectID, location string) string {
 	if location != "" {
 		return fmt.Sprintf(regionalSecretParentPath, projectID, location)
@@ -727,31 +705,56 @@ func getParentName(projectID, location string) string {
 	return fmt.Sprintf(globalSecretParentPath, projectID)
 }
 
+// isErrSecretDestroyedOrDisabled checks if an error indicates that a secret version
+// is in DESTROYED or DISABLED state. These states occur when a version has been
+// explicitly destroyed or disabled and cannot be accessed.
 func isErrSecretDestroyedOrDisabled(err error) bool {
 	st, _ := status.FromError(err)
 	return st.Code() == codes.FailedPrecondition &&
 		(strings.Contains(st.Message(), "DESTROYED state") || strings.Contains(st.Message(), "DISABLED state"))
 }
 
+// getLatestEnabledVersion retrieves the most recent enabled version of a secret.
+// It lists all enabled versions and selects the one with the latest creation time.
+// If no enabled versions are found, it falls back to accessing the "latest" version,
+// which will return an appropriate error if the secret doesn't exist or has no accessible versions.
+//
+// Note: The version.Name field contains the full resource path (e.g., projects/*/secrets/*/versions/*),
+// not just the version number, so it's used directly in the AccessSecretVersion request.
 func getLatestEnabledVersion(ctx context.Context, client GoogleSecretManagerClient, name string) (*secretmanagerpb.AccessSecretVersionResponse, error) {
 	iter := client.ListSecretVersions(ctx, &secretmanagerpb.ListSecretVersionsRequest{
 		Parent: name,
 		Filter: "state:ENABLED",
 	})
+
 	latestCreateTime := time.Unix(0, 0)
-	latestVersion := &secretmanagerpb.SecretVersion{}
+	var versionName string
 	for {
 		version, err := iter.Next()
-		if errors.Is(err, iterator.Done) {
-			break
+		if err != nil {
+			if errors.Is(err, iterator.Done) {
+				break
+			}
+			return nil, err
 		}
 		if version.CreateTime.AsTime().After(latestCreateTime) {
 			latestCreateTime = version.CreateTime.AsTime()
-			latestVersion = version
+			versionName = version.Name
 		}
 	}
+
+	// If no enabled versions found, fall back to "latest"
+	// This will return the appropriate error (NotFound, FailedPrecondition, etc.)
+	if versionName == "" {
+		versionName = fmt.Sprintf("%s/versions/latest", name)
+	}
 	req := &secretmanagerpb.AccessSecretVersionRequest{
-		Name: fmt.Sprintf("%s/versions/%s", name, latestVersion.Name),
+		Name: versionName,
+	}
+	version, err := client.AccessSecretVersion(ctx, req)
+	metrics.ObserveAPICall(constants.ProviderGCPSM, constants.CallGCPSMAccessSecretVersion, err)
+	if err != nil {
+		return nil, err
 	}
-	return client.AccessSecretVersion(ctx, req)
+	return version, nil
 }

+ 200 - 100
providers/v1/gcp/secretmanager/client_test.go

@@ -23,6 +23,7 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	"cloud.google.com/go/secretmanager/apiv1/secretmanagerpb"
 	"github.com/googleapis/gax-go/v2"
@@ -31,6 +32,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/types/known/timestamppb"
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	pointer "k8s.io/utils/ptr"
@@ -79,7 +81,7 @@ func makeValidSecretManagerTestCase() *secretManagerTestCase {
 		latestEnabledSecretPolicy: esv1.SecretVersionSelectionPolicyLatestOrFail,
 	}
 	smtc.mockClient.NilClose()
-	smtc.mockClient.WithValue(context.Background(), smtc.apiInput, smtc.apiOutput, smtc.apiErr)
+	smtc.mockClient.WithValue(context.TODO(), smtc.apiInput, smtc.apiOutput, smtc.apiErr)
 	return &smtc
 }
 
@@ -109,7 +111,7 @@ func makeValidSecretManagerTestCaseCustom(tweaks ...func(smtc *secretManagerTest
 	for _, fn := range tweaks {
 		fn(smtc)
 	}
-	smtc.mockClient.WithValue(context.Background(), smtc.apiInput, smtc.apiOutput, smtc.apiErr)
+	smtc.mockClient.WithValue(context.TODO(), smtc.apiInput, smtc.apiOutput, smtc.apiErr)
 	return smtc
 }
 
@@ -214,7 +216,7 @@ func TestSecretManagerGetSecret(t *testing.T) {
 	for k, v := range successCases {
 		sm.store = &esv1.GCPSMProvider{ProjectID: v.projectID, SecretVersionSelectionPolicy: v.latestEnabledSecretPolicy}
 		sm.smClient = v.mockClient
-		out, err := sm.GetSecret(context.Background(), *v.ref)
+		out, err := sm.GetSecret(t.Context(), *v.ref)
 		if !ErrorContains(err, v.expectError) {
 			t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError)
 		}
@@ -401,7 +403,7 @@ func TestGetSecretMetadataPolicyFetch(t *testing.T) {
 					ProjectID: "foo",
 				},
 			}
-			got, err := client.GetSecret(context.TODO(), tc.ref)
+			got, err := client.GetSecret(t.Context(), tc.ref)
 			if tc.expectedErr != "" {
 				if err == nil {
 					t.Fatalf("expected to receive an error but got nit")
@@ -450,7 +452,6 @@ func TestDeleteSecret(t *testing.T) {
 				client: fakeClient,
 				getSecretOutput: fakesm.SecretMockReturn{
 					Secret: &secretmanagerpb.Secret{
-
 						Name: testSecretName,
 						Labels: map[string]string{
 							managedBy: externalSecrets,
@@ -465,7 +466,6 @@ func TestDeleteSecret(t *testing.T) {
 				client: fakeClient,
 				getSecretOutput: fakesm.SecretMockReturn{
 					Secret: &secretmanagerpb.Secret{
-
 						Name:   testSecretName,
 						Labels: map[string]string{},
 					},
@@ -518,7 +518,7 @@ func TestDeleteSecret(t *testing.T) {
 			}
 			tc.args.client.NewGetSecretFn(tc.args.getSecretOutput)
 			tc.args.client.NewDeleteSecretFn(tc.args.deleteSecretErr)
-			err := client.DeleteSecret(context.TODO(), ref)
+			err := client.DeleteSecret(t.Context(), ref)
 			// Error nil XOR tc.want.err nil
 			if ((err == nil) || (tc.want.err == nil)) && !((err == nil) && (tc.want.err == nil)) {
 				t.Errorf("\nTesting SetSecret:\nName: %v\nReason: %v\nWant error: %v\nGot error: %v", name, tc.reason, tc.want.err, err)
@@ -600,25 +600,25 @@ func TestPushSecret(t *testing.T) {
 		expectedData:   map[string][]byte{},
 	}
 
-	var payload = secretmanagerpb.SecretPayload{
+	payload := secretmanagerpb.SecretPayload{
 		Data: []byte("payload"),
 	}
 
-	var payload2 = secretmanagerpb.SecretPayload{
+	payload2 := secretmanagerpb.SecretPayload{
 		Data: []byte("fake-value"),
 	}
 
-	var res = secretmanagerpb.AccessSecretVersionResponse{
+	res := secretmanagerpb.AccessSecretVersionResponse{
 		Name:    "projects/default/secrets/foo-bar",
 		Payload: &payload,
 	}
 
-	var res2 = secretmanagerpb.AccessSecretVersionResponse{
+	res2 := secretmanagerpb.AccessSecretVersionResponse{
 		Name:    "projects/default/secrets/baz",
 		Payload: &payload2,
 	}
 
-	var secretVersion = secretmanagerpb.SecretVersion{}
+	secretVersion := secretmanagerpb.SecretVersion{}
 
 	type args struct {
 		store                         *esv1.GCPSMProvider
@@ -648,7 +648,8 @@ func TestPushSecret(t *testing.T) {
 				mock:                          smtc.mockClient,
 				GetSecretMockReturn:           fakesm.SecretMockReturn{Secret: &secret, Err: nil},
 				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res, Err: nil},
-				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil}},
+				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil},
+			},
 			want: want{
 				err: nil,
 			},
@@ -685,7 +686,8 @@ func TestPushSecret(t *testing.T) {
 					},
 				}, Err: nil},
 				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res, Err: nil},
-				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil}},
+				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil},
+			},
 			want: want{
 				err: nil,
 			},
@@ -727,7 +729,8 @@ func TestPushSecret(t *testing.T) {
 					},
 				}, Err: nil},
 				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res, Err: nil},
-				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil}},
+				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil},
+			},
 			want: want{
 				err: nil,
 				req: func(m *fakesm.MockSMClient) error {
@@ -783,7 +786,8 @@ func TestPushSecret(t *testing.T) {
 					},
 				}, Err: nil},
 				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res, Err: nil},
-				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil}},
+				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil},
+			},
 			want: want{
 				err: nil,
 				req: func(m *fakesm.MockSMClient) error {
@@ -815,7 +819,8 @@ func TestPushSecret(t *testing.T) {
 				CreateSecretMockReturn:        fakesm.SecretMockReturn{Secret: &secretWithTopics, Err: nil},
 				GetSecretMockReturn:           fakesm.SecretMockReturn{Secret: nil, Err: notFoundError},
 				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res, Err: nil},
-				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil}},
+				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil},
+			},
 			want: want{
 				err: nil,
 				req: func(m *fakesm.MockSMClient) error {
@@ -952,7 +957,8 @@ func TestPushSecret(t *testing.T) {
 				mock:                          smtc.mockClient,
 				GetSecretMockReturn:           fakesm.SecretMockReturn{Secret: &secret, Err: nil},
 				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res, Err: nil},
-				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil}},
+				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil},
+			},
 			want: want{
 				err: nil,
 			},
@@ -982,7 +988,7 @@ func TestPushSecret(t *testing.T) {
 				RemoteKey: "/baz",
 			}
 
-			err := c.PushSecret(context.Background(), s, data)
+			err := c.PushSecret(t.Context(), s, data)
 			if err != nil {
 				if tc.want.err == nil {
 					t.Errorf("received an unexpected error: %v", err)
@@ -1009,108 +1015,197 @@ func TestPushSecret(t *testing.T) {
 
 func TestSecretExists(t *testing.T) {
 	tests := []struct {
-		name                          string
-		ref                           esv1.PushSecretRemoteRef
-		getSecretMockReturn           fakesm.SecretMockReturn
-		accessSecretVersionMockReturn fakesm.AccessSecretVersionMockReturn
-		expectedSecret                bool
-		expectedErr                   func(t *testing.T, err error)
+		name           string
+		ref            v1alpha1.PushSecretRemoteRef
+		location       string
+		setupMock      func(*fakesm.MockSMClient)
+		expectedExists bool
+		expectedErr    func(t *testing.T, err error)
 	}{
 		{
-			name: "secret exists",
+			name: "secret and accessible version exist",
 			ref: v1alpha1.PushSecretRemoteRef{
 				RemoteKey: "bar",
 			},
-			getSecretMockReturn: fakesm.SecretMockReturn{
-				Secret: &secretmanagerpb.Secret{
-					Name: testSecretName,
-				},
-				Err: nil,
-			},
-			accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{
-				Res: &secretmanagerpb.AccessSecretVersionResponse{
-					Name: "projects/foo/secrets/bar/versions/latest",
-					Payload: &secretmanagerpb.SecretPayload{
-						Data: []byte("test-data"),
+			setupMock: func(mc *fakesm.MockSMClient) {
+				// Mock ListSecretVersions to return an iterator with enabled versions
+				mc.NewListSecretVersionsFn(fakesm.ListSecretVersionsMockReturn{
+					Res: fakesm.NewSecretVersionIterator([]*secretmanagerpb.SecretVersion{
+						{
+							Name:       "projects/foo/secrets/bar/versions/1",
+							State:      secretmanagerpb.SecretVersion_ENABLED,
+							CreateTime: timestamppb.New(time.Now()),
+						},
+					}),
+				})
+				mc.NewAccessSecretVersionFn(fakesm.AccessSecretVersionMockReturn{
+					Res: &secretmanagerpb.AccessSecretVersionResponse{
+						Name: "projects/foo/secrets/bar/versions/1",
+						Payload: &secretmanagerpb.SecretPayload{
+							Data: []byte("test-data"),
+						},
 					},
-				},
-				Err: nil,
+					Err: nil,
+				})
 			},
-			expectedSecret: true,
+			expectedExists: true,
 			expectedErr: func(t *testing.T, err error) {
 				require.NoError(t, err)
 			},
 		},
 		{
-			name: "secret exists but has no versions",
+			name: "multiple versions exist - selects latest by create time",
 			ref: v1alpha1.PushSecretRemoteRef{
 				RemoteKey: "bar",
 			},
-			getSecretMockReturn: fakesm.SecretMockReturn{
-				Secret: &secretmanagerpb.Secret{
-					Name: testSecretName,
-				},
-				Err: nil,
-			},
-			accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{
-				Res: nil,
-				Err: status.Errorf(codes.NotFound, "secret version not found"),
+			setupMock: func(mc *fakesm.MockSMClient) {
+				// Mock ListSecretVersions to return multiple versions with different create times
+				now := time.Now()
+				mc.NewListSecretVersionsFn(fakesm.ListSecretVersionsMockReturn{
+					Res: fakesm.NewSecretVersionIterator([]*secretmanagerpb.SecretVersion{
+						{
+							Name:       "projects/foo/secrets/bar/versions/1",
+							State:      secretmanagerpb.SecretVersion_ENABLED,
+							CreateTime: timestamppb.New(now.Add(-2 * time.Hour)),
+						},
+						{
+							Name:       "projects/foo/secrets/bar/versions/3",
+							State:      secretmanagerpb.SecretVersion_ENABLED,
+							CreateTime: timestamppb.New(now), // Most recent
+						},
+						{
+							Name:       "projects/foo/secrets/bar/versions/2",
+							State:      secretmanagerpb.SecretVersion_ENABLED,
+							CreateTime: timestamppb.New(now.Add(-1 * time.Hour)),
+						},
+					}),
+				})
+				mc.NewAccessSecretVersionFn(fakesm.AccessSecretVersionMockReturn{
+					Res: &secretmanagerpb.AccessSecretVersionResponse{
+						Name: "projects/foo/secrets/bar/versions/3",
+						Payload: &secretmanagerpb.SecretPayload{
+							Data: []byte("latest-data"),
+						},
+					},
+					Err: nil,
+				})
 			},
-			expectedSecret: false,
+			expectedExists: true,
 			expectedErr: func(t *testing.T, err error) {
 				require.NoError(t, err)
 			},
 		},
 		{
-			name: "secret does not exists",
+			name: "secret does not exist - NotFound error",
 			ref: v1alpha1.PushSecretRemoteRef{
 				RemoteKey: "bar",
 			},
-			getSecretMockReturn: fakesm.SecretMockReturn{
-				Secret: nil,
-				Err:    status.Errorf(codes.NotFound, "secret not found"),
-			},
-			accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{},
-			expectedSecret:                false,
+			setupMock: func(mc *fakesm.MockSMClient) {
+				// Mock ListSecretVersions to return an empty iterator (no enabled versions)
+				mc.NewListSecretVersionsFn(fakesm.ListSecretVersionsMockReturn{
+					Res: fakesm.NewSecretVersionIterator([]*secretmanagerpb.SecretVersion{}),
+				})
+				mc.NewAccessSecretVersionFn(fakesm.AccessSecretVersionMockReturn{
+					Res: nil,
+					Err: status.Error(codes.NotFound, "secret not found"),
+				})
+			},
+			expectedExists: false,
 			expectedErr: func(t *testing.T, err error) {
 				require.NoError(t, err)
 			},
 		},
 		{
-			name: "unexpected error when getting secret",
+			name: "unexpected error occurs on access secret version",
 			ref: v1alpha1.PushSecretRemoteRef{
-				RemoteKey: "bar2",
+				RemoteKey: "bar",
 			},
-			getSecretMockReturn: fakesm.SecretMockReturn{
-				Secret: &secretmanagerpb.Secret{
-					Name: testSecretName,
-				},
-				Err: errors.New("some error"),
+			setupMock: func(mc *fakesm.MockSMClient) {
+				// Mock ListSecretVersions to return an empty iterator
+				mc.NewListSecretVersionsFn(fakesm.ListSecretVersionsMockReturn{
+					Res: fakesm.NewSecretVersionIterator([]*secretmanagerpb.SecretVersion{}),
+				})
+				mc.NewAccessSecretVersionFn(fakesm.AccessSecretVersionMockReturn{
+					Res: nil,
+					Err: status.Error(codes.PermissionDenied, "permission denied"),
+				})
+			},
+			expectedExists: false,
+			expectedErr: func(t *testing.T, err error) {
+				assert.ErrorContains(t, err, "permission denied")
+			},
+		},
+		{
+			name: "regional secret exists",
+			ref: v1alpha1.PushSecretRemoteRef{
+				RemoteKey: "bar",
+			},
+			location: "us-east1",
+			setupMock: func(mc *fakesm.MockSMClient) {
+				// Mock ListSecretVersions to return an iterator with enabled versions
+				mc.NewListSecretVersionsFn(fakesm.ListSecretVersionsMockReturn{
+					Res: fakesm.NewSecretVersionIterator([]*secretmanagerpb.SecretVersion{
+						{
+							Name:       "projects/foo/locations/us-east1/secrets/bar/versions/1",
+							State:      secretmanagerpb.SecretVersion_ENABLED,
+							CreateTime: timestamppb.New(time.Now()),
+						},
+					}),
+				})
+				mc.NewAccessSecretVersionFn(fakesm.AccessSecretVersionMockReturn{
+					Res: &secretmanagerpb.AccessSecretVersionResponse{
+						Name: "projects/foo/locations/us-east1/secrets/bar/versions/1",
+						Payload: &secretmanagerpb.SecretPayload{
+							Data: []byte("test-data"),
+						},
+					},
+					Err: nil,
+				})
 			},
-			accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{},
-			expectedSecret:                false,
+			expectedExists: true,
 			expectedErr: func(t *testing.T, err error) {
-				assert.ErrorContains(t, err, "some error")
+				require.NoError(t, err)
 			},
 		},
 		{
-			name: "unexpected error occurs when accessing secret version",
+			name: "regional secret does not exist",
 			ref: v1alpha1.PushSecretRemoteRef{
-				RemoteKey: "bar3",
+				RemoteKey: "bar",
 			},
-			getSecretMockReturn: fakesm.SecretMockReturn{
-				Secret: &secretmanagerpb.Secret{
-					Name: testSecretName,
-				},
-				Err: nil,
+			location: "us-east1",
+			setupMock: func(mc *fakesm.MockSMClient) {
+				// Mock ListSecretVersions to return an empty iterator
+				mc.NewListSecretVersionsFn(fakesm.ListSecretVersionsMockReturn{
+					Res: fakesm.NewSecretVersionIterator([]*secretmanagerpb.SecretVersion{}),
+				})
+				mc.NewAccessSecretVersionFn(fakesm.AccessSecretVersionMockReturn{
+					Res: nil,
+					Err: status.Error(codes.NotFound, "regional secret not found"),
+				})
+			},
+			expectedExists: false,
+			expectedErr: func(t *testing.T, err error) {
+				require.NoError(t, err)
 			},
-			accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{
-				Res: nil,
-				Err: errors.New("version access error"),
+		},
+		{
+			name: "secret version does not exist - latest version is destroyed/disabled",
+			ref: v1alpha1.PushSecretRemoteRef{
+				RemoteKey: "bar",
 			},
-			expectedSecret: false,
+			setupMock: func(mc *fakesm.MockSMClient) {
+				// Mock ListSecretVersions to return an empty iterator (no enabled versions)
+				mc.NewListSecretVersionsFn(fakesm.ListSecretVersionsMockReturn{
+					Res: fakesm.NewSecretVersionIterator([]*secretmanagerpb.SecretVersion{}),
+				})
+				mc.NewAccessSecretVersionFn(fakesm.AccessSecretVersionMockReturn{
+					Res: nil,
+					Err: status.Error(codes.FailedPrecondition, "Secret version is in DESTROYED state"),
+				})
+			},
+			expectedExists: false,
 			expectedErr: func(t *testing.T, err error) {
-				assert.ErrorContains(t, err, "version access error")
+				assert.ErrorContains(t, err, "DESTROYED state")
 			},
 		},
 	}
@@ -1118,20 +1213,20 @@ func TestSecretExists(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			smClient := fakesm.MockSMClient{}
-			smClient.NewGetSecretFn(tc.getSecretMockReturn)
-			smClient.NewAccessSecretVersionFn(tc.accessSecretVersionMockReturn)
+			tc.setupMock(&smClient)
 
 			client := Client{
 				smClient: &smClient,
 				store: &esv1.GCPSMProvider{
 					ProjectID: "foo",
+					Location:  tc.location,
 				},
 			}
-			got, err := client.SecretExists(context.TODO(), tc.ref)
+			got, err := client.SecretExists(t.Context(), tc.ref)
 			tc.expectedErr(t, err)
 
-			if got != tc.expectedSecret {
-				t.Fatalf("unexpected secret: expected %t, got %t", tc.expectedSecret, got)
+			if got != tc.expectedExists {
+				t.Fatalf("unexpected result: expected %t, got %t", tc.expectedExists, got)
 			}
 		})
 	}
@@ -1142,7 +1237,7 @@ func TestSecretExistsWithRegionalEndpoint(t *testing.T) {
 		name                          string
 		location                      string
 		ref                           esv1.PushSecretRemoteRef
-		getSecretMockReturn           fakesm.SecretMockReturn
+		listSecretVersionsMockReturn  fakesm.ListSecretVersionsMockReturn
 		accessSecretVersionMockReturn fakesm.AccessSecretVersionMockReturn
 		expectedSecret                bool
 		expectedErr                   func(t *testing.T, err error)
@@ -1153,15 +1248,18 @@ func TestSecretExistsWithRegionalEndpoint(t *testing.T) {
 			ref: v1alpha1.PushSecretRemoteRef{
 				RemoteKey: "bar",
 			},
-			getSecretMockReturn: fakesm.SecretMockReturn{
-				Secret: &secretmanagerpb.Secret{
-					Name: testSecretName,
-				},
-				Err: nil,
+			listSecretVersionsMockReturn: fakesm.ListSecretVersionsMockReturn{
+				Res: fakesm.NewSecretVersionIterator([]*secretmanagerpb.SecretVersion{
+					{
+						Name:       "projects/foo/locations/us-east1/secrets/bar/versions/1",
+						State:      secretmanagerpb.SecretVersion_ENABLED,
+						CreateTime: timestamppb.New(time.Now()),
+					},
+				}),
 			},
 			accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{
 				Res: &secretmanagerpb.AccessSecretVersionResponse{
-					Name: "projects/foo/locations/us-east1/secrets/bar/versions/latest",
+					Name: "projects/foo/locations/us-east1/secrets/bar/versions/1",
 					Payload: &secretmanagerpb.SecretPayload{
 						Data: []byte("test-data"),
 					},
@@ -1179,12 +1277,14 @@ func TestSecretExistsWithRegionalEndpoint(t *testing.T) {
 			ref: v1alpha1.PushSecretRemoteRef{
 				RemoteKey: "bar",
 			},
-			getSecretMockReturn: fakesm.SecretMockReturn{
-				Secret: nil,
-				Err:    status.Errorf(codes.NotFound, "secret not found"),
+			listSecretVersionsMockReturn: fakesm.ListSecretVersionsMockReturn{
+				Res: fakesm.NewSecretVersionIterator([]*secretmanagerpb.SecretVersion{}),
+			},
+			accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{
+				Res: nil,
+				Err: status.Error(codes.NotFound, "secret not found"),
 			},
-			accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{},
-			expectedSecret:                false,
+			expectedSecret: false,
 			expectedErr: func(t *testing.T, err error) {
 				require.NoError(t, err)
 			},
@@ -1194,7 +1294,7 @@ func TestSecretExistsWithRegionalEndpoint(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			smClient := fakesm.MockSMClient{}
-			smClient.NewGetSecretFn(tc.getSecretMockReturn)
+			smClient.NewListSecretVersionsFn(tc.listSecretVersionsMockReturn)
 			smClient.NewAccessSecretVersionFn(tc.accessSecretVersionMockReturn)
 
 			client := Client{
@@ -1397,7 +1497,7 @@ func TestPushSecretProperty(t *testing.T) {
 				store:    &esv1.GCPSMProvider{},
 			}
 			s := &corev1.Secret{Data: map[string][]byte{secretKey: []byte(tc.payload)}}
-			err := client.PushSecret(context.Background(), s, tc.data)
+			err := client.PushSecret(t.Context(), s, tc.data)
 			if err != nil {
 				if tc.expectedErr == "" {
 					t.Fatalf("PushSecret returns unexpected error: %v", err)
@@ -1449,7 +1549,7 @@ func TestGetSecretMap(t *testing.T) {
 	for k, v := range successCases {
 		sm.store = &esv1.GCPSMProvider{ProjectID: v.projectID}
 		sm.smClient = v.mockClient
-		out, err := sm.GetSecretMap(context.Background(), *v.ref)
+		out, err := sm.GetSecretMap(t.Context(), *v.ref)
 		if !ErrorContains(err, v.expectError) {
 			t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError)
 		}

+ 95 - 0
providers/v1/gcp/secretmanager/fake/fake.go

@@ -20,12 +20,15 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"reflect"
+	"unsafe"
 
 	secretmanager "cloud.google.com/go/secretmanager/apiv1"
 	"cloud.google.com/go/secretmanager/apiv1/secretmanagerpb"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/googleapis/gax-go/v2"
+	"google.golang.org/api/iterator"
 )
 
 type MockSMClient struct {
@@ -59,6 +62,86 @@ type AddSecretVersionMockReturn struct {
 	Err           error
 }
 
+type ListSecretVersionsMockReturn struct {
+	Res *secretmanager.SecretVersionIterator
+}
+
+// NewSecretVersionIterator creates a mock SecretVersionIterator for testing.
+// It takes a slice of SecretVersion objects and returns an iterator that will
+// yield them on successive Next() calls. The iterator uses reflection to set
+// unexported fields required by the google.golang.org/api/iterator package.
+//
+// The iterator returns all provided versions on the first fetch, then returns
+// iterator.Done on subsequent calls to indicate no more items are available.
+func NewSecretVersionIterator(versions []*secretmanagerpb.SecretVersion) *secretmanager.SecretVersionIterator {
+	it := &secretmanager.SecretVersionIterator{}
+
+	// Simple helper to set an unexported field using reflection
+	setField := func(name string, value any) {
+		v := reflect.ValueOf(it).Elem()
+		field := v.FieldByName(name)
+		if field.IsValid() {
+			//#nosec G103 -- Audited this use and it's only for mocking in testing
+			reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())).
+				Elem().Set(reflect.ValueOf(value))
+		}
+	}
+
+	// Simple state: are we done?
+	done := false
+
+	// InternalFetch returns all versions on first call, then nothing
+	it.InternalFetch = func(pageSize int, pageToken string) ([]*secretmanagerpb.SecretVersion, string, error) {
+		if done {
+			return nil, "", nil
+		}
+		done = true
+		return versions, "", nil
+	}
+
+	// Simple fetch: call InternalFetch and store results in items field
+	fetch := func(pageSize int, pageToken string) (string, error) {
+		results, nextToken, err := it.InternalFetch(pageSize, pageToken)
+		if err != nil {
+			return "", err
+		}
+		setField("items", results)
+		return nextToken, nil
+	}
+
+	// bufLen: return length of items field
+	bufLen := func() int {
+		v := reflect.ValueOf(it).Elem()
+		itemsField := v.FieldByName("items")
+		if itemsField.IsValid() {
+			return itemsField.Len()
+		}
+		return 0
+	}
+
+	// takeBuf: return and clear items field
+	takeBuf := func() any {
+		v := reflect.ValueOf(it).Elem()
+		itemsField := v.FieldByName("items")
+		if itemsField.IsValid() {
+			items := itemsField.Interface()
+			setField("items", []*secretmanagerpb.SecretVersion(nil))
+			return items
+		}
+		return []*secretmanagerpb.SecretVersion(nil)
+	}
+
+	// Create the PageInfo and nextFunc using the iterator package
+	pageInfo, nextFunc := iterator.NewPageInfo(fetch, bufLen, takeBuf)
+
+	// Set the required unexported fields
+	setField("pageInfo", pageInfo)
+	setField("nextFunc", nextFunc)
+	setField("items", []*secretmanagerpb.SecretVersion(nil))
+
+	return it
+}
+
 type SecretMockReturn struct {
 	Secret *secretmanagerpb.Secret
 	Err    error
@@ -67,11 +150,13 @@ type SecretMockReturn struct {
 func (mc *MockSMClient) DeleteSecret(ctx context.Context, req *secretmanagerpb.DeleteSecretRequest, _ ...gax.CallOption) error {
 	return mc.DeleteSecretFn(ctx, req)
 }
+
 func (mc *MockSMClient) NewDeleteSecretFn(err error) {
 	mc.DeleteSecretFn = func(_ context.Context, _ *secretmanagerpb.DeleteSecretRequest, _ ...gax.CallOption) error {
 		return err
 	}
 }
+
 func (mc *MockSMClient) GetSecret(ctx context.Context, req *secretmanagerpb.GetSecretRequest, _ ...gax.CallOption) (*secretmanagerpb.Secret, error) {
 	return mc.GetSecretFn(ctx, req)
 }
@@ -95,6 +180,7 @@ func (mc *MockSMClient) NewAccessSecretVersionFn(mock AccessSecretVersionMockRet
 func (mc *MockSMClient) ListSecrets(ctx context.Context, req *secretmanagerpb.ListSecretsRequest, _ ...gax.CallOption) *secretmanager.SecretIterator {
 	return mc.ListSecretsFn(ctx, req)
 }
+
 func (mc *MockSMClient) Close() error {
 	return mc.closeFn()
 }
@@ -207,6 +293,15 @@ func (mc *MockSMClient) ListSecretVersions(ctx context.Context, req *secretmanag
 	return mc.ListSecretVersionsFn(ctx, req)
 }
 
+// NewListSecretVersionsFn configures the mock ListSecretVersions function to return
+// a predefined iterator. This is used in tests to control what versions are returned
+// when listing secret versions.
+func (mc *MockSMClient) NewListSecretVersionsFn(mock ListSecretVersionsMockReturn) {
+	mc.ListSecretVersionsFn = func(_ context.Context, _ *secretmanagerpb.ListSecretVersionsRequest, _ ...gax.CallOption) *secretmanager.SecretVersionIterator {
+		return mock.Res
+	}
+}
+
 func (mc *MockSMClient) WithValue(_ context.Context, req *secretmanagerpb.AccessSecretVersionRequest, val *secretmanagerpb.AccessSecretVersionResponse, err error) {
 	if mc != nil {
 		mc.accessSecretFn = func(paramCtx context.Context, paramReq *secretmanagerpb.AccessSecretVersionRequest, paramOpts ...gax.CallOption) (*secretmanagerpb.AccessSecretVersionResponse, error) {