Browse Source

feat(gcp): get latest enabled secret (#5131)

* Get latest enabled secret

Signed-off-by: Itai Spiegel <itai.spiegel@gmail.com>

* Add a feature flag to control the new behavior

Signed-off-by: Itai Spiegel <itai.spiegel@gmail.com>

* Simplify conditions

Signed-off-by: Itai Spiegel <itai.spiegel@gmail.com>

* Replace boolean field with string field

Signed-off-by: Itai Spiegel <itai.spiegel@gmail.com>

* Add test for LatestOrFail policy

Signed-off-by: Itai Spiegel <itai.spiegel@gmail.com>

* Fix golangci

Signed-off-by: Itai Spiegel <itai.spiegel@gmail.com>

* Fix CI

Signed-off-by: Itai Spiegel <itai.spiegel@gmail.com>

* Fix metrics.ObserveAPICall

Signed-off-by: Itai Spiegel <itai.spiegel@gmail.com>

---------

Signed-off-by: Itai Spiegel <itai.spiegel@gmail.com>
Itai Spiegel 6 months ago
parent
commit
bd62d5b801

+ 19 - 0
apis/externalsecrets/v1/secretstore_gcpsm_types.go

@@ -20,6 +20,16 @@ import (
 	esmeta "github.com/external-secrets/external-secrets/apis/meta/v1"
 )
 
+type SecretVersionSelectionPolicy string
+
+const (
+	// SecretVersionSelectionPolicyLatestOrFail means the provider always uses "latest", or fails if that version is disabled/destroyed.
+	SecretVersionSelectionPolicyLatestOrFail SecretVersionSelectionPolicy = "LatestOrFail"
+
+	// SecretVersionSelectionPolicyLatestOrFetch behaves like SecretVersionSelectionPolicyLatestOrFail but falls back to fetching the latest version if the version is DESTROYED or DISABLED.
+	SecretVersionSelectionPolicyLatestOrFetch SecretVersionSelectionPolicy = "LatestOrFetch"
+)
+
 type GCPSMAuth struct {
 	// +optional
 	SecretRef *GCPSMAuthSecretRef `json:"secretRef,omitempty"`
@@ -63,6 +73,15 @@ type GCPSMProvider struct {
 
 	// Location optionally defines a location for a secret
 	Location string `json:"location,omitempty"`
+
+	// SecretVersionSelectionPolicy specifies how the provider selects a secret version
+	// when "latest" is disabled or destroyed.
+	// Possible values are:
+	// - LatestOrFail: the provider always uses "latest", or fails if that version is disabled/destroyed.
+	// - LatestOrFetch: the provider falls back to fetching the latest version if the version is DESTROYED or DISABLED
+	// +optional
+	// +kubebuilder:default=LatestOrFail
+	SecretVersionSelectionPolicy SecretVersionSelectionPolicy `json:"secretVersionSelectionPolicy,omitempty"`
 }
 
 // GCPWorkloadIdentityFederation holds the configurations required for generating federated access tokens.

+ 9 - 0
config/crds/bases/external-secrets.io_clustersecretstores.yaml

@@ -2072,6 +2072,15 @@ spec:
                       projectID:
                         description: ProjectID project where secret is located
                         type: string
+                      secretVersionSelectionPolicy:
+                        default: LatestOrFail
+                        description: |-
+                          SecretVersionSelectionPolicy specifies how the provider selects a secret version
+                          when "latest" is disabled or destroyed.
+                          Possible values are:
+                          - LatestOrFail: the provider always uses "latest", or fails if that version is disabled/destroyed.
+                          - LatestOrFetch: the provider falls back to fetching the latest version if the version is DESTROYED or DISABLED
+                        type: string
                     type: object
                   github:
                     description: |-

+ 9 - 0
config/crds/bases/external-secrets.io_secretstores.yaml

@@ -2072,6 +2072,15 @@ spec:
                       projectID:
                         description: ProjectID project where secret is located
                         type: string
+                      secretVersionSelectionPolicy:
+                        default: LatestOrFail
+                        description: |-
+                          SecretVersionSelectionPolicy specifies how the provider selects a secret version
+                          when "latest" is disabled or destroyed.
+                          Possible values are:
+                          - LatestOrFail: the provider always uses "latest", or fails if that version is disabled/destroyed.
+                          - LatestOrFetch: the provider falls back to fetching the latest version if the version is DESTROYED or DISABLED
+                        type: string
                     type: object
                   github:
                     description: |-

+ 18 - 0
deploy/crds/bundle.yaml

@@ -3962,6 +3962,15 @@ spec:
                         projectID:
                           description: ProjectID project where secret is located
                           type: string
+                        secretVersionSelectionPolicy:
+                          default: LatestOrFail
+                          description: |-
+                            SecretVersionSelectionPolicy specifies how the provider selects a secret version
+                            when "latest" is disabled or destroyed.
+                            Possible values are:
+                            - LatestOrFail: the provider always uses "latest", or fails if that version is disabled/destroyed.
+                            - LatestOrFetch: the provider falls back to fetching the latest version if the version is DESTROYED or DISABLED
+                          type: string
                       type: object
                     github:
                       description: |-
@@ -15097,6 +15106,15 @@ spec:
                         projectID:
                           description: ProjectID project where secret is located
                           type: string
+                        secretVersionSelectionPolicy:
+                          default: LatestOrFail
+                          description: |-
+                            SecretVersionSelectionPolicy specifies how the provider selects a secret version
+                            when "latest" is disabled or destroyed.
+                            Possible values are:
+                            - LatestOrFail: the provider always uses "latest", or fails if that version is disabled/destroyed.
+                            - LatestOrFetch: the provider falls back to fetching the latest version if the version is DESTROYED or DISABLED
+                          type: string
                       type: object
                     github:
                       description: |-

+ 41 - 0
docs/api/spec.md

@@ -5267,6 +5267,24 @@ string
 <p>Location optionally defines a location for a secret</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>secretVersionSelectionPolicy</code></br>
+<em>
+<a href="#external-secrets.io/v1.SecretVersionSelectionPolicy">
+SecretVersionSelectionPolicy
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>SecretVersionSelectionPolicy specifies how the provider selects a secret version
+when &ldquo;latest&rdquo; is disabled or destroyed.
+Possible values are:
+- LatestOrFail: the provider always uses &ldquo;latest&rdquo;, or fails if that version is disabled/destroyed.
+- LatestOrFetch: the provider falls back to fetching the latest version if the version is DESTROYED or DISABLED</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="external-secrets.io/v1.GCPWorkloadIdentity">GCPWorkloadIdentity
@@ -9172,6 +9190,29 @@ Kubernetes meta/v1.Time
 </tr>
 </tbody>
 </table>
+<h3 id="external-secrets.io/v1.SecretVersionSelectionPolicy">SecretVersionSelectionPolicy
+(<code>string</code> alias)</p></h3>
+<p>
+(<em>Appears on:</em>
+<a href="#external-secrets.io/v1.GCPSMProvider">GCPSMProvider</a>)
+</p>
+<p>
+</p>
+<table>
+<thead>
+<tr>
+<th>Value</th>
+<th>Description</th>
+</tr>
+</thead>
+<tbody><tr><td><p>&#34;LatestOrFail&#34;</p></td>
+<td><p>SecretVersionSelectionPolicyLatestOrFail means the provider always uses &ldquo;latest&rdquo;, or fails if that version is disabled/destroyed.</p>
+</td>
+</tr><tr><td><p>&#34;LatestOrFetch&#34;</p></td>
+<td><p>SecretVersionSelectionPolicyLatestOrFetch behaves like SecretVersionSelectionPolicyLatestOrFail but falls back to fetching the latest version if the version is DESTROYED or DISABLED.</p>
+</td>
+</tr></tbody>
+</table>
 <h3 id="external-secrets.io/v1.SecretsClient">SecretsClient
 </h3>
 <p>

+ 32 - 0
docs/provider/google-secrets-manager.md

@@ -379,3 +379,35 @@ spec:
       projectID: my-project
       location: us-east1 # uses regional secrets on us-east1
 ```
+
+## Secret Version Management
+
+### Secret Version Selection Policy
+
+The Google Secret Manager provider includes a `secretVersionSelectionPolicy` field that controls how the provider handles secret version selection when the default "latest" version is unavailable.
+
+By default, when you request a secret without specifying a version, the provider attempts to fetch the "latest" version. The `secretVersionSelectionPolicy` determines what happens if that version is in a DESTROYED or DISABLED state.
+
+#### Available Policies
+
+- **`LatestOrFail`** (default): The provider always uses "latest", or fails if that version is disabled/destroyed.
+- **`LatestOrFetch`**: The provider falls back to fetching the latest enabled version if the "latest" version is DESTROYED or DISABLED.
+
+#### Configuration Example
+
+```yaml
+apiVersion: external-secrets.io/v1beta1
+kind: SecretStore
+metadata:
+  name: gcp-secret-store
+spec:
+  provider:
+    gcpsm:
+      projectID: my-project
+      location: us-east1
+      secretVersionSelectionPolicy: LatestOrFetch  # or LatestOrFail (default)
+```
+
+**Note**: When using `secretVersionSelectionPolicy: LatestOrFetch`, the service account requires additional permissions to list secret versions. You'll need to grant the `roles/secretmanager.viewer` role (which includes `secretmanager.versions.list`) or the specific `secretmanager.versions.list` permission in addition to the standard `secretmanager.secretAccessor` role.
+
+```

+ 2 - 0
docs/snippets/full-secret-store.yaml

@@ -128,6 +128,8 @@ spec:
             name: gcpsm-secret
             key: secret-access-credentials
       projectID: myproject
+      location: us-east1
+      secretVersionSelectionPolicy: LatestOrFetch
     # (TODO): add more provider examples here
 
 status:

+ 43 - 1
pkg/provider/gcp/secretmanager/client.go

@@ -25,6 +25,7 @@ import (
 	"slices"
 	"strconv"
 	"strings"
+	"time"
 
 	secretmanager "cloud.google.com/go/secretmanager/apiv1"
 	"cloud.google.com/go/secretmanager/apiv1/secretmanagerpb"
@@ -101,6 +102,7 @@ type GoogleSecretManagerClient interface {
 	Close() error
 	GetSecret(ctx context.Context, req *secretmanagerpb.GetSecretRequest, opts ...gax.CallOption) (*secretmanagerpb.Secret, error)
 	UpdateSecret(context.Context, *secretmanagerpb.UpdateSecretRequest, ...gax.CallOption) (*secretmanagerpb.Secret, error)
+	ListSecretVersions(ctx context.Context, req *secretmanagerpb.ListSecretVersionsRequest, opts ...gax.CallOption) *secretmanager.SecretVersionIterator
 }
 
 var log = ctrl.Log.WithName("provider").WithName("gcp").WithName("secretsmanager")
@@ -500,8 +502,19 @@ func (c *Client) GetSecret(ctx context.Context, ref esv1.ExternalSecretDataRemot
 	}
 	result, err := c.smClient.AccessSecretVersion(ctx, req)
 	metrics.ObserveAPICall(constants.ProviderGCPSM, constants.CallGCPSMAccessSecretVersion, err)
-	err = parseError(err)
+	if err != nil && c.store.SecretVersionSelectionPolicy == esv1.SecretVersionSelectionPolicyLatestOrFetch &&
+		ref.Version == "" && isErrSecretDestroyedOrDisabled(err) {
+		// if the secret is destroyed or disabled, and we are configured to get the latest enabled secret,
+		// we need to get the latest enabled secret
+		// Extract the secret name from the version name for ListSecretVersions
+		secretName := fmt.Sprintf(globalSecretPath, c.store.ProjectID, ref.Key)
+		if c.store.Location != "" {
+			secretName = fmt.Sprintf(regionalSecretPath, c.store.ProjectID, c.store.Location, ref.Key)
+		}
+		result, err = getLatestEnabledVersion(ctx, c.smClient, secretName)
+	}
 	if err != nil {
+		err = parseError(err)
 		return nil, fmt.Errorf(errClientGetSecretAccess, err)
 	}
 
@@ -677,3 +690,32 @@ func getParentName(projectID, location string) string {
 	}
 	return fmt.Sprintf(globalSecretParentPath, projectID)
 }
+
+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"))
+}
+
+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{}
+	for {
+		version, err := iter.Next()
+		if errors.Is(err, iterator.Done) {
+			break
+		}
+		if version.CreateTime.AsTime().After(latestCreateTime) {
+			latestCreateTime = version.CreateTime.AsTime()
+			latestVersion = version
+		}
+	}
+	req := &secretmanagerpb.AccessSecretVersionRequest{
+		Name: fmt.Sprintf("%s/versions/%s", name, latestVersion.Name),
+	}
+	return client.AccessSecretVersion(ctx, req)
+}

+ 23 - 11
pkg/provider/gcp/secretmanager/client_test.go

@@ -61,20 +61,22 @@ type secretManagerTestCase struct {
 	expectError    string
 	expectedSecret string
 	// for testing SecretMap
-	expectedData map[string][]byte
+	expectedData              map[string][]byte
+	latestEnabledSecretPolicy esv1.SecretVersionSelectionPolicy
 }
 
 func makeValidSecretManagerTestCase() *secretManagerTestCase {
 	smtc := secretManagerTestCase{
-		mockClient:     &fakesm.MockSMClient{},
-		apiInput:       makeValidAPIInput(),
-		ref:            makeValidRef(),
-		apiOutput:      makeValidAPIOutput(),
-		projectID:      "default",
-		apiErr:         nil,
-		expectError:    "",
-		expectedSecret: "",
-		expectedData:   map[string][]byte{},
+		mockClient:                &fakesm.MockSMClient{},
+		apiInput:                  makeValidAPIInput(),
+		ref:                       makeValidRef(),
+		apiOutput:                 makeValidAPIOutput(),
+		projectID:                 "default",
+		apiErr:                    nil,
+		expectError:               "",
+		expectedSecret:            "",
+		expectedData:              map[string][]byte{},
+		latestEnabledSecretPolicy: esv1.SecretVersionSelectionPolicyLatestOrFail,
 	}
 	smtc.mockClient.NilClose()
 	smtc.mockClient.WithValue(context.Background(), smtc.apiInput, smtc.apiOutput, smtc.apiErr)
@@ -132,6 +134,15 @@ func TestSecretManagerGetSecret(t *testing.T) {
 		smtc.apiOutput.Payload.Data = []byte("testtesttest")
 		smtc.expectedSecret = "testtesttest"
 	}
+	latestSecretDestroyed := func(smtc *secretManagerTestCase) {
+		// Test the LatestOrFail policy (default behavior)
+		// Ideally we would test the LatestOrFetch policy, but we don't have a mock for the ListSecretVersions call
+		// so we can't test that until it's implemented.
+		smtc.apiErr = status.Error(codes.FailedPrecondition, "DESTROYED state")
+		smtc.latestEnabledSecretPolicy = esv1.SecretVersionSelectionPolicyLatestOrFail
+		smtc.expectedSecret = ""
+		smtc.expectError = smtc.apiErr.Error()
+	}
 	secretNotFound := func(smtc *secretManagerTestCase) {
 		fErr := status.Error(codes.NotFound, "failed")
 		notFoundError, _ := apierror.FromError(fErr)
@@ -190,6 +201,7 @@ func TestSecretManagerGetSecret(t *testing.T) {
 	successCases := []*secretManagerTestCase{
 		makeValidSecretManagerTestCase(),
 		makeValidSecretManagerTestCaseCustom(setSecretString),
+		makeValidSecretManagerTestCaseCustom(latestSecretDestroyed),
 		makeValidSecretManagerTestCaseCustom(secretNotFound),
 		makeValidSecretManagerTestCaseCustom(setCustomVersion),
 		makeValidSecretManagerTestCaseCustom(setAPIErr),
@@ -200,7 +212,7 @@ func TestSecretManagerGetSecret(t *testing.T) {
 
 	sm := Client{}
 	for k, v := range successCases {
-		sm.store = &esv1.GCPSMProvider{ProjectID: v.projectID}
+		sm.store = &esv1.GCPSMProvider{ProjectID: v.projectID, SecretVersionSelectionPolicy: v.latestEnabledSecretPolicy}
 		sm.smClient = v.mockClient
 		out, err := sm.GetSecret(context.Background(), *v.ref)
 		if !ErrorContains(err, v.expectError) {

+ 5 - 0
pkg/provider/gcp/secretmanager/fake/fake.go

@@ -40,6 +40,7 @@ type MockSMClient struct {
 	closeFn                 func() error
 	GetSecretFn             func(ctx context.Context, req *secretmanagerpb.GetSecretRequest, opts ...gax.CallOption) (*secretmanagerpb.Secret, error)
 	DeleteSecretFn          func(ctx context.Context, req *secretmanagerpb.DeleteSecretRequest, opts ...gax.CallOption) error
+	ListSecretVersionsFn    func(ctx context.Context, req *secretmanagerpb.ListSecretVersionsRequest, opts ...gax.CallOption) *secretmanager.SecretVersionIterator
 }
 
 func (mc *MockSMClient) Cleanup() {
@@ -202,6 +203,10 @@ func (mc *MockSMClient) NewUpdateSecretFn(mock SecretMockReturn) {
 	}
 }
 
+func (mc *MockSMClient) ListSecretVersions(ctx context.Context, req *secretmanagerpb.ListSecretVersionsRequest, _ ...gax.CallOption) *secretmanager.SecretVersionIterator {
+	return mc.ListSecretVersionsFn(ctx, req)
+}
+
 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) {