Browse Source

fix: allow cross-namespace push with ClusterSecretStore objects (#5998)

Gergely Bräutigam 1 month ago
parent
commit
fea5257d83

+ 22 - 19
providers/v1/kubernetes/client.go

@@ -115,37 +115,43 @@ func (c *Client) PushSecret(ctx context.Context, secret *v1.Secret, data esv1.Pu
 	if data.GetProperty() == "" && data.GetSecretKey() != "" {
 		return errors.New("requires property in RemoteRef to push secret value if secret key is defined")
 	}
+
+	targetNamespace := c.store.RemoteNamespace
+	pushMeta, err := metadata.ParseMetadataParameters[PushSecretMetadataSpec](data.GetMetadata())
+	if err != nil {
+		return fmt.Errorf("unable to parse metadata parameters: %w", err)
+	}
+	if pushMeta != nil && pushMeta.Spec.RemoteNamespace != "" {
+		if c.storeKind != esv1.ClusterSecretStoreKind {
+			return fmt.Errorf("remoteNamespace override is only supported with ClusterSecretStore")
+		}
+		targetNamespace = pushMeta.Spec.RemoteNamespace
+	}
+
+	secretClient := c.secretsClientFor(targetNamespace)
 	remoteSecret := &v1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			Namespace: c.store.RemoteNamespace,
+			Namespace: targetNamespace,
 			Name:      data.GetRemoteKey(),
 		},
 	}
 
-	return c.createOrUpdate(ctx, remoteSecret, func() error {
-		return c.mergePushSecretData(data, remoteSecret, secret)
+	return c.createOrUpdate(ctx, secretClient, remoteSecret, func() error {
+		return c.mergePushSecretData(data, pushMeta, remoteSecret, secret)
 	})
 }
 
-func (c *Client) mergePushSecretData(remoteRef esv1.PushSecretData, remoteSecret, localSecret *v1.Secret) error {
-	// apply secret type
+func (c *Client) mergePushSecretData(remoteRef esv1.PushSecretData, pushMeta *metadata.PushSecretMetadata[PushSecretMetadataSpec], remoteSecret, localSecret *v1.Secret) error {
 	secretType := v1.SecretTypeOpaque
 	if localSecret.Type != "" {
 		secretType = localSecret.Type
 	}
 	remoteSecret.Type = secretType
 
-	// merge secret data with existing secret data
 	if remoteSecret.Data == nil {
 		remoteSecret.Data = make(map[string][]byte)
 	}
 
-	pushMeta, err := metadata.ParseMetadataParameters[PushSecretMetadataSpec](remoteRef.GetMetadata())
-	if err != nil {
-		return fmt.Errorf("unable to parse metadata parameters: %w", err)
-	}
-
-	// merge metadata based on the policy
 	var targetLabels, targetAnnotations map[string]string
 	sourceLabels, sourceAnnotations, err := mergeSourceMetadata(localSecret, pushMeta)
 	if err != nil {
@@ -157,9 +163,6 @@ func (c *Client) mergePushSecretData(remoteRef esv1.PushSecretData, remoteSecret
 	}
 	remoteSecret.ObjectMeta.Labels = targetLabels
 	remoteSecret.ObjectMeta.Annotations = targetAnnotations
-	if pushMeta != nil && pushMeta.Spec.RemoteNamespace != "" {
-		remoteSecret.ObjectMeta.Namespace = pushMeta.Spec.RemoteNamespace
-	}
 
 	// case 1: push the whole secret
 	if remoteRef.GetProperty() == "" {
@@ -185,8 +188,8 @@ func (c *Client) mergePushSecretData(remoteRef esv1.PushSecretData, remoteSecret
 	return nil
 }
 
-func (c *Client) createOrUpdate(ctx context.Context, targetSecret *v1.Secret, f func() error) error {
-	target, err := c.userSecretClient.Get(ctx, targetSecret.Name, metav1.GetOptions{})
+func (c *Client) createOrUpdate(ctx context.Context, secretClient KClient, targetSecret *v1.Secret, f func() error) error {
+	target, err := secretClient.Get(ctx, targetSecret.Name, metav1.GetOptions{})
 	metrics.ObserveAPICall(constants.ProviderKubernetes, constants.CallKubernetesGetSecret, err)
 	if err != nil {
 		if !apierrors.IsNotFound(err) {
@@ -195,7 +198,7 @@ func (c *Client) createOrUpdate(ctx context.Context, targetSecret *v1.Secret, f
 		if err := f(); err != nil {
 			return err
 		}
-		_, err := c.userSecretClient.Create(ctx, targetSecret, metav1.CreateOptions{})
+		_, err := secretClient.Create(ctx, targetSecret, metav1.CreateOptions{})
 		metrics.ObserveAPICall(constants.ProviderKubernetes, constants.CallKubernetesCreateSecret, err)
 		if err != nil {
 			return err
@@ -213,7 +216,7 @@ func (c *Client) createOrUpdate(ctx context.Context, targetSecret *v1.Secret, f
 		return nil
 	}
 
-	_, err = c.userSecretClient.Update(ctx, targetSecret, metav1.UpdateOptions{})
+	_, err = secretClient.Update(ctx, targetSecret, metav1.UpdateOptions{})
 	metrics.ObserveAPICall(constants.ProviderKubernetes, constants.CallKubernetesUpdateSecret, err)
 	if err != nil {
 		return err

+ 79 - 4
providers/v1/kubernetes/client_test.go

@@ -30,6 +30,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/kubernetes/fake"
 
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
 	"github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
@@ -788,10 +789,11 @@ func TestPushSecret(t *testing.T) {
 		Client KClient
 	}
 	tests := []struct {
-		name   string
-		fields fields
-		data   testingfake.PushSecretData
-		secret *v1.Secret
+		name      string
+		fields    fields
+		storeKind string
+		data      testingfake.PushSecretData
+		secret    *v1.Secret
 
 		wantSecretMap map[string]*v1.Secret
 		wantErr       bool
@@ -1408,6 +1410,7 @@ func TestPushSecret(t *testing.T) {
 					secretMap: map[string]*v1.Secret{},
 				},
 			},
+			storeKind: esv1.ClusterSecretStoreKind,
 			secret: &v1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "mysec",
@@ -1445,6 +1448,7 @@ func TestPushSecret(t *testing.T) {
 			p := &Client{
 				userSecretClient: tt.fields.Client,
 				store:            &esv1.KubernetesProvider{},
+				storeKind:        tt.storeKind,
 			}
 			err := p.PushSecret(context.Background(), tt.secret, tt.data)
 			if (err != nil) != tt.wantErr {
@@ -1459,3 +1463,74 @@ func TestPushSecret(t *testing.T) {
 		})
 	}
 }
+
+func TestPushSecretRemoteNamespaceRouting(t *testing.T) {
+	secretKey := "secret-key"
+	storeNamespace := "store-ns"
+	targetNamespace := "target-ns"
+
+	fakeClientset := fake.NewSimpleClientset()
+	coreV1 := fakeClientset.CoreV1()
+
+	p := &Client{
+		userCoreV1:       coreV1,
+		userSecretClient: coreV1.Secrets(storeNamespace),
+		storeKind:        esv1.ClusterSecretStoreKind,
+		store: &esv1.KubernetesProvider{
+			RemoteNamespace: storeNamespace,
+		},
+	}
+
+	localSecret := &v1.Secret{
+		Data: map[string][]byte{secretKey: []byte("bar")},
+	}
+	data := testingfake.PushSecretData{
+		SecretKey: secretKey,
+		RemoteKey: "mysec",
+		Property:  "secret",
+		Metadata: &apiextensionsv1.JSON{
+			Raw: []byte(`{"apiVersion":"kubernetes.external-secrets.io/v1alpha1", "kind": "PushSecretMetadata", "spec": {"remoteNamespace": "` + targetNamespace + `"}}`),
+		},
+	}
+
+	err := p.PushSecret(t.Context(), localSecret, data)
+	if err != nil {
+		t.Fatalf("PushSecret failed: %v", err)
+	}
+
+	got, err := coreV1.Secrets(targetNamespace).Get(t.Context(), "mysec", metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("secret not found in target namespace %q: %v", targetNamespace, err)
+	}
+	if got.Namespace != targetNamespace {
+		t.Errorf("secret namespace = %q, want %q", got.Namespace, targetNamespace)
+	}
+
+	_, err = coreV1.Secrets(storeNamespace).Get(t.Context(), "mysec", metav1.GetOptions{})
+	if !apierrors.IsNotFound(err) {
+		t.Errorf("secret should not exist in store namespace %q, got err: %v", storeNamespace, err)
+	}
+}
+
+func TestPushSecretRemoteNamespaceRejectedForSecretStore(t *testing.T) {
+	p := &Client{
+		userSecretClient: &fakeClient{t: t, secretMap: map[string]*v1.Secret{}},
+		storeKind:        esv1.SecretStoreKind,
+		store:            &esv1.KubernetesProvider{},
+	}
+
+	data := testingfake.PushSecretData{
+		RemoteKey: "mysec",
+		Metadata: &apiextensionsv1.JSON{
+			Raw: []byte(`{"apiVersion":"kubernetes.external-secrets.io/v1alpha1", "kind": "PushSecretMetadata", "spec": {"remoteNamespace": "other-ns"}}`),
+		},
+	}
+
+	err := p.PushSecret(t.Context(), &v1.Secret{Data: map[string][]byte{"k": []byte("v")}}, data)
+	if err == nil {
+		t.Fatal("expected error for remoteNamespace with SecretStore, got nil")
+	}
+	if !strings.Contains(err.Error(), "ClusterSecretStore") {
+		t.Errorf("error should mention ClusterSecretStore, got: %v", err)
+	}
+}

+ 12 - 1
providers/v1/kubernetes/provider.go

@@ -69,6 +69,9 @@ type Client struct {
 	// ctrlClientset is a client-go CoreV1() client
 	// with RBAC scope of the controller (privileged!)
 	ctrlClientset typedcorev1.CoreV1Interface
+	// userCoreV1 is a client-go CoreV1() interface
+	// with user-defined scope, used for dynamic namespace resolution.
+	userCoreV1 typedcorev1.CoreV1Interface
 	// userSecretClient is a client-go CoreV1().Secrets() client
 	// with user-defined scope.
 	userSecretClient KClient
@@ -136,12 +139,20 @@ func (p *Provider) newClient(ctx context.Context, store esv1.GenericStore, ctrlC
 	if err != nil {
 		return nil, fmt.Errorf("error configuring clientset: %w", err)
 	}
-	client.userSecretClient = userClientset.CoreV1().Secrets(client.store.RemoteNamespace)
+	client.userCoreV1 = userClientset.CoreV1()
+	client.userSecretClient = client.userCoreV1.Secrets(client.store.RemoteNamespace)
 	client.userReviewClient = userClientset.AuthorizationV1().SelfSubjectRulesReviews()
 	client.userAccessReviewClient = userClientset.AuthorizationV1().SelfSubjectAccessReviews()
 	return client, nil
 }
 
+func (c *Client) secretsClientFor(namespace string) KClient {
+	if c.userCoreV1 != nil && namespace != "" && namespace != c.store.RemoteNamespace {
+		return c.userCoreV1.Secrets(namespace)
+	}
+	return c.userSecretClient
+}
+
 func isReferentSpec(prov *esv1.KubernetesProvider) bool {
 	if prov.Auth == nil {
 		return false