Added additional validation for a usecase where a namespace is provided for SecretStore CAprovider (#4359)

* feat: added additional validation for a use case where a namespace is specified for SecretStore caProvider

Signed-off-by: Alexander Chernov <alexander@chernov.it>

* chore: improved error message to highlight an issue when namespace is filtered out when trying to get a namespaced secret from secretstore.

Signed-off-by: Alexander Chernov <alexander@chernov.it>

* chore: fixed failing tests

Signed-off-by: Alexander Chernov <alexander@chernov.it>

---------

Signed-off-by: Alexander Chernov <alexander@chernov.it>
Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

pkg/provider/azure/keyvault/keyvault_auth_test.go

@@ -399,7 +399,7 @@ func TestAuth(t *testing.T) {
 		},
 		{
 			name:   "bad config: missing secret",
-			expErr: "cannot get Kubernetes secret \"password\": secrets \"password\" not found",
+			expErr: "cannot get Kubernetes secret \"password\" from namespace \"default\": secrets \"password\" not found",
 			store:  &defaultStore,
 			provider: &esv1beta1.AzureKVProvider{
 				AuthType: &authType,
@@ -413,7 +413,7 @@ func TestAuth(t *testing.T) {
 		},
 		{
 			name:   "cluster secret store",
-			expErr: "cannot get Kubernetes secret \"password\": secrets \"password\" not found",
+			expErr: "cannot get Kubernetes secret \"password\" from namespace \"foo\": secrets \"password\" not found",
 			store: &esv1beta1.ClusterSecretStore{
 				TypeMeta: metav1.TypeMeta{
 					Kind: esv1beta1.ClusterSecretStoreKind,

pkg/provider/gitlab/gitlab_test.go

@@ -48,7 +48,7 @@ const (
 	groupvalue            = "groupvalue"
 	groupid               = "groupId"
 	defaultErrorMessage   = "[%d] unexpected error: [%s], expected: [%s]"
-	errMissingCredentials = "cannot get Kubernetes secret \"\": secrets \"\" not found"
+	errMissingCredentials = "cannot get Kubernetes secret \"\" from namespace \"namespace\": secrets \"\" not found"
 	testKey               = "testKey"
 	findTestPrefix        = "test.*"
 )
@@ -353,7 +353,7 @@ func TestNewClient(t *testing.T) {
 	store.Spec.Provider.Gitlab.Auth.SecretRef.AccessToken.Name = authorizedKeySecretName
 	store.Spec.Provider.Gitlab.Auth.SecretRef.AccessToken.Key = authorizedKeySecretKey
 	secretClient, err = provider.NewClient(context.Background(), store, k8sClient, namespace)
-	tassert.EqualError(t, err, "cannot get Kubernetes secret \"authorizedKeySecretName\": secrets \"authorizedKeySecretName\" not found")
+	tassert.EqualError(t, err, "cannot get Kubernetes secret \"authorizedKeySecretName\" from namespace \"namespace\": secrets \"authorizedKeySecretName\" not found")
 	tassert.Nil(t, secretClient)
 
 	err = createK8sSecret(ctx, t, k8sClient, namespace, authorizedKeySecretName, authorizedKeySecretKey, toJSON(t, newFakeAuthorizedKey()))

pkg/provider/kubernetes/validate.go

@@ -41,6 +41,11 @@ func (p *Provider) ValidateStore(store esv1beta1.GenericStore) (admission.Warnin
 		k8sSpec.Server.CAProvider.Namespace == nil {
 		return nil, errors.New("CAProvider.namespace must not be empty with ClusterSecretStore")
 	}
+	if store.GetObjectKind().GroupVersionKind().Kind == esv1beta1.SecretStoreKind &&
+		k8sSpec.Server.CAProvider != nil &&
+		k8sSpec.Server.CAProvider.Namespace != nil {
+		return nil, errors.New("CAProvider.namespace must be empty with SecretStore")
+	}
 	if k8sSpec.Auth.Cert != nil {
 		if k8sSpec.Auth.Cert.ClientCert.Name == "" {
 			return nil, errors.New("ClientCert.Name cannot be empty")

pkg/provider/kubernetes/validate_test.go

@@ -86,6 +86,47 @@ func TestValidateStore(t *testing.T) {
 			wantErr: true,
 		},
 		{
+			name: "caprovider with empty namespace for cluster secret store",
+			store: &esv1beta1.ClusterSecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: "ClusterSecretStore",
+				},
+				Spec: esv1beta1.SecretStoreSpec{
+					Provider: &esv1beta1.SecretStoreProvider{
+						Kubernetes: &esv1beta1.KubernetesProvider{
+							Server: esv1beta1.KubernetesServer{
+								CAProvider: &esv1beta1.CAProvider{
+									Name: "foobar",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "caprovider with non empty namespace for secret store",
+			store: &esv1beta1.SecretStore{
+				TypeMeta: metav1.TypeMeta{
+					Kind: "SecretStore",
+				},
+				Spec: esv1beta1.SecretStoreSpec{
+					Provider: &esv1beta1.SecretStoreProvider{
+						Kubernetes: &esv1beta1.KubernetesProvider{
+							Server: esv1beta1.KubernetesServer{
+								CAProvider: &esv1beta1.CAProvider{
+									Name:      "foobar",
+									Namespace: pointer.To("noop"),
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
 			name: "invalid client cert key",
 			store: &esv1beta1.SecretStore{
 				Spec: esv1beta1.SecretStoreSpec{

pkg/provider/oracle/oracle_test.go

@@ -442,7 +442,7 @@ func TestVaultManagementServiceNewClient(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: `cannot get Kubernetes secret "non-existing-secret": secrets "non-existing-secret" not found`,
+			expectedErr: `cannot get Kubernetes secret "non-existing-secret" from namespace "default": secrets "non-existing-secret" not found`,
 		},
 		{
 			desc: "invalid retry interval",

pkg/provider/vault/provider_test.go

@@ -347,7 +347,7 @@ MIIFkTCCA3mgAwIBAgIUBEUg3m/WqAsWHG4Q/II3IePFfuowDQYJKoZIhvcNAQELBQAwWDELMAkGA1UE
 				kube: clientfake.NewClientBuilder().Build(),
 			},
 			want: want{
-				err: fmt.Errorf(`cannot get Kubernetes secret "vault-secret": %w`, errors.New(`secrets "vault-secret" not found`)),
+				err: fmt.Errorf(`cannot get Kubernetes secret "vault-secret" from namespace "default": %w`, errors.New(`secrets "vault-secret" not found`)),
 			},
 		},
 		"SuccessfulVaultStoreWithCertAuth": {

pkg/provider/yandex/certificatemanager/certificatemanager_test.go

@@ -81,7 +81,7 @@ func TestNewClient(t *testing.T) {
 	store.Spec.Provider.YandexCertificateManager.Auth.AuthorizedKey.Name = authorizedKeySecretName
 	store.Spec.Provider.YandexCertificateManager.Auth.AuthorizedKey.Key = authorizedKeySecretKey
 	secretClient, err = provider.NewClient(context.Background(), store, k8sClient, namespace)
-	tassert.EqualError(t, err, "cannot get Kubernetes secret \"authorizedKeySecretName\": secrets \"authorizedKeySecretName\" not found")
+	tassert.EqualError(t, err, "cannot get Kubernetes secret \"authorizedKeySecretName\" from namespace \"namespace\": secrets \"authorizedKeySecretName\" not found")
 	tassert.Nil(t, secretClient)
 
 	err = createK8sSecret(ctx, t, k8sClient, namespace, authorizedKeySecretName, authorizedKeySecretKey, toJSON(t, newFakeAuthorizedKey()))
@@ -96,7 +96,7 @@ func TestNewClient(t *testing.T) {
 		},
 	}
 	secretClient, err = provider.NewClient(context.Background(), store, k8sClient, namespace)
-	tassert.EqualError(t, err, "cannot get Kubernetes secret \"caCertificateSecretName\": secrets \"caCertificateSecretName\" not found")
+	tassert.EqualError(t, err, "cannot get Kubernetes secret \"caCertificateSecretName\" from namespace \"namespace\": secrets \"caCertificateSecretName\" not found")
 	tassert.Nil(t, secretClient)
 
 	err = createK8sSecret(ctx, t, k8sClient, namespace, caCertificateSecretName, caCertificateSecretKey, []byte("it-is-not-a-certificate"))

pkg/provider/yandex/lockbox/lockbox_test.go

@@ -81,7 +81,7 @@ func TestNewClient(t *testing.T) {
 	store.Spec.Provider.YandexLockbox.Auth.AuthorizedKey.Name = authorizedKeySecretName
 	store.Spec.Provider.YandexLockbox.Auth.AuthorizedKey.Key = authorizedKeySecretKey
 	secretClient, err = provider.NewClient(context.Background(), store, k8sClient, namespace)
-	tassert.EqualError(t, err, "cannot get Kubernetes secret \"authorizedKeySecretName\": secrets \"authorizedKeySecretName\" not found")
+	tassert.EqualError(t, err, "cannot get Kubernetes secret \"authorizedKeySecretName\" from namespace \"namespace\": secrets \"authorizedKeySecretName\" not found")
 	tassert.Nil(t, secretClient)
 
 	err = createK8sSecret(ctx, t, k8sClient, namespace, authorizedKeySecretName, authorizedKeySecretKey, toJSON(t, newFakeAuthorizedKey()))
@@ -96,7 +96,7 @@ func TestNewClient(t *testing.T) {
 		},
 	}
 	secretClient, err = provider.NewClient(context.Background(), store, k8sClient, namespace)
-	tassert.EqualError(t, err, "cannot get Kubernetes secret \"caCertificateSecretName\": secrets \"caCertificateSecretName\" not found")
+	tassert.EqualError(t, err, "cannot get Kubernetes secret \"caCertificateSecretName\" from namespace \"namespace\": secrets \"caCertificateSecretName\" not found")
 	tassert.Nil(t, secretClient)
 
 	err = createK8sSecret(ctx, t, k8sClient, namespace, caCertificateSecretName, caCertificateSecretKey, []byte("it-is-not-a-certificate"))

pkg/utils/resolvers/secret_ref.go

@@ -35,7 +35,7 @@ const (
 	// we can remove this and replace it with a interface.
 	EmptyStoreKind = "EmptyStoreKind"
 
-	errGetKubeSecret         = "cannot get Kubernetes secret %q: %w"
+	errGetKubeSecret         = "cannot get Kubernetes secret %q from namespace %q: %w"
 	errSecretKeyFmt          = "cannot find secret data for key: %q"
 	errGetKubeSATokenRequest = "cannot request Kubernetes service account token for service account %q: %w"
 )
@@ -61,7 +61,7 @@ func SecretKeyRef(
 	secret := &corev1.Secret{}
 	err := c.Get(ctx, key, secret)
 	if err != nil {
-		return "", fmt.Errorf(errGetKubeSecret, ref.Name, err)
+		return "", fmt.Errorf(errGetKubeSecret, ref.Name, key.Namespace, err)
 	}
 	val, ok := secret.Data[ref.Key]
 	if !ok {

pkg/utils/resolvers/secret_ref_test.go

@@ -88,7 +88,7 @@ func TestResolveSecretKeyRef(t *testing.T) {
 				Namespace: ptr.To(testNamespace),
 				Key:       testKey,
 			},
-			err: errors.New(`cannot get Kubernetes secret "test-secret": secrets "test-secret" not found`),
+			err: errors.New(`cannot get Kubernetes secret "test-secret" from namespace "other-namespace": secrets "test-secret" not found`),
 		},
 		{
 			name:      "cluster secret store may access all namespaces",