Browse Source

BREAKING: Standardize GCP Secret Manager PushSecret metadata format and add CMEK support (#4210)

* feat: add support for customer-managed encryption (CMEK) in GCP Secret Manager

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

* refactor: enhance Docker build command and update API documentation

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

* refactor: run make reviewable

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

* refactor: remove CMEKKeyName field from GCPSMProvider and related CRDs

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

* refactor: replace hardcoded region strings with constants in GCP Secret Manager tests

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

* refactor: update GCP Secret Manager tests to use consistent region naming and enhance metadata handling

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

* refactor: enhance PushSecret metadata handling and update related tests

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

* revert: back to previous Docker build command in Makefile

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

* revert: back to make ProjectID and Location fields optional

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

* Update pkg/provider/gcp/secretmanager/client.go

Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

* fix: update KmsKeyName assignment in PushSecret to use CMEKKeyName from metadata

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

* docs: add migration guide for PushSecret metadata format from v0.11.x to v0.12.0

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

* chore: update golang.org/x/crypto dependency to v0.31.0 in go.mod and go.sum files

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>

---------

Signed-off-by: Jan Lauber <jan.lauber@protonmail.ch>
Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Jan Lauber 1 year ago
parent
commit
2e528ffa85

+ 108 - 1
docs/provider/google-secrets-manager.md

@@ -146,9 +146,116 @@ spec:
       metadata:
         mergePolicy: Merge
         labels:
-          anotherLabel: anotherValue 
+          anotherLabel: anotherValue
       match:
         secretKey: bestpokemon
         remoteRef:
           remoteKey: best-pokemon
 ```
+
+### Secret Replication and Encryption Configuration
+
+#### Location and Replication
+
+By default, secrets are automatically replicated across multiple regions. You can specify a single location for your secrets by setting the `location` field:
+
+```yaml
+apiVersion: external-secrets.io/v1beta1
+kind: SecretStore
+metadata:
+  name: gcp-secret-store
+spec:
+  provider:
+    gcpsm:
+      projectID: my-project
+      location: us-east1  # Specify a single location
+```
+
+#### Customer-Managed Encryption Keys (CMEK)
+
+You can use your own encryption keys to encrypt secrets at rest. To use Customer-Managed Encryption Keys (CMEK), you need to:
+
+1. Create a Cloud KMS key
+2. Grant the service account the `roles/cloudkms.cryptoKeyEncrypterDecrypter` role on the key
+3. Specify the key in the PushSecret metadata
+
+```yaml
+apiVersion: external-secrets.io/v1alpha1
+kind: PushSecret
+metadata:
+  name: pushsecret-example
+spec:
+  # ... other fields ...
+  data:
+    - match:
+        secretKey: mykey
+        remoteRef:
+          remoteKey: my-secret
+      metadata:
+        apiVersion: kubernetes.external-secrets.io/v1alpha1
+        kind: PushSecretMetadata
+        spec:
+          cmekKeyName: "projects/my-project/locations/us-east1/keyRings/my-keyring/cryptoKeys/my-key"
+```
+
+Note: When using CMEK, you must specify a location in the SecretStore as customer-managed encryption keys are region-specific.
+
+```yaml
+apiVersion: external-secrets.io/v1beta1
+kind: SecretStore
+metadata:
+  name: gcp-secret-store
+spec:
+  provider:
+    gcpsm:
+      projectID: my-project
+      location: us-east1  # Required when using CMEK
+```
+
+### Migration Guide: PushSecret Metadata Format (v0.11.x to v0.12.0)
+
+In version 0.12.0, the metadata format for PushSecrets has been standardized to use a structured format. If you're upgrading from v0.11.x, you'll need to update your PushSecret specifications.
+
+#### Old Format (v0.11.x)
+```yaml
+apiVersion: external-secrets.io/v1alpha1
+kind: PushSecret
+spec:
+  data:
+    - match:
+        secretKey: mykey
+        remoteRef:
+          remoteKey: my-secret
+      metadata:
+        annotations:
+          key1: "value1"
+        labels:
+          key2: "value2"
+        topics:
+          - "topic1"
+          - "topic2"
+```
+
+#### New Format (v0.12.0+)
+```yaml
+apiVersion: external-secrets.io/v1alpha1
+kind: PushSecret
+spec:
+  data:
+    - match:
+        secretKey: mykey
+        remoteRef:
+          remoteKey: my-secret
+      metadata:
+        apiVersion: kubernetes.external-secrets.io/v1alpha1
+        kind: PushSecretMetadata
+        spec:
+          annotations:
+            key1: "value1"
+          labels:
+            key2: "value2"
+          topics:
+            - "topic1"
+            - "topic2"
+          cmekKeyName: "projects/my-project/locations/us-east1/keyRings/my-keyring/cryptoKeys/my-key"  # Optional: for CMEK
+```

+ 1 - 1
e2e/go.mod

@@ -191,7 +191,7 @@ require (
 	go.opentelemetry.io/otel v1.32.0 // indirect
 	go.opentelemetry.io/otel/metric v1.32.0 // indirect
 	go.opentelemetry.io/otel/trace v1.32.0 // indirect
-	golang.org/x/crypto v0.30.0 // indirect
+	golang.org/x/crypto v0.31.0 // indirect
 	golang.org/x/exp v0.0.0-20241204233417-43b7b7cde48d // indirect
 	golang.org/x/net v0.32.0 // indirect
 	golang.org/x/sync v0.10.0 // indirect

+ 2 - 2
e2e/go.sum

@@ -555,8 +555,8 @@ golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliY
 golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4=
 golang.org/x/crypto v0.16.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4=
 golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4=
-golang.org/x/crypto v0.30.0 h1:RwoQn3GkWiMkzlX562cLB7OxWvjH1L8xutO2WoJcRoY=
-golang.org/x/crypto v0.30.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
+golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U=
+golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8=

+ 1 - 1
go.mod

@@ -43,7 +43,7 @@ require (
 	github.com/yandex-cloud/go-sdk v0.0.0-20241206142255-6c3760d17eea
 	github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78
 	go.uber.org/zap v1.27.0
-	golang.org/x/crypto v0.30.0
+	golang.org/x/crypto v0.31.0
 	golang.org/x/oauth2 v0.24.0
 	google.golang.org/api v0.210.0
 	google.golang.org/genproto v0.0.0-20241206012308-a4fef0638583

+ 2 - 2
go.sum

@@ -745,8 +745,8 @@ golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq
 golang.org/x/crypto v0.18.0/go.mod h1:R0j02AL6hcrfOiy9T4ZYp/rcWeMxM3L6QYxlOuEG1mg=
 golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU=
 golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs=
-golang.org/x/crypto v0.30.0 h1:RwoQn3GkWiMkzlX562cLB7OxWvjH1L8xutO2WoJcRoY=
-golang.org/x/crypto v0.30.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
+golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U=
+golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8=

+ 19 - 3
pkg/provider/gcp/secretmanager/client.go

@@ -43,6 +43,7 @@ import (
 	"github.com/external-secrets/external-secrets/pkg/metrics"
 	"github.com/external-secrets/external-secrets/pkg/provider/util/locks"
 	"github.com/external-secrets/external-secrets/pkg/utils"
+	"github.com/external-secrets/external-secrets/pkg/utils/metadata"
 )
 
 const (
@@ -183,13 +184,28 @@ func (c *Client) PushSecret(ctx context.Context, secret *corev1.Secret, pushSecr
 		}
 
 		if c.store.Location != "" {
+			replica := &secretmanagerpb.Replication_UserManaged_Replica{
+				Location: c.store.Location,
+			}
+
+			if pushSecretData.GetMetadata() != nil {
+				var err error
+				meta, err := metadata.ParseMetadataParameters[PushSecretMetadataSpec](pushSecretData.GetMetadata())
+				if err != nil {
+					return fmt.Errorf("failed to parse PushSecret metadata: %w", err)
+				}
+				if meta != nil && meta.Spec.CMEKKeyName != "" {
+					replica.CustomerManagedEncryption = &secretmanagerpb.CustomerManagedEncryption{
+						KmsKeyName: meta.Spec.CMEKKeyName,
+					}
+				}
+			}
+
 			replication = &secretmanagerpb.Replication{
 				Replication: &secretmanagerpb.Replication_UserManaged_{
 					UserManaged: &secretmanagerpb.Replication_UserManaged{
 						Replicas: []*secretmanagerpb.Replication_UserManaged_Replica{
-							{
-								Location: c.store.Location,
-							},
+							replica,
 						},
 					},
 				},

+ 28 - 9
pkg/provider/gcp/secretmanager/client_test.go

@@ -40,6 +40,12 @@ import (
 	testingfake "github.com/external-secrets/external-secrets/pkg/provider/testing/fake"
 )
 
+const (
+	errCallNotFoundAtIndex0   = "index 0 for call not found in the list of calls"
+	usEast1                   = "us-east1"
+	errInvalidReplicationType = "req.Secret.Replication.Replication was not of type *secretmanagerpb.Replication_UserManaged_ but: %T"
+)
+
 type secretManagerTestCase struct {
 	mockClient     *fakesm.MockSMClient
 	apiInput       *secretmanagerpb.AccessSecretVersionRequest
@@ -636,7 +642,14 @@ func TestPushSecret(t *testing.T) {
 				store: &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
 				mock:  smtc.mockClient,
 				Metadata: &apiextensionsv1.JSON{
-					Raw: []byte(`{"annotations":{"annotation-key1":"annotation-value1"},"labels":{"label-key1":"label-value1"}}`),
+					Raw: []byte(`{
+						"apiVersion": "kubernetes.external-secrets.io/v1alpha1",
+						"kind": "PushSecretMetadata",
+						"spec": {
+							"annotations": {"annotation-key1":"annotation-value1"},
+							"labels": {"label-key1":"label-value1"}
+						}
+					}`),
 				},
 				GetSecretMockReturn: fakesm.SecretMockReturn{Secret: &secret, Err: nil},
 				UpdateSecretReturn: fakesm.SecretMockReturn{Secret: &secretmanagerpb.Secret{
@@ -663,7 +676,7 @@ func TestPushSecret(t *testing.T) {
 		{
 			desc: "successfully pushes a secret with defined region",
 			args: args{
-				store:               &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID, Location: "us-east-1"},
+				store:               &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID, Location: usEast1},
 				mock:                smtc.mockClient,
 				GetSecretMockReturn: fakesm.SecretMockReturn{Secret: nil, Err: notFoundError},
 				CreateSecretMockReturn: fakesm.SecretMockReturn{Secret: &secretmanagerpb.Secret{
@@ -673,7 +686,7 @@ func TestPushSecret(t *testing.T) {
 							UserManaged: &secretmanagerpb.Replication_UserManaged{
 								Replicas: []*secretmanagerpb.Replication_UserManaged_Replica{
 									{
-										Location: "us-east-1",
+										Location: usEast1,
 									},
 								},
 							},
@@ -694,19 +707,19 @@ func TestPushSecret(t *testing.T) {
 				req: func(m *fakesm.MockSMClient) error {
 					req, ok := m.CreateSecretCalledWithN[0]
 					if !ok {
-						return errors.New("index 0 for call not found in the list of calls")
+						return errors.New(errCallNotFoundAtIndex0)
 					}
 
 					user, ok := req.Secret.Replication.Replication.(*secretmanagerpb.Replication_UserManaged_)
 					if !ok {
-						return fmt.Errorf("req.Secret.Replication.Replication was not of type *secretmanagerpb.Replication_UserManaged_ but: %T", req.Secret.Replication.Replication)
+						return fmt.Errorf(errInvalidReplicationType, req.Secret.Replication.Replication)
 					}
 
 					if len(user.UserManaged.Replicas) < 1 {
 						return errors.New("req.Secret.Replication.Replication.Replicas was not empty")
 					}
 
-					if user.UserManaged.Replicas[0].Location != "us-east-1" {
+					if user.UserManaged.Replicas[0].Location != usEast1 {
 						return fmt.Errorf("req.Secret.Replication.Replicas[0].Location was not equal to us-east-1 but was %s", user.UserManaged.Replicas[0].Location)
 					}
 
@@ -718,7 +731,13 @@ func TestPushSecret(t *testing.T) {
 			desc: "SetSecret successfully pushes a secret with topics",
 			args: args{
 				Metadata: &apiextensionsv1.JSON{
-					Raw: []byte(`{"topics":["topic1", "topic2"]}`),
+					Raw: []byte(`{
+						"apiVersion": "kubernetes.external-secrets.io/v1alpha1",
+						"kind": "PushSecretMetadata",
+						"spec": {
+							"topics": ["topic1", "topic2"]
+						}
+					}`),
 				},
 				store:                         &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
 				mock:                          &fakesm.MockSMClient{}, // the mock should NOT be shared between test cases
@@ -731,7 +750,7 @@ func TestPushSecret(t *testing.T) {
 				req: func(m *fakesm.MockSMClient) error {
 					scrt, ok := m.CreateSecretCalledWithN[0]
 					if !ok {
-						return errors.New("index 0 for call not found in the list of calls")
+						return errors.New(errCallNotFoundAtIndex0)
 					}
 
 					if scrt.Secret == nil {
@@ -1182,7 +1201,7 @@ func TestPushSecret_Property(t *testing.T) {
 				}
 
 				if !strings.Contains(err.Error(), tc.expectedErr) {
-					t.Fatalf("PushSecret returns unexpected error: %q is supposed to contain %q", err, tc.expectedErr)
+					t.Fatalf("PushSecret returns unexpected error: %q should have contained %s", err, tc.expectedErr)
 				}
 
 				return

+ 17 - 18
pkg/provider/gcp/secretmanager/push_secret.go

@@ -16,7 +16,6 @@ package secretmanager
 
 import (
 	"bytes"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"maps"
@@ -25,6 +24,7 @@ import (
 	"github.com/tidwall/sjson"
 
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
+	"github.com/external-secrets/external-secrets/pkg/utils/metadata"
 )
 
 type PushSecretMetadataMergePolicy string
@@ -34,11 +34,12 @@ const (
 	PushSecretMetadataMergePolicyMerge   PushSecretMetadataMergePolicy = "Merge"
 )
 
-type Metadata struct {
-	Annotations map[string]string             `json:"annotations"`
-	Labels      map[string]string             `json:"labels"`
+type PushSecretMetadataSpec struct {
+	Annotations map[string]string             `json:"annotations,omitempty"`
+	Labels      map[string]string             `json:"labels,omitempty"`
 	Topics      []string                      `json:"topics,omitempty"`
 	MergePolicy PushSecretMetadataMergePolicy `json:"mergePolicy,omitempty"`
+	CMEKKeyName string                        `json:"cmekKeyName,omitempty"`
 }
 
 func newPushSecretBuilder(payload []byte, data esv1beta1.PushSecretData) (pushSecretBuilder, error) {
@@ -75,31 +76,29 @@ func (b *psBuilder) buildMetadata(_, labels map[string]string, _ []*secretmanage
 		return nil, nil, nil, fmt.Errorf("secret %v is not managed by external secrets", b.pushSecretData.GetRemoteKey())
 	}
 
-	var metadata Metadata
+	var meta *metadata.PushSecretMetadata[PushSecretMetadataSpec]
 	if b.pushSecretData.GetMetadata() != nil {
-		decoder := json.NewDecoder(bytes.NewReader(b.pushSecretData.GetMetadata().Raw))
-		// Want to return an error if unknown fields exist
-		decoder.DisallowUnknownFields()
-
-		if err := decoder.Decode(&metadata); err != nil {
-			return nil, nil, nil, fmt.Errorf("failed to decode PushSecret metadata: %w", err)
+		var err error
+		meta, err = metadata.ParseMetadataParameters[PushSecretMetadataSpec](b.pushSecretData.GetMetadata())
+		if err != nil {
+			return nil, nil, nil, fmt.Errorf("failed to parse PushSecret metadata: %w", err)
 		}
+	}
 
-		if metadata.MergePolicy == "" {
-			// Set default MergePolicy to be Replace
-			metadata.MergePolicy = PushSecretMetadataMergePolicyReplace
-		}
+	var spec PushSecretMetadataSpec
+	if meta != nil {
+		spec = meta.Spec
 	}
 
 	newLabels := map[string]string{}
-	maps.Copy(newLabels, metadata.Labels)
-	if metadata.MergePolicy == PushSecretMetadataMergePolicyMerge {
+	maps.Copy(newLabels, spec.Labels)
+	if spec.MergePolicy == PushSecretMetadataMergePolicyMerge {
 		// Keep labels from the existing GCP Secret Manager Secret
 		maps.Copy(newLabels, labels)
 	}
 	newLabels[managedByKey] = managedByValue
 
-	return metadata.Annotations, newLabels, metadata.Topics, nil
+	return spec.Annotations, newLabels, spec.Topics, nil
 }
 
 func (b *psBuilder) needUpdate(original []byte) bool {

+ 42 - 2
pkg/provider/gcp/secretmanager/push_secret_test.go

@@ -47,7 +47,14 @@ func TestBuildMetadata(t *testing.T) {
 				"someOtherKey": "someOtherValue",
 			},
 			metadata: &apiextensionsv1.JSON{
-				Raw: []byte(`{"annotations":{"key1":"value1"},"labels":{"key2":"value2"}}`),
+				Raw: []byte(`{
+					"apiVersion": "kubernetes.external-secrets.io/v1alpha1",
+					"kind": "PushSecretMetadata",
+					"spec": {
+						"annotations": {"key1":"value1"},
+						"labels": {"key2":"value2"}
+					}
+				}`),
 			},
 			expectedError: false,
 			expectedLabels: map[string]string{
@@ -66,7 +73,15 @@ func TestBuildMetadata(t *testing.T) {
 				"existingKey": "existingValue",
 			},
 			metadata: &apiextensionsv1.JSON{
-				Raw: []byte(`{"annotations":{"key1":"value1"},"labels":{"key2":"value2"},"mergePolicy":"Merge"}`),
+				Raw: []byte(`{
+					"apiVersion": "kubernetes.external-secrets.io/v1alpha1",
+					"kind": "PushSecretMetadata",
+					"spec": {
+						"annotations": {"key1":"value1"},
+						"labels": {"key2":"value2"},
+						"mergePolicy": "Merge"
+					}
+				}`),
 			},
 			expectedError: false,
 			expectedLabels: map[string]string{
@@ -79,6 +94,31 @@ func TestBuildMetadata(t *testing.T) {
 			},
 			expectedTopics: nil,
 		},
+		{
+			name: "metadata with CMEK key name",
+			labels: map[string]string{
+				managedByKey: managedByValue,
+			},
+			metadata: &apiextensionsv1.JSON{
+				Raw: []byte(`{
+					"apiVersion": "kubernetes.external-secrets.io/v1alpha1",
+					"kind": "PushSecretMetadata",
+					"spec": {
+						"annotations": {"key1":"value1"},
+						"labels": {"key2":"value2"},
+						"cmekKeyName": "projects/my-project/locations/us-east1/keyRings/my-keyring/cryptoKeys/my-key"
+					}
+				}`),
+			},
+			expectedError: false,
+			expectedLabels: map[string]string{
+				managedByKey: managedByValue,
+				"key2":       "value2",
+			},
+			expectedAnnotations: map[string]string{
+				"key1": "value1",
+			},
+		},
 	}
 
 	for _, tt := range tests {