feat: add option to configure topic information for GCM (#4055)

* feat: add option to configure topic information for GCM

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

* fix the comparison logic for updates to include topics

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

---------

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

pkg/provider/gcp/secretmanager/client.go

@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	"maps"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -68,6 +69,7 @@ const (
 	managedByValue = "external-secrets"
 
 	providerName = "GCPSecretManager"
+	topicsKey    = "topics"
 )
 
 type Client struct {
@@ -182,15 +184,33 @@ func (c *Client) PushSecret(ctx context.Context, secret *corev1.Secret, pushSecr
 			}
 		}
 
+		scrt := &secretmanagerpb.Secret{
+			Labels: map[string]string{
+				managedByKey: managedByValue,
+			},
+			Replication: replication,
+		}
+
+		topics, err := utils.FetchValueFromMetadata(topicsKey, pushSecretData.GetMetadata(), []any{})
+		if err != nil {
+			return fmt.Errorf("failed to fetch topics from metadata: %w", err)
+		}
+
+		for _, t := range topics {
+			name, ok := t.(string)
+			if !ok {
+				return fmt.Errorf("invalid topic type")
+			}
+
+			scrt.Topics = append(scrt.Topics, &secretmanagerpb.Topic{
+				Name: name,
+			})
+		}
+
 		gcpSecret, err = c.smClient.CreateSecret(ctx, &secretmanagerpb.CreateSecretRequest{
 			Parent:   fmt.Sprintf("projects/%s", c.store.ProjectID),
 			SecretId: pushSecretData.GetRemoteKey(),
-			Secret: &secretmanagerpb.Secret{
-				Labels: map[string]string{
-					managedByKey: managedByValue,
-				},
-				Replication: replication,
-			},
+			Secret:   scrt,
 		})
 		metrics.ObserveAPICall(constants.ProviderGCPSM, constants.CallGCPSMCreateSecret, err)
 		if err != nil {
@@ -203,17 +223,25 @@ func (c *Client) PushSecret(ctx context.Context, secret *corev1.Secret, pushSecr
 		return err
 	}
 
-	annotations, labels, err := builder.buildMetadata(gcpSecret.Annotations, gcpSecret.Labels)
+	annotations, labels, topics, err := builder.buildMetadata(gcpSecret.Annotations, gcpSecret.Labels, gcpSecret.Topics)
 	if err != nil {
 		return err
 	}
 
-	if !maps.Equal(gcpSecret.Annotations, annotations) || !maps.Equal(gcpSecret.Labels, labels) {
+	// Comparing with a pointer based slice doesn't work so we are converting
+	// it to a string slice.
+	existingTopics := make([]string, 0, len(gcpSecret.Topics))
+	for _, t := range gcpSecret.Topics {
+		existingTopics = append(existingTopics, t.Name)
+	}
+
+	if !maps.Equal(gcpSecret.Annotations, annotations) || !maps.Equal(gcpSecret.Labels, labels) || !slices.Equal(existingTopics, topics) {
 		scrt := &secretmanagerpb.Secret{
 			Name:        gcpSecret.Name,
 			Etag:        gcpSecret.Etag,
 			Labels:      labels,
 			Annotations: annotations,
+			Topics:      gcpSecret.Topics,
 		}
 
 		if c.store.Location != "" {

pkg/provider/gcp/secretmanager/client_test.go

@@ -531,6 +531,25 @@ func TestPushSecret(t *testing.T) {
 			"managed-by": "external-secrets",
 		},
 	}
+	secretWithTopics := secretmanagerpb.Secret{
+		Name: "projects/default/secrets/baz",
+		Replication: &secretmanagerpb.Replication{
+			Replication: &secretmanagerpb.Replication_Automatic_{
+				Automatic: &secretmanagerpb.Replication_Automatic{},
+			},
+		},
+		Labels: map[string]string{
+			"managed-by": "external-secrets",
+		},
+		Topics: []*secretmanagerpb.Topic{
+			{
+				Name: "topic1",
+			},
+			{
+				Name: "topic2",
+			},
+		},
+	}
 	wrongLabelSecret := secretmanagerpb.Secret{
 		Name: "projects/default/secrets/foo-bar",
 		Replication: &secretmanagerpb.Replication{
@@ -677,7 +696,7 @@ func TestPushSecret(t *testing.T) {
 
 					user, ok := req.Secret.Replication.Replication.(*secretmanagerpb.Replication_UserManaged_)
 					if !ok {
-						return errors.New("req.Secret.Replication.Replication was not of type *secretmanagerpb.Replication_UserManaged_")
+						return fmt.Errorf("req.Secret.Replication.Replication was not of type *secretmanagerpb.Replication_UserManaged_ but: %T", req.Secret.Replication.Replication)
 					}
 
 					if len(user.UserManaged.Replicas) < 1 {
@@ -693,16 +712,47 @@ func TestPushSecret(t *testing.T) {
 			},
 		},
 		{
-			desc: "failed to push a secret with invalid metadata type",
+			desc: "SetSecret successfully pushes a secret with topics",
 			args: args{
-				store: &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
-				mock:  smtc.mockClient,
 				Metadata: &apiextensionsv1.JSON{
-					Raw: []byte(`{"tags":{"tag-key1":"tag-value1"}}`),
+					Raw: []byte(`{"topics":["topic1", "topic2"]}`),
 				},
-				GetSecretMockReturn: fakesm.SecretMockReturn{Secret: &secret, Err: nil}},
+				store:                         &esv1beta1.GCPSMProvider{ProjectID: smtc.projectID},
+				mock:                          &fakesm.MockSMClient{}, // the mock should NOT be shared between test cases
+				CreateSecretMockReturn:        fakesm.SecretMockReturn{Secret: &secretWithTopics, Err: nil},
+				GetSecretMockReturn:           fakesm.SecretMockReturn{Secret: nil, Err: notFoundError},
+				AccessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{Res: &res, Err: nil},
+				AddSecretVersionMockReturn:    fakesm.AddSecretVersionMockReturn{SecretVersion: &secretVersion, Err: nil}},
 			want: want{
-				err: errors.New("failed to decode PushSecret metadata"),
+				err: nil,
+				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")
+					}
+
+					if scrt.Secret == nil {
+						return errors.New("index 0 for call was nil")
+					}
+
+					if len(scrt.Secret.Topics) != 2 {
+						return fmt.Errorf("secret topics count was not 2 but: %d", len(scrt.Secret.Topics))
+					}
+
+					if scrt.Secret.Topics[0].Name != "topic1" {
+						return fmt.Errorf("secret topic name for 1 was not topic1 but: %s", scrt.Secret.Topics[0].Name)
+					}
+
+					if scrt.Secret.Topics[1].Name != "topic2" {
+						return fmt.Errorf("secret topic name for 2 was not topic2 but: %s", scrt.Secret.Topics[1].Name)
+					}
+
+					if m.UpdateSecretCallN != 0 {
+						return fmt.Errorf("updateSecret called with %d", m.UpdateSecretCallN)
+					}
+
+					return nil
+				},
 			},
 		},
 		{

pkg/provider/gcp/secretmanager/fake/fake.go

@@ -34,6 +34,7 @@ type MockSMClient struct {
 	CreateSecretCalledWithN map[int]*secretmanagerpb.CreateSecretRequest
 	createSecretCallN       int
 	updateSecretFn          func(ctx context.Context, req *secretmanagerpb.UpdateSecretRequest, opts ...gax.CallOption) (*secretmanagerpb.Secret, error)
+	UpdateSecretCallN       int
 	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
@@ -183,6 +184,7 @@ func (mc *MockSMClient) AccessSecretVersionWithError(err error) {
 }
 
 func (mc *MockSMClient) UpdateSecret(ctx context.Context, req *secretmanagerpb.UpdateSecretRequest, _ ...gax.CallOption) (*secretmanagerpb.Secret, error) {
+	mc.UpdateSecretCallN++
 	return mc.updateSecretFn(ctx, req)
 }
 

pkg/provider/gcp/secretmanager/push_secret.go

@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 
+	"cloud.google.com/go/secretmanager/apiv1/secretmanagerpb"
 	"github.com/tidwall/sjson"
 
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
@@ -28,6 +29,7 @@ import (
 type Metadata struct {
 	Annotations map[string]string `json:"annotations"`
 	Labels      map[string]string `json:"labels"`
+	Topics      []string          `json:"topics,omitempty"`
 }
 
 func newPushSecretBuilder(payload []byte, data esv1beta1.PushSecretData) (pushSecretBuilder, error) {
@@ -49,7 +51,7 @@ func newPushSecretBuilder(payload []byte, data esv1beta1.PushSecretData) (pushSe
 }
 
 type pushSecretBuilder interface {
-	buildMetadata(annotations, labels map[string]string) (map[string]string, map[string]string, error)
+	buildMetadata(annotations, labels map[string]string, topics []*secretmanagerpb.Topic) (map[string]string, map[string]string, []string, error)
 	needUpdate(original []byte) bool
 	buildData(original []byte) ([]byte, error)
 }
@@ -59,9 +61,9 @@ type psBuilder struct {
 	pushSecretData esv1beta1.PushSecretData
 }
 
-func (b *psBuilder) buildMetadata(_, labels map[string]string) (map[string]string, map[string]string, error) {
+func (b *psBuilder) buildMetadata(_, labels map[string]string, _ []*secretmanagerpb.Topic) (map[string]string, map[string]string, []string, error) {
 	if manager, ok := labels[managedByKey]; !ok || manager != managedByValue {
-		return nil, nil, fmt.Errorf("secret %v is not managed by external secrets", b.pushSecretData.GetRemoteKey())
+		return nil, nil, nil, fmt.Errorf("secret %v is not managed by external secrets", b.pushSecretData.GetRemoteKey())
 	}
 
 	var metadata Metadata
@@ -71,7 +73,7 @@ func (b *psBuilder) buildMetadata(_, labels map[string]string) (map[string]strin
 		decoder.DisallowUnknownFields()
 
 		if err := decoder.Decode(&metadata); err != nil {
-			return nil, nil, fmt.Errorf("failed to decode PushSecret metadata: %w", err)
+			return nil, nil, nil, fmt.Errorf("failed to decode PushSecret metadata: %w", err)
 		}
 	}
 
@@ -81,7 +83,7 @@ func (b *psBuilder) buildMetadata(_, labels map[string]string) (map[string]strin
 	}
 	newLabels[managedByKey] = managedByValue
 
-	return metadata.Annotations, newLabels, nil
+	return metadata.Annotations, newLabels, metadata.Topics, nil
 }
 
 func (b *psBuilder) needUpdate(original []byte) bool {
@@ -101,7 +103,7 @@ type propertyPSBuilder struct {
 	pushSecretData esv1beta1.PushSecretData
 }
 
-func (b *propertyPSBuilder) buildMetadata(annotations, labels map[string]string) (map[string]string, map[string]string, error) {
+func (b *propertyPSBuilder) buildMetadata(annotations, labels map[string]string, topics []*secretmanagerpb.Topic) (map[string]string, map[string]string, []string, error) {
 	newAnnotations := map[string]string{}
 	newLabels := map[string]string{}
 	if annotations != nil {
@@ -112,7 +114,13 @@ func (b *propertyPSBuilder) buildMetadata(annotations, labels map[string]string)
 	}
 
 	newLabels[managedByKey] = managedByValue
-	return newAnnotations, newLabels, nil
+
+	result := make([]string, 0, len(topics))
+	for _, t := range topics {
+		result = append(result, t.Name)
+	}
+
+	return newAnnotations, newLabels, result, nil
 }
 
 func (b *propertyPSBuilder) needUpdate(original []byte) bool {

pkg/utils/utils_test.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/oracle/oci-go-sdk/v65/vault"
+	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -734,6 +735,20 @@ func TestFetchValueFromMetadata(t *testing.T) {
 			wantT:   "value",
 			wantErr: false,
 		},
+		{
+			name: "digging for a slice",
+			args: args{
+				key: "topics",
+				data: &apiextensionsv1.JSON{
+					Raw: []byte(
+						`{"topics": ["topic1", "topic2"]}`,
+					),
+				},
+				def: []string{},
+			},
+			wantT:   []any{"topic1", "topic2"}, // we don't have deep type matching so it's not an []string{} but []any.
+			wantErr: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -742,9 +757,7 @@ func TestFetchValueFromMetadata(t *testing.T) {
 				t.Errorf("FetchValueFromMetadata() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
-			if !reflect.DeepEqual(gotT, tt.wantT) {
-				t.Errorf("FetchValueFromMetadata() gotT = %v, want %v", gotT, tt.wantT)
-			}
+			assert.Equal(t, tt.wantT, gotT)
 		})
 	}
 }