Browse Source

fix: ensure existing labels are retained for secrets in GCP secrets m… (#4160)

* fix: ensure existing labels are retained for secrets in GCP secrets manager for existing secrets (#4016)

Signed-off-by: Craig Newton <newtondev@gmail.com>

* fix: ensure existing labels are retained for secrets in GCP secrets manager for existing secrets (#4016)

Signed-off-by: Craig Newton <newtondev@gmail.com>

* fix: add missing header to push_secret_test.go

Signed-off-by: Craig Newton <newtondev@gmail.com>

---------

Signed-off-by: Craig Newton <newtondev@gmail.com>
Craig Newton 1 year ago
parent
commit
388158a4d4

File diff suppressed because it is too large
+ 41 - 0
docs/provider/google-secrets-manager.md


+ 21 - 5
pkg/provider/gcp/secretmanager/push_secret.go

@@ -19,6 +19,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"maps"
 
 	"cloud.google.com/go/secretmanager/apiv1/secretmanagerpb"
 	"github.com/tidwall/sjson"
@@ -26,10 +27,18 @@ import (
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
 )
 
+type PushSecretMetadataMergePolicy string
+
+const (
+	PushSecretMetadataMergePolicyReplace PushSecretMetadataMergePolicy = "Replace"
+	PushSecretMetadataMergePolicyMerge   PushSecretMetadataMergePolicy = "Merge"
+)
+
 type Metadata struct {
-	Annotations map[string]string `json:"annotations"`
-	Labels      map[string]string `json:"labels"`
-	Topics      []string          `json:"topics,omitempty"`
+	Annotations map[string]string             `json:"annotations"`
+	Labels      map[string]string             `json:"labels"`
+	Topics      []string                      `json:"topics,omitempty"`
+	MergePolicy PushSecretMetadataMergePolicy `json:"mergePolicy,omitempty"`
 }
 
 func newPushSecretBuilder(payload []byte, data esv1beta1.PushSecretData) (pushSecretBuilder, error) {
@@ -75,11 +84,18 @@ func (b *psBuilder) buildMetadata(_, labels map[string]string, _ []*secretmanage
 		if err := decoder.Decode(&metadata); err != nil {
 			return nil, nil, nil, fmt.Errorf("failed to decode PushSecret metadata: %w", err)
 		}
+
+		if metadata.MergePolicy == "" {
+			// Set default MergePolicy to be Replace
+			metadata.MergePolicy = PushSecretMetadataMergePolicyReplace
+		}
 	}
 
 	newLabels := map[string]string{}
-	if metadata.Labels != nil {
-		newLabels = metadata.Labels
+	maps.Copy(newLabels, metadata.Labels)
+	if metadata.MergePolicy == PushSecretMetadataMergePolicyMerge {
+		// Keep labels from the existing GCP Secret Manager Secret
+		maps.Copy(newLabels, labels)
 	}
 	newLabels[managedByKey] = managedByValue
 

+ 104 - 0
pkg/provider/gcp/secretmanager/push_secret_test.go

@@ -0,0 +1,104 @@
+/*
+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
+
+	http://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"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+
+	testingfake "github.com/external-secrets/external-secrets/pkg/provider/testing/fake"
+)
+
+func TestBuildMetadata(t *testing.T) {
+	tests := []struct {
+		name                string
+		labels              map[string]string
+		metadata            *apiextensionsv1.JSON
+		expectedError       bool
+		expectedLabels      map[string]string
+		expectedAnnotations map[string]string
+		expectedTopics      []string
+	}{
+		{
+			name: "secret not managed by external secrets",
+			labels: map[string]string{
+				"someKey": "someValue",
+			},
+			expectedError: true,
+		},
+		{
+			name: "metadata with default MergePolicy of Replace",
+			labels: map[string]string{
+				managedByKey:   managedByValue,
+				"someOtherKey": "someOtherValue",
+			},
+			metadata: &apiextensionsv1.JSON{
+				Raw: []byte(`{"annotations":{"key1":"value1"},"labels":{"key2":"value2"}}`),
+			},
+			expectedError: false,
+			expectedLabels: map[string]string{
+				managedByKey: managedByValue,
+				"key2":       "value2",
+			},
+			expectedAnnotations: map[string]string{
+				"key1": "value1",
+			},
+			expectedTopics: nil,
+		},
+		{
+			name: "metadata with merge policy",
+			labels: map[string]string{
+				managedByKey:  managedByValue,
+				"existingKey": "existingValue",
+			},
+			metadata: &apiextensionsv1.JSON{
+				Raw: []byte(`{"annotations":{"key1":"value1"},"labels":{"key2":"value2"},"mergePolicy":"Merge"}`),
+			},
+			expectedError: false,
+			expectedLabels: map[string]string{
+				managedByKey:  managedByValue,
+				"existingKey": "existingValue",
+				"key2":        "value2",
+			},
+			expectedAnnotations: map[string]string{
+				"key1": "value1",
+			},
+			expectedTopics: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			psData := testingfake.PushSecretData{
+				Metadata: tt.metadata,
+			}
+			builder := &psBuilder{
+				pushSecretData: psData,
+			}
+
+			annotations, labels, topics, err := builder.buildMetadata(nil, tt.labels, nil)
+			if tt.expectedError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expectedLabels, labels)
+				assert.Equal(t, tt.expectedAnnotations, annotations)
+				assert.Equal(t, tt.expectedTopics, topics)
+			}
+		})
+	}
+}