Browse Source

feat: add location to GCP push secret (#3502)

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Gergely Brautigam 1 year ago
parent
commit
94c9a33a11

+ 3 - 0
apis/externalsecrets/v1beta1/secretstore_gcpsm_types.go

@@ -46,4 +46,7 @@ type GCPSMProvider struct {
 
 	// ProjectID project where secret is located
 	ProjectID string `json:"projectID,omitempty"`
+
+	// Location optionally defines a location for a secret
+	Location string `json:"location,omitempty"`
 }

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

@@ -2761,6 +2761,10 @@ spec:
                             - serviceAccountRef
                             type: object
                         type: object
+                      location:
+                        description: Location optionally defines a location for a
+                          secret
+                        type: string
                       projectID:
                         description: ProjectID project where secret is located
                         type: string

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

@@ -2761,6 +2761,10 @@ spec:
                             - serviceAccountRef
                             type: object
                         type: object
+                      location:
+                        description: Location optionally defines a location for a
+                          secret
+                        type: string
                       projectID:
                         description: ProjectID project where secret is located
                         type: string

+ 6 - 0
deploy/crds/bundle.yaml

@@ -3232,6 +3232,9 @@ spec:
                                 - serviceAccountRef
                               type: object
                           type: object
+                        location:
+                          description: Location optionally defines a location for a secret
+                          type: string
                         projectID:
                           description: ProjectID project where secret is located
                           type: string
@@ -8604,6 +8607,9 @@ spec:
                                 - serviceAccountRef
                               type: object
                           type: object
+                        location:
+                          description: Location optionally defines a location for a secret
+                          type: string
                         projectID:
                           description: ProjectID project where secret is located
                           type: string

+ 11 - 0
docs/api/spec.md

@@ -3920,6 +3920,17 @@ string
 <p>ProjectID project where secret is located</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>location</code></br>
+<em>
+string
+</em>
+</td>
+<td>
+<p>Location optionally defines a location for a secret</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="external-secrets.io/v1beta1.GCPWorkloadIdentity">GCPWorkloadIdentity

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

@@ -152,6 +152,26 @@ func (c *Client) PushSecret(ctx context.Context, secret *corev1.Secret, pushSecr
 			return err
 		}
 
+		var replication = &secretmanagerpb.Replication{
+			Replication: &secretmanagerpb.Replication_Automatic_{
+				Automatic: &secretmanagerpb.Replication_Automatic{},
+			},
+		}
+
+		if c.store.Location != "" {
+			replication = &secretmanagerpb.Replication{
+				Replication: &secretmanagerpb.Replication_UserManaged_{
+					UserManaged: &secretmanagerpb.Replication_UserManaged{
+						Replicas: []*secretmanagerpb.Replication_UserManaged_Replica{
+							{
+								Location: c.store.Location,
+							},
+						},
+					},
+				},
+			}
+		}
+
 		gcpSecret, err = c.smClient.CreateSecret(ctx, &secretmanagerpb.CreateSecretRequest{
 			Parent:   fmt.Sprintf("projects/%s", c.store.ProjectID),
 			SecretId: pushSecretData.GetRemoteKey(),
@@ -159,11 +179,7 @@ func (c *Client) PushSecret(ctx context.Context, secret *corev1.Secret, pushSecr
 				Labels: map[string]string{
 					managedByKey: managedByValue,
 				},
-				Replication: &secretmanagerpb.Replication{
-					Replication: &secretmanagerpb.Replication_Automatic_{
-						Automatic: &secretmanagerpb.Replication_Automatic{},
-					},
-				},
+				Replication: replication,
 			},
 		})
 		metrics.ObserveAPICall(constants.ProviderGCPSM, constants.CallGCPSMCreateSecret, err)
@@ -183,13 +199,29 @@ func (c *Client) PushSecret(ctx context.Context, secret *corev1.Secret, pushSecr
 	}
 
 	if !maps.Equal(gcpSecret.Annotations, annotations) || !maps.Equal(gcpSecret.Labels, labels) {
+		scrt := &secretmanagerpb.Secret{
+			Name:        gcpSecret.Name,
+			Etag:        gcpSecret.Etag,
+			Labels:      labels,
+			Annotations: annotations,
+		}
+
+		if c.store.Location != "" {
+			scrt.Replication = &secretmanagerpb.Replication{
+				Replication: &secretmanagerpb.Replication_UserManaged_{
+					UserManaged: &secretmanagerpb.Replication_UserManaged{
+						Replicas: []*secretmanagerpb.Replication_UserManaged_Replica{
+							{
+								Location: c.store.Location,
+							},
+						},
+					},
+				},
+			}
+		}
+
 		_, err = c.smClient.UpdateSecret(ctx, &secretmanagerpb.UpdateSecretRequest{
-			Secret: &secretmanagerpb.Secret{
-				Name:        gcpSecret.Name,
-				Etag:        gcpSecret.Etag,
-				Labels:      labels,
-				Annotations: annotations,
-			},
+			Secret: scrt,
 			UpdateMask: &field_mask.FieldMask{
 				Paths: []string{"labels", "annotations"},
 			},

+ 77 - 6
pkg/provider/gcp/secretmanager/client_test.go

@@ -46,7 +46,7 @@ type secretManagerTestCase struct {
 	apiErr         error
 	expectError    string
 	expectedSecret string
-	// for testing secretmap
+	// for testing SecretMap
 	expectedData map[string][]byte
 }
 
@@ -576,6 +576,7 @@ func TestPushSecret(t *testing.T) {
 	var secretVersion = secretmanagerpb.SecretVersion{}
 
 	type args struct {
+		store                         *esv1beta1.GCPSMProvider
 		mock                          *fakesm.MockSMClient
 		Metadata                      *apiextensionsv1.JSON
 		GetSecretMockReturn           fakesm.SecretMockReturn
@@ -587,6 +588,7 @@ func TestPushSecret(t *testing.T) {
 
 	type want struct {
 		err error
+		req func(*fakesm.MockSMClient) error
 	}
 	tests := []struct {
 		desc string
@@ -596,6 +598,7 @@ func TestPushSecret(t *testing.T) {
 		{
 			desc: "SetSecret successfully pushes a secret",
 			args: args{
+				store:                         &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
 				mock:                          smtc.mockClient,
 				GetSecretMockReturn:           fakesm.SecretMockReturn{Secret: &secret, Err: nil},
 				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res, Err: nil},
@@ -607,7 +610,8 @@ func TestPushSecret(t *testing.T) {
 		{
 			desc: "successfully pushes a secret with metadata",
 			args: args{
-				mock: smtc.mockClient,
+				store: &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
+				mock:  smtc.mockClient,
 				Metadata: &apiextensionsv1.JSON{
 					Raw: []byte(`{"annotations":{"annotation-key1":"annotation-value1"},"labels":{"label-key1":"label-value1"}}`),
 				},
@@ -634,9 +638,64 @@ 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"},
+				mock:                smtc.mockClient,
+				GetSecretMockReturn: fakesm.SecretMockReturn{Secret: nil, Err: notFoundError},
+				CreateSecretMockReturn: fakesm.SecretMockReturn{Secret: &secretmanagerpb.Secret{
+					Name: "projects/default/secrets/baz",
+					Replication: &secretmanagerpb.Replication{
+						Replication: &secretmanagerpb.Replication_UserManaged_{
+							UserManaged: &secretmanagerpb.Replication_UserManaged{
+								Replicas: []*secretmanagerpb.Replication_UserManaged_Replica{
+									{
+										Location: "us-east-1",
+									},
+								},
+							},
+						},
+					},
+					Labels: map[string]string{
+						"managed-by": "external-secrets",
+						"label-key1": "label-value1",
+					},
+					Annotations: map[string]string{
+						"annotation-key1": "annotation-value1",
+					},
+				}, Err: nil},
+				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res, Err: nil},
+				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil}},
+			want: want{
+				err: nil,
+				req: func(m *fakesm.MockSMClient) error {
+					req, ok := m.CreateSecretCalledWithN[0]
+					if !ok {
+						return fmt.Errorf("index 0 for call not found in the list of calls")
+					}
+
+					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_")
+					}
+
+					if len(user.UserManaged.Replicas) < 1 {
+						return fmt.Errorf("req.Secret.Replication.Replication.Replicas was not empty")
+					}
+
+					if user.UserManaged.Replicas[0].Location != "us-east-1" {
+						return fmt.Errorf("req.Secret.Replication.Replicas[0].Location was not equal to us-east-1 but was %s", user.UserManaged.Replicas[0].Location)
+					}
+
+					return nil
+				},
+			},
+		},
+		{
 			desc: "failed to push a secret with invalid metadata type",
 			args: args{
-				mock: smtc.mockClient,
+				store: &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
+				mock:  smtc.mockClient,
 				Metadata: &apiextensionsv1.JSON{
 					Raw: []byte(`{"tags":{"tag-key1":"tag-value1"}}`),
 				},
@@ -648,6 +707,7 @@ func TestPushSecret(t *testing.T) {
 		{
 			desc: "secret not pushed if AddSecretVersion errors",
 			args: args{
+				store:                         &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
 				mock:                          smtc.mockClient,
 				GetSecretMockReturn:           fakesm.SecretMockReturn{Secret: &secret, Err: nil},
 				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res, Err: nil},
@@ -660,6 +720,7 @@ func TestPushSecret(t *testing.T) {
 		{
 			desc: "secret not pushed if AccessSecretVersion errors",
 			args: args{
+				store:                         &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
 				mock:                          smtc.mockClient,
 				GetSecretMockReturn:           fakesm.SecretMockReturn{Secret: &secret, Err: nil},
 				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: nil, Err: APIerror},
@@ -671,6 +732,7 @@ func TestPushSecret(t *testing.T) {
 		{
 			desc: "secret not pushed if not managed-by external-secrets",
 			args: args{
+				store:               &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
 				mock:                smtc.mockClient,
 				GetSecretMockReturn: fakesm.SecretMockReturn{Secret: &wrongLabelSecret, Err: nil},
 			},
@@ -681,6 +743,7 @@ func TestPushSecret(t *testing.T) {
 		{
 			desc: "don't push a secret with the same key and value",
 			args: args{
+				store:                         &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
 				mock:                          smtc.mockClient,
 				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res2, Err: nil},
 				GetSecretMockReturn:           fakesm.SecretMockReturn{Secret: &secret, Err: nil},
@@ -692,6 +755,7 @@ func TestPushSecret(t *testing.T) {
 		{
 			desc: "secret is created if one doesn't already exist",
 			args: args{
+				store:                         &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
 				mock:                          smtc.mockClient,
 				GetSecretMockReturn:           fakesm.SecretMockReturn{Secret: nil, Err: notFoundError},
 				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: nil, Err: notFoundError},
@@ -705,6 +769,7 @@ func TestPushSecret(t *testing.T) {
 		{
 			desc: "secret not created if CreateSecret returns not found error",
 			args: args{
+				store:                  &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
 				mock:                   smtc.mockClient,
 				GetSecretMockReturn:    fakesm.SecretMockReturn{Secret: nil, Err: notFoundError},
 				CreateSecretMockReturn: fakesm.SecretMockReturn{Secret: &secret, Err: notFoundError},
@@ -716,6 +781,7 @@ func TestPushSecret(t *testing.T) {
 		{
 			desc: "secret not created if CreateSecret returns error",
 			args: args{
+				store:               &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
 				mock:                smtc.mockClient,
 				GetSecretMockReturn: fakesm.SecretMockReturn{Secret: nil, Err: canceledError},
 			},
@@ -726,6 +792,7 @@ func TestPushSecret(t *testing.T) {
 		{
 			desc: "access secret version for an existing secret returns error",
 			args: args{
+				store:                         &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
 				mock:                          smtc.mockClient,
 				GetSecretMockReturn:           fakesm.SecretMockReturn{Secret: &secret, Err: nil},
 				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: nil, Err: canceledError},
@@ -745,9 +812,7 @@ func TestPushSecret(t *testing.T) {
 
 			c := Client{
 				smClient: tc.args.mock,
-				store: &esv1beta1.GCPSMProvider{
-					ProjectID: smtc.projectID,
-				},
+				store:    tc.args.store,
 			}
 			s := &corev1.Secret{Data: map[string][]byte{secretKey: []byte("fake-value")}}
 			data := testingfake.PushSecretData{
@@ -771,6 +836,12 @@ func TestPushSecret(t *testing.T) {
 			if tc.want.err != nil {
 				t.Errorf("expected to receive an error but got nil")
 			}
+
+			if tc.want.req != nil {
+				if err := tc.want.req(tc.args.mock); err != nil {
+					t.Errorf("received an unexpected error while checking request: %v", err)
+				}
+			}
 		})
 	}
 }

+ 16 - 8
pkg/provider/gcp/secretmanager/fake/fake.go

@@ -27,14 +27,16 @@ import (
 )
 
 type MockSMClient struct {
-	accessSecretFn func(ctx context.Context, req *secretmanagerpb.AccessSecretVersionRequest, opts ...gax.CallOption) (*secretmanagerpb.AccessSecretVersionResponse, error)
-	ListSecretsFn  func(ctx context.Context, req *secretmanagerpb.ListSecretsRequest, opts ...gax.CallOption) *secretmanager.SecretIterator
-	AddSecretFn    func(ctx context.Context, req *secretmanagerpb.AddSecretVersionRequest, opts ...gax.CallOption) (*secretmanagerpb.SecretVersion, error)
-	createSecretFn func(ctx context.Context, req *secretmanagerpb.CreateSecretRequest, opts ...gax.CallOption) (*secretmanagerpb.Secret, error)
-	updateSecretFn func(ctx context.Context, req *secretmanagerpb.UpdateSecretRequest, opts ...gax.CallOption) (*secretmanagerpb.Secret, error)
-	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
+	accessSecretFn          func(ctx context.Context, req *secretmanagerpb.AccessSecretVersionRequest, opts ...gax.CallOption) (*secretmanagerpb.AccessSecretVersionResponse, error)
+	ListSecretsFn           func(ctx context.Context, req *secretmanagerpb.ListSecretsRequest, opts ...gax.CallOption) *secretmanager.SecretIterator
+	AddSecretFn             func(ctx context.Context, req *secretmanagerpb.AddSecretVersionRequest, opts ...gax.CallOption) (*secretmanagerpb.SecretVersion, error)
+	createSecretFn          func(ctx context.Context, req *secretmanagerpb.CreateSecretRequest, opts ...gax.CallOption) (*secretmanagerpb.Secret, error)
+	CreateSecretCalledWithN map[int]*secretmanagerpb.CreateSecretRequest
+	createSecretCallN       int
+	updateSecretFn          func(ctx context.Context, req *secretmanagerpb.UpdateSecretRequest, opts ...gax.CallOption) (*secretmanagerpb.Secret, error)
+	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
 }
 
 type AccessSecretVersionMockReturn struct {
@@ -98,6 +100,12 @@ func (mc *MockSMClient) NewAddSecretVersionFn(mock AddSecretVersionMockReturn) {
 }
 
 func (mc *MockSMClient) CreateSecret(ctx context.Context, req *secretmanagerpb.CreateSecretRequest, _ ...gax.CallOption) (*secretmanagerpb.Secret, error) {
+	if mc.CreateSecretCalledWithN == nil {
+		mc.CreateSecretCalledWithN = make(map[int]*secretmanagerpb.CreateSecretRequest)
+	}
+	mc.CreateSecretCalledWithN[mc.createSecretCallN] = req
+	mc.createSecretCallN++
+
 	return mc.createSecretFn(ctx, req)
 }