From c2e45b0244b2b40292ace0a307866f3d335e644a Mon Sep 17 00:00:00 2001 From: Burak Yuksel Date: Fri, 25 Mar 2022 15:25:48 +0100 Subject: [PATCH 1/4] Validate for Kubernetes Provider --- pkg/provider/kubernetes/kubernetes.go | 32 +++++++++++++++- pkg/provider/kubernetes/kubernetes_test.go | 44 ++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/pkg/provider/kubernetes/kubernetes.go b/pkg/provider/kubernetes/kubernetes.go index 4672f76cb..83b1cfb44 100644 --- a/pkg/provider/kubernetes/kubernetes.go +++ b/pkg/provider/kubernetes/kubernetes.go @@ -18,6 +18,7 @@ import ( "context" "fmt" + authv1 "k8s.io/api/authorization/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -46,9 +47,15 @@ type KClient interface { Get(ctx context.Context, name string, opts metav1.GetOptions) (*corev1.Secret, error) } +type RClient interface { + Create(ctx context.Context, SelfSubjectAccessReview *authv1.SelfSubjectAccessReview, opts metav1.CreateOptions) (*authv1.SelfSubjectAccessReview, error) +} + // ProviderKubernetes is a provider for Kubernetes. type ProviderKubernetes struct { - Client KClient + Client KClient + ReviewClient RClient + Namespace string } var _ provider.SecretsClient = &ProviderKubernetes{} @@ -106,6 +113,8 @@ func (k *ProviderKubernetes) NewClient(ctx context.Context, store esv1beta1.Gene } k.Client = kubeClientSet.CoreV1().Secrets(bStore.store.RemoteNamespace) + k.Namespace = bStore.store.RemoteNamespace + k.ReviewClient = kubeClientSet.AuthorizationV1().SelfSubjectAccessReviews() return k, nil } @@ -231,5 +240,26 @@ func (k *BaseClient) fetchSecretKey(ctx context.Context, key esmeta.SecretKeySel } func (k *ProviderKubernetes) Validate() error { + + ctx := context.Background() + + authReview, err := k.ReviewClient.Create(ctx, &authv1.SelfSubjectAccessReview{ + Spec: authv1.SelfSubjectAccessReviewSpec{ + ResourceAttributes: &authv1.ResourceAttributes{ + Resource: "secrets", + Namespace: k.Namespace, + Verb: "get", + }, + }, + }, metav1.CreateOptions{}) + + if err != nil { + return fmt.Errorf("could not verify if client is valid: %w", err) + } + + if !authReview.Status.Allowed { + return fmt.Errorf("client is not allowed to get secrets") + } + return nil } diff --git a/pkg/provider/kubernetes/kubernetes_test.go b/pkg/provider/kubernetes/kubernetes_test.go index 1a93a5f4b..63e54e4b7 100644 --- a/pkg/provider/kubernetes/kubernetes_test.go +++ b/pkg/provider/kubernetes/kubernetes_test.go @@ -21,6 +21,7 @@ import ( "strings" "testing" + authv1 "k8s.io/api/authorization/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" fclient "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -47,6 +48,17 @@ func (fk fakeClient) Get(ctx context.Context, name string, opts metav1.GetOption return &secret, nil } +type fakeReviewClient struct { + authReview *authv1.SelfSubjectAccessReview +} + +func (fk fakeReviewClient) Create(ctx context.Context, SelfSubjectAccessReview *authv1.SelfSubjectAccessReview, opts metav1.CreateOptions) (*authv1.SelfSubjectAccessReview, error) { + if fk.authReview == nil { + return nil, errors.New("Something went wrong") + } + return fk.authReview, nil +} + func TestKubernetesSecretManagerGetSecret(t *testing.T) { expected := make(map[string][]byte) value := "bar" @@ -258,3 +270,35 @@ func ErrorContains(out error, want string) bool { } return strings.Contains(out.Error(), want) } + +func TestValidate(t *testing.T) { + authReview := authv1.SelfSubjectAccessReview{ + Status: authv1.SubjectAccessReviewStatus{ + Allowed: true, + }, + } + fakeClient := fakeReviewClient{authReview: &authReview} + k := ProviderKubernetes{ReviewClient: fakeClient} + err := k.Validate() + if err != nil { + t.Errorf("Test Failed! %v", err) + } + authReview = authv1.SelfSubjectAccessReview{ + Status: authv1.SubjectAccessReviewStatus{ + Allowed: false, + }, + } + fakeClient = fakeReviewClient{authReview: &authReview} + k = ProviderKubernetes{ReviewClient: fakeClient} + err = k.Validate() + if err.Error() != "client is not allowed to get secrets" { + t.Errorf("Test Failed! Wanted client is not allowed to get secrets got: %v", err) + } + + fakeClient = fakeReviewClient{} + k = ProviderKubernetes{ReviewClient: fakeClient} + err = k.Validate() + if err.Error() != "could not verify if client is valid: Something went wrong" { + t.Errorf("Test Failed! Wanted could not verify if client is valid: Something went wrong got: %v", err) + } +} From 2f1a5b8ee7acbe781d4542bd683d06fda8e89f17 Mon Sep 17 00:00:00 2001 From: Burak Yuksel Date: Fri, 25 Mar 2022 15:35:11 +0100 Subject: [PATCH 2/4] For failing tests --- pkg/provider/kubernetes/kubernetes.go | 1 - pkg/provider/kubernetes/kubernetes_test.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/provider/kubernetes/kubernetes.go b/pkg/provider/kubernetes/kubernetes.go index 83b1cfb44..092c9a75a 100644 --- a/pkg/provider/kubernetes/kubernetes.go +++ b/pkg/provider/kubernetes/kubernetes.go @@ -240,7 +240,6 @@ func (k *BaseClient) fetchSecretKey(ctx context.Context, key esmeta.SecretKeySel } func (k *ProviderKubernetes) Validate() error { - ctx := context.Background() authReview, err := k.ReviewClient.Create(ctx, &authv1.SelfSubjectAccessReview{ diff --git a/pkg/provider/kubernetes/kubernetes_test.go b/pkg/provider/kubernetes/kubernetes_test.go index 63e54e4b7..2e4d20bb2 100644 --- a/pkg/provider/kubernetes/kubernetes_test.go +++ b/pkg/provider/kubernetes/kubernetes_test.go @@ -52,7 +52,7 @@ type fakeReviewClient struct { authReview *authv1.SelfSubjectAccessReview } -func (fk fakeReviewClient) Create(ctx context.Context, SelfSubjectAccessReview *authv1.SelfSubjectAccessReview, opts metav1.CreateOptions) (*authv1.SelfSubjectAccessReview, error) { +func (fk fakeReviewClient) Create(ctx context.Context, selfSubjectAccessReview *authv1.SelfSubjectAccessReview, opts metav1.CreateOptions) (*authv1.SelfSubjectAccessReview, error) { if fk.authReview == nil { return nil, errors.New("Something went wrong") } From b766dd226d521dd0d98f3919ebe2948dedb27888 Mon Sep 17 00:00:00 2001 From: Burak Yuksel Date: Fri, 25 Mar 2022 15:40:58 +0100 Subject: [PATCH 3/4] For failing SonarCloud tests --- pkg/provider/kubernetes/kubernetes_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/provider/kubernetes/kubernetes_test.go b/pkg/provider/kubernetes/kubernetes_test.go index 2e4d20bb2..e0c0cac37 100644 --- a/pkg/provider/kubernetes/kubernetes_test.go +++ b/pkg/provider/kubernetes/kubernetes_test.go @@ -33,6 +33,7 @@ import ( const ( errTestFetchCredentialsSecret = "test could not fetch Credentials secret failed" errTestAuthValue = "test failed key didn't match expected value" + errSomethingWentWrong = "Something went wrong" ) type fakeClient struct { @@ -43,7 +44,7 @@ func (fk fakeClient) Get(ctx context.Context, name string, opts metav1.GetOption secret, ok := fk.secretMap[name] if !ok { - return nil, errors.New("Something went wrong") + return nil, errors.New(errSomethingWentWrong) } return &secret, nil } @@ -54,7 +55,7 @@ type fakeReviewClient struct { func (fk fakeReviewClient) Create(ctx context.Context, selfSubjectAccessReview *authv1.SelfSubjectAccessReview, opts metav1.CreateOptions) (*authv1.SelfSubjectAccessReview, error) { if fk.authReview == nil { - return nil, errors.New("Something went wrong") + return nil, errors.New(errSomethingWentWrong) } return fk.authReview, nil } @@ -82,7 +83,7 @@ func TestKubernetesSecretManagerGetSecret(t *testing.T) { ref = esv1beta1.ExternalSecretDataRemoteRef{Key: "Key2", Property: "foo"} _, err := kp.GetSecret(ctx, ref) - if err.Error() != "Something went wrong" { + if err.Error() != errSomethingWentWrong { t.Error("test failed") } From 43a65a089b6513066a9543daddb7845ee5e46e85 Mon Sep 17 00:00:00 2001 From: Burak Yuksel Date: Tue, 29 Mar 2022 16:59:28 +0200 Subject: [PATCH 4/4] Documentation note added for required permission --- docs/provider-kubernetes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/provider-kubernetes.md b/docs/provider-kubernetes.md index 00c072a88..4d6ceeebc 100644 --- a/docs/provider-kubernetes.md +++ b/docs/provider-kubernetes.md @@ -4,6 +4,8 @@ External Secrets Operator allows to retrieve in-cluster secrets or from a remote It's possible to authenticate against the Kubernetes API using client certificates, a bearer token or a service account (not implemented yet). The operator enforces that exactly one authentication method is used. +**NOTE:** `SelfSubjectAccessReview` permission is required for the service account in order to validation work properly. + ## Example ### In-cluster secrets using Client certificates