Browse Source

feat(gcpsm): auto-detect projectID from GCP metadata server (#5922)

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Patrick Martin 1 month ago
parent
commit
b6539062f4

+ 58 - 2
docs/provider/google-secrets-manager.md

@@ -200,15 +200,71 @@ _For details and further information on WIF and Secret Manager permissions, refe
 * _[Authenticate to Google Cloud APIs from GKE workloads](https://cloud.google.com/kubernetes-engine/docs/how-to/workload-identity) in the GKE documentation._
 * _[Access control with IAM](https://cloud.google.com/secret-manager/docs/access-control) in the Secret Manager documentation._
 
-Once the Core Controller Pod can access the Secret Manager secret(s) through WIF via its Kubernetes service account, you can create `SecretStore` or `ClusterSecretStore` instances that only specify the GCP project ID, omitting the `auth` section entirely:
+Once the Core Controller Pod can access the Secret Manager secret(s) through WIF via its Kubernetes service account, you can create `SecretStore` or `ClusterSecretStore` instances without authentication configuration. You can optionally specify the GCP project ID, or omit it to use auto-detection from the GCP metadata server:
 
 ```yaml
 {% include 'gcpsm-wif-core-controller-secret-store.yaml' %}
 ```
 
+Alternatively, with projectID auto-detection (GKE only):
+
+```yaml
+apiVersion: external-secrets.io/v1beta1
+kind: SecretStore
+metadata:
+  name: gcp-secret-store
+  namespace: demo
+spec:
+  provider:
+    gcpsm: {} # Both projectID and auth are optional when using Core Controller authentication in GKE
+```
+
+#### Auto-detection of GCP project ID
+
+When creating a `SecretStore` or `ClusterSecretStore` that uses Workload Identity, Workload Identity Federation, or default credentials (ADC), the `projectID` field is optional. If omitted, the operator will automatically detect the GCP project ID from the [GCP metadata server](https://cloud.google.com/compute/docs/metadata/overview) when running in GKE.
+
+This allows you to create portable SecretStore configurations that work across multiple GCP projects without modification:
+
+```yaml
+apiVersion: external-secrets.io/v1beta1
+kind: SecretStore
+metadata:
+  name: gcp-secret-store
+spec:
+  provider:
+    gcpsm:
+      # projectID is optional - will be auto-detected from GCP metadata server
+      auth:
+        workloadIdentity:
+          serviceAccountRef:
+            name: demo-secrets-sa
+```
+
+You must set `projectID` explicitly when using static service account credentials (`auth.secretRef`), when running outside GKE, or when accessing secrets in a different project than your cluster. When running in GKE with Workload Identity, Workload Identity Federation, or default credentials, `projectID` can be omitted if the secrets live in the same project as the cluster.
+
+#### projectID vs clusterProjectID
+
+`projectID` (`spec.provider.gcpsm.projectID`) tells the provider which GCP project holds the secrets. It is used in secret resource paths like `projects/{projectID}/secrets/{name}`. For Workload Identity, it also serves as a fallback for authentication if `clusterProjectID` is not set.
+
+`clusterProjectID` (`spec.provider.gcpsm.auth.workloadIdentity.clusterProjectID`) identifies the project hosting the GKE cluster. It is only used by Workload Identity to build the identity pool URL. When either field is omitted in GKE, the provider queries the [GCP metadata server](https://cloud.google.com/compute/docs/metadata/overview) to resolve the project ID.
+
+For cross-project access, set both fields explicitly:
+
+```yaml
+spec:
+  provider:
+    gcpsm:
+      projectID: "secrets-project-456"
+      auth:
+        workloadIdentity:
+          clusterProjectID: "cluster-project-123"
+          serviceAccountRef:
+            name: demo-sa
+```
+
 #### Explicitly specifying the GKE cluster's name and location
 
-When creating a `SecretStore` or `ClusterSecretStore` that uses WIF, the GKE cluster's project ID, name, and location are automatically determined through the [GCP metadata server](https://cloud.google.com/compute/docs/metadata/overview).
+When creating a `SecretStore` or `ClusterSecretStore` that uses Workload Identity, the GKE cluster's name and location are automatically determined through the [GCP metadata server](https://cloud.google.com/compute/docs/metadata/overview).
 Alternatively, you can explicitly specify some or all of these values.
 
 For a fully specified configuration, you'll need to know the following three values:

+ 41 - 17
providers/v1/gcp/secretmanager/provider.go

@@ -49,6 +49,10 @@ A Mutex was implemented to make sure only one connection can be in place at a ti
 */
 var useMu = sync.Mutex{}
 
+// metadataClientFactory is used to create metadata clients.
+// It can be overridden in tests to inject a fake client.
+var metadataClientFactory = newMetadataClient
+
 // Capabilities returns the provider's capabilities to read/write secrets.
 func (p *Provider) Capabilities() esv1.SecretStoreCapabilities {
 	return esv1.SecretStoreReadWrite
@@ -60,7 +64,7 @@ func (p *Provider) NewClient(ctx context.Context, store esv1.GenericStore, kube
 	if storeSpec == nil || storeSpec.Provider == nil || storeSpec.Provider.GCPSM == nil {
 		return nil, errors.New(errGCPSMStore)
 	}
-	gcpStore := storeSpec.Provider.GCPSM
+	gcpStore := storeSpec.Provider.GCPSM.DeepCopy()
 
 	useMu.Lock()
 
@@ -77,10 +81,18 @@ func (p *Provider) NewClient(ctx context.Context, store esv1.GenericStore, kube
 	}()
 
 	// this project ID is used for authentication (currently only relevant for workload identity)
-	clusterProjectID, err := clusterProjectID(storeSpec)
+	clusterProjectID, err := clusterProjectID(ctx, storeSpec)
 	if err != nil {
 		return nil, err
 	}
+
+	// If ProjectID is not explicitly set in the spec, use the clusterProjectID
+	// This allows the client to function when ProjectID is omitted for Workload Identity,
+	// Workload Identity Federation, or default credentials (not static credentials)
+	if gcpStore.ProjectID == "" {
+		gcpStore.ProjectID = clusterProjectID
+	}
+
 	isClusterKind := store.GetObjectKind().GroupVersionKind().Kind == esv1.ClusterSecretStoreKind
 	// allow SecretStore controller validation to pass
 	// when using referent namespace.
@@ -101,19 +113,9 @@ func (p *Provider) NewClient(ctx context.Context, store esv1.GenericStore, kube
 		return nil, fmt.Errorf(errUnableGetCredentials, err)
 	}
 
-	var clientGCPSM *secretmanager.Client
-	if gcpStore.Location != "" {
-		ep := fmt.Sprintf("secretmanager.%s.rep.googleapis.com:443", gcpStore.Location)
-		regional := option.WithEndpoint(ep)
-		clientGCPSM, err = secretmanager.NewClient(ctx, option.WithTokenSource(ts), regional)
-		if err != nil {
-			return nil, fmt.Errorf(errUnableCreateGCPSMClient, err)
-		}
-	} else {
-		clientGCPSM, err = secretmanager.NewClient(ctx, option.WithTokenSource(ts))
-		if err != nil {
-			return nil, fmt.Errorf(errUnableCreateGCPSMClient, err)
-		}
+	clientGCPSM, err := newSMClient(ctx, ts, gcpStore.Location)
+	if err != nil {
+		return nil, fmt.Errorf(errUnableCreateGCPSMClient, err)
 	}
 	client.smClient = clientGCPSM
 	return client, nil
@@ -132,7 +134,7 @@ func (p *Provider) ValidateStore(store esv1.GenericStore) (admission.Warnings, e
 		return nil, errors.New(errInvalidStoreProv)
 	}
 	g := spc.Provider.GCPSM
-	if p == nil {
+	if g == nil {
 		return nil, errors.New(errInvalidGCPProv)
 	}
 	if g.Auth.SecretRef != nil {
@@ -148,13 +150,35 @@ func (p *Provider) ValidateStore(store esv1.GenericStore) (admission.Warnings, e
 	return nil, nil
 }
 
-func clusterProjectID(spec *esv1.SecretStoreSpec) (string, error) {
+func newSMClient(ctx context.Context, ts oauth2.TokenSource, location string) (*secretmanager.Client, error) {
+	if location != "" {
+		ep := fmt.Sprintf("secretmanager.%s.rep.googleapis.com:443", location)
+		return secretmanager.NewClient(ctx, option.WithTokenSource(ts), option.WithEndpoint(ep))
+	}
+	return secretmanager.NewClient(ctx, option.WithTokenSource(ts))
+}
+
+func clusterProjectID(ctx context.Context, spec *esv1.SecretStoreSpec) (string, error) {
 	if spec.Provider.GCPSM.Auth.WorkloadIdentity != nil && spec.Provider.GCPSM.Auth.WorkloadIdentity.ClusterProjectID != "" {
 		return spec.Provider.GCPSM.Auth.WorkloadIdentity.ClusterProjectID, nil
 	}
 	if spec.Provider.GCPSM.ProjectID != "" {
 		return spec.Provider.GCPSM.ProjectID, nil
 	}
+	// If using static credentials, projectID must be explicitly set
+	// Do NOT fall back to metadata server for static credentials
+	if spec.Provider.GCPSM.Auth.SecretRef != nil {
+		return "", errors.New(errNoProjectID)
+	}
+	// Fall back to GCP metadata server when running in GKE
+	// This allows SecretStore/ClusterSecretStore to omit projectID
+	// when the secrets are in the same project as the GKE cluster
+	metadataClient := metadataClientFactory()
+	projectID, err := metadataClient.ProjectIDWithContext(ctx)
+	if err == nil && projectID != "" {
+		return projectID, nil
+	}
+	log.V(1).Info("failed to get projectID from metadata server", "error", err)
 	return "", errors.New(errNoProjectID)
 }
 

+ 268 - 0
providers/v1/gcp/secretmanager/provider_test.go

@@ -0,0 +1,268 @@
+/*
+Copyright © 2025 ESO Maintainer Team
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package secretmanager
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
+	esmeta "github.com/external-secrets/external-secrets/apis/meta/v1"
+)
+
+func TestClusterProjectIDMetadataFallback(t *testing.T) {
+	originalFactory := metadataClientFactory
+	defer func() { metadataClientFactory = originalFactory }()
+
+	metadataClientFactory = func() MetadataClient {
+		return &fakeMetadataClient{metadata: map[string]string{"project-id": "metadata-project-123"}}
+	}
+
+	store := &esv1.SecretStore{
+		Spec: esv1.SecretStoreSpec{
+			Provider: &esv1.SecretStoreProvider{
+				GCPSM: &esv1.GCPSMProvider{
+					Auth: esv1.GCPSMAuth{
+						WorkloadIdentity: &esv1.GCPWorkloadIdentity{
+							ServiceAccountRef: esmeta.ServiceAccountSelector{Name: "test-sa"},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	projectID, err := clusterProjectID(t.Context(), store.GetSpec())
+	assert.Nil(t, err)
+	assert.Equal(t, "metadata-project-123", projectID)
+
+	// metadata server returns nothing -> error
+	metadataClientFactory = func() MetadataClient {
+		return &fakeMetadataClient{metadata: map[string]string{}}
+	}
+
+	_, err = clusterProjectID(t.Context(), store.GetSpec())
+	assert.NotNil(t, err)
+	assert.Contains(t, err.Error(), "unable to find ProjectID")
+}
+
+func TestClusterProjectIDStaticCredentials(t *testing.T) {
+	originalFactory := metadataClientFactory
+	defer func() { metadataClientFactory = originalFactory }()
+
+	metadataClientFactory = func() MetadataClient {
+		return &fakeMetadataClient{metadata: map[string]string{"project-id": "metadata-project-123"}}
+	}
+
+	t.Run("with explicit projectID", func(t *testing.T) {
+		store := &esv1.SecretStore{
+			Spec: esv1.SecretStoreSpec{
+				Provider: &esv1.SecretStoreProvider{
+					GCPSM: &esv1.GCPSMProvider{
+						ProjectID: "explicit-project-456",
+						Auth: esv1.GCPSMAuth{
+							SecretRef: &esv1.GCPSMAuthSecretRef{
+								SecretAccessKey: esmeta.SecretKeySelector{Name: "my-secret", Key: "credentials"},
+							},
+						},
+					},
+				},
+			},
+		}
+
+		projectID, err := clusterProjectID(t.Context(), store.GetSpec())
+		assert.Nil(t, err)
+		assert.Equal(t, "explicit-project-456", projectID)
+	})
+
+	t.Run("without projectID should fail, not fall back to metadata", func(t *testing.T) {
+		store := &esv1.SecretStore{
+			Spec: esv1.SecretStoreSpec{
+				Provider: &esv1.SecretStoreProvider{
+					GCPSM: &esv1.GCPSMProvider{
+						Auth: esv1.GCPSMAuth{
+							SecretRef: &esv1.GCPSMAuthSecretRef{
+								SecretAccessKey: esmeta.SecretKeySelector{Name: "my-secret", Key: "credentials"},
+							},
+						},
+					},
+				},
+			},
+		}
+
+		_, err := clusterProjectID(t.Context(), store.GetSpec())
+		assert.NotNil(t, err)
+		assert.Contains(t, err.Error(), "unable to find ProjectID")
+	})
+}
+
+func TestClusterProjectIDWorkloadIdentityFederation(t *testing.T) {
+	originalFactory := metadataClientFactory
+	defer func() { metadataClientFactory = originalFactory }()
+
+	t.Run("metadata fallback", func(t *testing.T) {
+		metadataClientFactory = func() MetadataClient {
+			return &fakeMetadataClient{metadata: map[string]string{"project-id": "metadata-project-999"}}
+		}
+
+		store := &esv1.SecretStore{
+			Spec: esv1.SecretStoreSpec{
+				Provider: &esv1.SecretStoreProvider{
+					GCPSM: &esv1.GCPSMProvider{
+						Auth: esv1.GCPSMAuth{
+							WorkloadIdentityFederation: &esv1.GCPWorkloadIdentityFederation{
+								ServiceAccountRef: &esmeta.ServiceAccountSelector{Name: "test-sa"},
+								Audience:          "//iam.googleapis.com/projects/123/locations/global/workloadIdentityPools/pool/providers/provider",
+							},
+						},
+					},
+				},
+			},
+		}
+
+		projectID, err := clusterProjectID(t.Context(), store.GetSpec())
+		assert.Nil(t, err)
+		assert.Equal(t, "metadata-project-999", projectID)
+	})
+
+	t.Run("explicit projectID takes precedence", func(t *testing.T) {
+		metadataClientFactory = func() MetadataClient {
+			return &fakeMetadataClient{metadata: map[string]string{"project-id": "metadata-999"}}
+		}
+
+		store := &esv1.SecretStore{
+			Spec: esv1.SecretStoreSpec{
+				Provider: &esv1.SecretStoreProvider{
+					GCPSM: &esv1.GCPSMProvider{
+						ProjectID: "explicit-secrets-456",
+						Auth: esv1.GCPSMAuth{
+							WorkloadIdentityFederation: &esv1.GCPWorkloadIdentityFederation{
+								ServiceAccountRef: &esmeta.ServiceAccountSelector{Name: "test-sa"},
+								Audience:          "//iam.googleapis.com/projects/123/locations/global/workloadIdentityPools/pool/providers/provider",
+							},
+						},
+					},
+				},
+			},
+		}
+
+		projectID, err := clusterProjectID(t.Context(), store.GetSpec())
+		assert.Nil(t, err)
+		assert.Equal(t, "explicit-secrets-456", projectID)
+	})
+
+	t.Run("no projectID and no metadata should fail", func(t *testing.T) {
+		metadataClientFactory = func() MetadataClient {
+			return &fakeMetadataClient{metadata: map[string]string{}}
+		}
+
+		store := &esv1.SecretStore{
+			Spec: esv1.SecretStoreSpec{
+				Provider: &esv1.SecretStoreProvider{
+					GCPSM: &esv1.GCPSMProvider{
+						Auth: esv1.GCPSMAuth{
+							WorkloadIdentityFederation: &esv1.GCPWorkloadIdentityFederation{
+								ServiceAccountRef: &esmeta.ServiceAccountSelector{Name: "test-sa"},
+								Audience:          "//iam.googleapis.com/projects/123/locations/global/workloadIdentityPools/pool/providers/provider",
+							},
+						},
+					},
+				},
+			},
+		}
+
+		_, err := clusterProjectID(t.Context(), store.GetSpec())
+		assert.NotNil(t, err)
+		assert.Contains(t, err.Error(), "unable to find ProjectID")
+	})
+}
+
+func TestClusterProjectIDDefaultCredentials(t *testing.T) {
+	originalFactory := metadataClientFactory
+	defer func() { metadataClientFactory = originalFactory }()
+
+	t.Run("metadata fallback", func(t *testing.T) {
+		metadataClientFactory = func() MetadataClient {
+			return &fakeMetadataClient{metadata: map[string]string{"project-id": "metadata-project-999"}}
+		}
+
+		store := &esv1.SecretStore{
+			Spec: esv1.SecretStoreSpec{
+				Provider: &esv1.SecretStoreProvider{
+					GCPSM: &esv1.GCPSMProvider{},
+				},
+			},
+		}
+
+		projectID, err := clusterProjectID(t.Context(), store.GetSpec())
+		assert.Nil(t, err)
+		assert.Equal(t, "metadata-project-999", projectID)
+	})
+
+	t.Run("explicit projectID takes precedence", func(t *testing.T) {
+		metadataClientFactory = func() MetadataClient {
+			return &fakeMetadataClient{metadata: map[string]string{"project-id": "metadata-999"}}
+		}
+
+		store := &esv1.SecretStore{
+			Spec: esv1.SecretStoreSpec{
+				Provider: &esv1.SecretStoreProvider{
+					GCPSM: &esv1.GCPSMProvider{
+						ProjectID: "explicit-secrets-111",
+					},
+				},
+			},
+		}
+
+		projectID, err := clusterProjectID(t.Context(), store.GetSpec())
+		assert.Nil(t, err)
+		assert.Equal(t, "explicit-secrets-111", projectID)
+	})
+
+	t.Run("no projectID and no metadata should fail", func(t *testing.T) {
+		metadataClientFactory = func() MetadataClient {
+			return &fakeMetadataClient{metadata: map[string]string{}}
+		}
+
+		store := &esv1.SecretStore{
+			Spec: esv1.SecretStoreSpec{
+				Provider: &esv1.SecretStoreProvider{
+					GCPSM: &esv1.GCPSMProvider{},
+				},
+			},
+		}
+
+		_, err := clusterProjectID(t.Context(), store.GetSpec())
+		assert.NotNil(t, err)
+		assert.Contains(t, err.Error(), "unable to find ProjectID")
+	})
+}
+
+func TestValidateStoreNilGCPSM(t *testing.T) {
+	p := &Provider{}
+
+	_, err := p.ValidateStore(&esv1.SecretStore{
+		Spec: esv1.SecretStoreSpec{
+			Provider: &esv1.SecretStoreProvider{
+				GCPSM: nil,
+			},
+		},
+	})
+	assert.NotNil(t, err)
+	assert.Contains(t, err.Error(), errInvalidGCPProv)
+}

+ 2 - 2
providers/v1/gcp/secretmanager/workload_identity_test.go

@@ -193,10 +193,10 @@ func TestWorkloadIdentity(t *testing.T) {
 }
 
 func TestClusterProjectID(t *testing.T) {
-	clusterID, err := clusterProjectID(defaultStore().GetSpec())
+	clusterID, err := clusterProjectID(t.Context(), defaultStore().GetSpec())
 	assert.Nil(t, err)
 	assert.Equal(t, clusterID, "1234")
-	externalClusterID, err := clusterProjectID(defaultExternalStore().GetSpec())
+	externalClusterID, err := clusterProjectID(t.Context(), defaultExternalStore().GetSpec())
 	assert.Nil(t, err)
 	assert.Equal(t, externalClusterID, "5678")
 }