Allow specifying the same namespace for SecretStores (#3555)

* Allow specifying the same namespace for SecretStores

Signed-off-by: shuheiktgw <s-kitagawa@mercari.com>

* Fix unit tests

Signed-off-by: shuheiktgw <s-kitagawa@mercari.com>

---------

Signed-off-by: shuheiktgw <s-kitagawa@mercari.com>

pkg/provider/chef/chef_test.go

@@ -302,7 +302,7 @@ func TestValidateStore(t *testing.T) {
 		},
 		{
 			store: makeSecretStore(name, baseURL, makeAuth(authName, authNamespace, authKey)),
-			err:   fmt.Errorf("received invalid Chef SecretStore resource: namespace not allowed with namespaced SecretStore"),
+			err:   fmt.Errorf("received invalid Chef SecretStore resource: namespace should either be empty or match the namespace of the SecretStore for a namespaced SecretStore"),
 		},
 		{
 			store: &esv1beta1.SecretStore{

pkg/provider/doppler/doppler_test.go

@@ -423,7 +423,7 @@ func TestValidateStore(t *testing.T) {
 		{
 			label: "invalid store namespace not allowed",
 			store: makeSecretStore(withAuth(secretName, "", &namespace)),
-			err:   fmt.Errorf("invalid store: namespace not allowed with namespaced SecretStore"),
+			err:   fmt.Errorf("invalid store: namespace should either be empty or match the namespace of the SecretStore for a namespaced SecretStore"),
 		},
 		{
 			label: "valid provide optional dopplerToken.key",

pkg/provider/fortanix/provider_test.go

@@ -199,7 +199,7 @@ func TestValidateStore(t *testing.T) {
 					},
 				},
 			},
-			want: errors.New("namespace not allowed with namespaced SecretStore"),
+			want: errors.New("namespace should either be empty or match the namespace of the SecretStore for a namespaced SecretStore"),
 		},
 	}
 	for name, tc := range tests {

pkg/provider/gitlab/gitlab_test.go

@@ -861,7 +861,7 @@ func TestValidateStore(t *testing.T) {
 		},
 		{
 			store: makeSecretStore(project, environment, withAccessToken("userName", "userKey", &namespace)),
-			err:   fmt.Errorf("namespace not allowed with namespaced SecretStore"),
+			err:   fmt.Errorf("namespace should either be empty or match the namespace of the SecretStore for a namespaced SecretStore"),
 		},
 		{
 			store: makeSecretStore(project, environment, withAccessToken("userName", "userKey", nil)),

pkg/provider/ibm/provider_test.go

@@ -188,7 +188,7 @@ func TestValidateStore(t *testing.T) {
 	_, err = p.ValidateStore(store)
 	if err == nil {
 		t.Errorf(errExpectedErr)
-	} else if err.Error() != "namespace not allowed with namespaced SecretStore" {
+	} else if err.Error() != "namespace should either be empty or match the namespace of the SecretStore for a namespaced SecretStore" {
 		t.Errorf("KeySelector test failed: expected namespace not allowed, got %v", err)
 	}
 

pkg/provider/onboardbase/onboardbase_test.go

@@ -329,7 +329,7 @@ func TestValidateStore(t *testing.T) {
 		{
 			label: "invalid store namespace not allowed",
 			store: makeSecretStore(withAuth(secretName, "", &namespace, "passcode")),
-			err:   fmt.Errorf("invalid store: namespace not allowed with namespaced SecretStore"),
+			err:   fmt.Errorf("invalid store: namespace should either be empty or match the namespace of the SecretStore for a namespaced SecretStore"),
 		},
 		{
 			label: "valid provide optional onboardbaseAPIKey.key",

pkg/provider/onepassword/onepassword_test.go

@@ -441,7 +441,7 @@ func TestValidateStore(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: fmt.Errorf(errOnePasswordStore, fmt.Errorf("namespace not allowed with namespaced SecretStore")),
+			expectedErr: fmt.Errorf(errOnePasswordStore, fmt.Errorf("namespace should either be empty or match the namespace of the SecretStore for a namespaced SecretStore")),
 		},
 		{
 			checkNote: "invalid: more than one vault with the same number",

pkg/provider/oracle/oracle_test.go

@@ -284,7 +284,7 @@ func TestValidateStore(t *testing.T) {
 		},
 		{
 			store: makeSecretStore(vaultOCID, region, withSecretAuth(userOCID, tenant), withPrivateKey(secretName, secretKey, &namespace)),
-			err:   fmt.Errorf("namespace not allowed with namespaced SecretStore"),
+			err:   fmt.Errorf("namespace should either be empty or match the namespace of the SecretStore for a namespaced SecretStore"),
 		},
 		{
 			store: makeSecretStore(vaultOCID, region, withSecretAuth(userOCID, tenant), withPrivateKey(secretName, "", nil)),
@@ -296,7 +296,7 @@ func TestValidateStore(t *testing.T) {
 		},
 		{
 			store: makeSecretStore(vaultOCID, region, withSecretAuth(userOCID, tenant), withPrivateKey(secretName, secretKey, nil), withFingerprint(secretName, secretKey, &namespace)),
-			err:   fmt.Errorf("namespace not allowed with namespaced SecretStore"),
+			err:   fmt.Errorf("namespace should either be empty or match the namespace of the SecretStore for a namespaced SecretStore"),
 		},
 		{
 			store: makeSecretStore(vaultOCID, region, withSecretAuth(userOCID, tenant), withPrivateKey(secretName, secretKey, nil), withFingerprint(secretName, "", nil)),

pkg/utils/utils.go

@@ -359,7 +359,7 @@ func ErrorContains(out error, want string) bool {
 }
 
 var (
-	errNamespaceNotAllowed = errors.New("namespace not allowed with namespaced SecretStore")
+	errNamespaceNotAllowed = errors.New("namespace should either be empty or match the namespace of the SecretStore for a namespaced SecretStore")
 	errRequireNamespace    = errors.New("cluster scope requires namespace")
 )
 
@@ -371,7 +371,7 @@ func ValidateSecretSelector(store esv1beta1.GenericStore, ref esmeta.SecretKeySe
 	if clusterScope && ref.Namespace == nil {
 		return errRequireNamespace
 	}
-	if !clusterScope && ref.Namespace != nil {
+	if !clusterScope && ref.Namespace != nil && *ref.Namespace != store.GetNamespace() {
 		return errNamespaceNotAllowed
 	}
 	return nil
@@ -383,7 +383,7 @@ func ValidateSecretSelector(store esv1beta1.GenericStore, ref esmeta.SecretKeySe
 // support referent auth.
 func ValidateReferentSecretSelector(store esv1beta1.GenericStore, ref esmeta.SecretKeySelector) error {
 	clusterScope := store.GetObjectKind().GroupVersionKind().Kind == esv1beta1.ClusterSecretStoreKind
-	if !clusterScope && ref.Namespace != nil {
+	if !clusterScope && ref.Namespace != nil && *ref.Namespace != store.GetNamespace() {
 		return errNamespaceNotAllowed
 	}
 	return nil
@@ -397,7 +397,7 @@ func ValidateServiceAccountSelector(store esv1beta1.GenericStore, ref esmeta.Ser
 	if clusterScope && ref.Namespace == nil {
 		return errRequireNamespace
 	}
-	if !clusterScope && ref.Namespace != nil {
+	if !clusterScope && ref.Namespace != nil && *ref.Namespace != store.GetNamespace() {
 		return errNamespaceNotAllowed
 	}
 	return nil
@@ -409,7 +409,7 @@ func ValidateServiceAccountSelector(store esv1beta1.GenericStore, ref esmeta.Ser
 // support referent auth.
 func ValidateReferentServiceAccountSelector(store esv1beta1.GenericStore, ref esmeta.ServiceAccountSelector) error {
 	clusterScope := store.GetObjectKind().GroupVersionKind().Kind == esv1beta1.ClusterSecretStoreKind
-	if !clusterScope && ref.Namespace != nil {
+	if !clusterScope && ref.Namespace != nil && *ref.Namespace != store.GetNamespace() {
 		return errNamespaceNotAllowed
 	}
 	return nil

pkg/utils/utils_test.go

@@ -16,6 +16,7 @@ package utils
 
 import (
 	"encoding/json"
+	"errors"
 	"reflect"
 	"testing"
 	"time"
@@ -24,9 +25,11 @@ import (
 	"github.com/oracle/oci-go-sdk/v65/vault"
 	v1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
+	esmetav1 "github.com/external-secrets/external-secrets/apis/meta/v1"
 )
 
 const (
@@ -904,3 +907,311 @@ func TestCompareStringAndByteSlices(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateSecretSelector(t *testing.T) {
+	tests := []struct {
+		desc     string
+		store    esv1beta1.GenericStore
+		ref      esmetav1.SecretKeySelector
+		expected error
+	}{
+		{
+			desc: "cluster secret store with namespace reference",
+			store: &esv1beta1.ClusterSecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.ClusterSecretStoreKind,
+				},
+			},
+			ref: esmetav1.SecretKeySelector{
+				Namespace: Ptr("test"),
+			},
+			expected: nil,
+		},
+		{
+			desc: "secret store without namespace reference",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.SecretStoreKind,
+				},
+			},
+			ref:      esmetav1.SecretKeySelector{},
+			expected: nil,
+		},
+		{
+			desc: "secret store with the same namespace reference",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.SecretStoreKind,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "test",
+				},
+			},
+			ref: esmetav1.SecretKeySelector{
+				Namespace: Ptr("test"),
+			},
+			expected: nil,
+		},
+		{
+			desc: "cluster secret store without namespace reference",
+			store: &esv1beta1.ClusterSecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.ClusterSecretStoreKind,
+				},
+			},
+			ref:      esmetav1.SecretKeySelector{},
+			expected: errRequireNamespace,
+		},
+		{
+			desc: "secret store with the different namespace reference",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.SecretStoreKind,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "test",
+				},
+			},
+			ref: esmetav1.SecretKeySelector{
+				Namespace: Ptr("different"),
+			},
+			expected: errNamespaceNotAllowed,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			got := ValidateSecretSelector(tt.store, tt.ref)
+			if !errors.Is(got, tt.expected) {
+				t.Errorf("ValidateSecretSelector() got = %v, want = %v", got, tt.expected)
+				return
+			}
+		})
+	}
+}
+
+func TestValidateReferentSecretSelector(t *testing.T) {
+	tests := []struct {
+		desc     string
+		store    esv1beta1.GenericStore
+		ref      esmetav1.SecretKeySelector
+		expected error
+	}{
+		{
+			desc: "cluster secret store with namespace reference",
+			store: &esv1beta1.ClusterSecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.ClusterSecretStoreKind,
+				},
+			},
+			ref: esmetav1.SecretKeySelector{
+				Namespace: Ptr("test"),
+			},
+			expected: nil,
+		},
+		{
+			desc: "secret store without namespace reference",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.SecretStoreKind,
+				},
+			},
+			ref:      esmetav1.SecretKeySelector{},
+			expected: nil,
+		},
+		{
+			desc: "secret store with the same namespace reference",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.SecretStoreKind,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "test",
+				},
+			},
+			ref: esmetav1.SecretKeySelector{
+				Namespace: Ptr("test"),
+			},
+			expected: nil,
+		},
+		{
+			desc: "secret store with the different namespace reference",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.SecretStoreKind,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "test",
+				},
+			},
+			ref: esmetav1.SecretKeySelector{
+				Namespace: Ptr("different"),
+			},
+			expected: errNamespaceNotAllowed,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			got := ValidateReferentSecretSelector(tt.store, tt.ref)
+			if !errors.Is(got, tt.expected) {
+				t.Errorf("ValidateReferentSecretSelector() got = %v, want = %v", got, tt.expected)
+				return
+			}
+		})
+	}
+}
+
+func TestValidateServiceAccountSelector(t *testing.T) {
+	tests := []struct {
+		desc     string
+		store    esv1beta1.GenericStore
+		ref      esmetav1.ServiceAccountSelector
+		expected error
+	}{
+		{
+			desc: "cluster secret store with namespace reference",
+			store: &esv1beta1.ClusterSecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.ClusterSecretStoreKind,
+				},
+			},
+			ref: esmetav1.ServiceAccountSelector{
+				Namespace: Ptr("test"),
+			},
+			expected: nil,
+		},
+		{
+			desc: "secret store without namespace reference",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.SecretStoreKind,
+				},
+			},
+			ref:      esmetav1.ServiceAccountSelector{},
+			expected: nil,
+		},
+		{
+			desc: "secret store with the same namespace reference",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.SecretStoreKind,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "test",
+				},
+			},
+			ref: esmetav1.ServiceAccountSelector{
+				Namespace: Ptr("test"),
+			},
+			expected: nil,
+		},
+		{
+			desc: "cluster secret store without namespace reference",
+			store: &esv1beta1.ClusterSecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.ClusterSecretStoreKind,
+				},
+			},
+			ref:      esmetav1.ServiceAccountSelector{},
+			expected: errRequireNamespace,
+		},
+		{
+			desc: "secret store with the different namespace reference",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.SecretStoreKind,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "test",
+				},
+			},
+			ref: esmetav1.ServiceAccountSelector{
+				Namespace: Ptr("different"),
+			},
+			expected: errNamespaceNotAllowed,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			got := ValidateServiceAccountSelector(tt.store, tt.ref)
+			if !errors.Is(got, tt.expected) {
+				t.Errorf("ValidateServiceAccountSelector() got = %v, want = %v", got, tt.expected)
+				return
+			}
+		})
+	}
+}
+
+func TestValidateReferentServiceAccountSelector(t *testing.T) {
+	tests := []struct {
+		desc     string
+		store    esv1beta1.GenericStore
+		ref      esmetav1.ServiceAccountSelector
+		expected error
+	}{
+		{
+			desc: "cluster secret store with namespace reference",
+			store: &esv1beta1.ClusterSecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.ClusterSecretStoreKind,
+				},
+			},
+			ref: esmetav1.ServiceAccountSelector{
+				Namespace: Ptr("test"),
+			},
+			expected: nil,
+		},
+		{
+			desc: "secret store without namespace reference",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.SecretStoreKind,
+				},
+			},
+			ref:      esmetav1.ServiceAccountSelector{},
+			expected: nil,
+		},
+		{
+			desc: "secret store with the same namespace reference",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.SecretStoreKind,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "test",
+				},
+			},
+			ref: esmetav1.ServiceAccountSelector{
+				Namespace: Ptr("test"),
+			},
+			expected: nil,
+		},
+		{
+			desc: "secret store with the different namespace reference",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: esv1beta1.SecretStoreKind,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "test",
+				},
+			},
+			ref: esmetav1.ServiceAccountSelector{
+				Namespace: Ptr("different"),
+			},
+			expected: errNamespaceNotAllowed,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			got := ValidateReferentServiceAccountSelector(tt.store, tt.ref)
+			if !errors.Is(got, tt.expected) {
+				t.Errorf("ValidateReferentServiceAccountSelector() got = %v, want = %v", got, tt.expected)
+				return
+			}
+		})
+	}
+}