refactor: auth package and add full unit test coverage (#5749)

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

pkg/auth/auth.go

@@ -5,13 +5,17 @@ import (
 	"fmt"
 	"reflect"
 
-	"github.com/kyverno/kyverno/pkg/clients/dclient"
 	authorizationv1 "k8s.io/api/authorization/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/client-go/kubernetes"
+	authorizationv1client "k8s.io/client-go/kubernetes/typed/authorization/v1"
 )
 
+// Discovery provides interface to mange Kind and GVR mapping
+type Discovery interface {
+	GetGVRFromKind(kind string) (schema.GroupVersionResource, error)
+}
+
 // CanIOptions provides utility to check if user has authorization for the given operation
 type CanIOptions interface {
 	// RunAccessCheck checks if the caller can perform the operation
@@ -28,19 +32,19 @@ type canIOptions struct {
 	verb        string
 	kind        string
 	subresource string
-	discovery   dclient.IDiscovery
+	discovery   Discovery
-	client      kubernetes.Interface
+	ssarClient  authorizationv1client.SelfSubjectAccessReviewInterface
 }
 
 // NewCanI returns a new instance of operation access controller evaluator
-func NewCanI(client dclient.Interface, kind, namespace, verb, subresource string) CanIOptions {
+func NewCanI(discovery Discovery, ssarClient authorizationv1client.SelfSubjectAccessReviewInterface, kind, namespace, verb, subresource string) CanIOptions {
 	return &canIOptions{
 		namespace:   namespace,
 		verb:        verb,
 		kind:        kind,
 		subresource: subresource,
-		discovery:   client.Discovery(),
+		discovery:   discovery,
-		client:      client.GetKubeClient(),
+		ssarClient:  ssarClient,
 	}
 }
 
@@ -82,7 +86,7 @@ func (o *canIOptions) RunAccessCheck(ctx context.Context) (bool, error) {
 	logger := logger.WithValues("kind", sar.Kind, "namespace", sar.Namespace, "name", sar.Name)
 
 	// Create the Resource
-	resp, err := o.client.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, sar, metav1.CreateOptions{})
+	resp, err := o.ssarClient.Create(ctx, sar, metav1.CreateOptions{})
 	if err != nil {
 		logger.Error(err, "failed to create resource")
 		return false, err

pkg/auth/auth_test.go

@@ -2,10 +2,15 @@ package auth
 
 import (
 	"context"
+	"errors"
 	"testing"
 
 	"github.com/kyverno/kyverno/pkg/clients/dclient"
 	"github.com/stretchr/testify/assert"
+	v1 "k8s.io/api/authorization/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	authorizationv1client "k8s.io/client-go/kubernetes/typed/authorization/v1"
 )
 
 func TestNewCanI(t *testing.T) {
@@ -29,12 +34,100 @@ func TestNewCanI(t *testing.T) {
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := NewCanI(tt.args.client, tt.args.kind, tt.args.namespace, tt.args.verb, "")
+			got := NewCanI(tt.args.client.Discovery(), tt.args.client.GetKubeClient().AuthorizationV1().SelfSubjectAccessReviews(), tt.args.kind, tt.args.namespace, tt.args.verb, "")
 			assert.NotNil(t, got)
 		})
 	}
 }
 
+type discovery struct{}
+
+func (d *discovery) GetGVRFromKind(kind string) (schema.GroupVersionResource, error) {
+	return schema.GroupVersionResource{}, errors.New("dummy")
+}
+
+func TestCanIOptions_DiscoveryError(t *testing.T) {
+	type fields struct {
+		namespace string
+		verb      string
+		kind      string
+		discovery Discovery
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		want    bool
+		wantErr bool
+	}{{
+		name: "deployments",
+		fields: fields{
+			discovery: &discovery{},
+			kind:      "Deployment",
+			namespace: "default",
+			verb:      "test",
+		},
+		want:    false,
+		wantErr: true,
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			o := NewCanI(tt.fields.discovery, nil, tt.fields.kind, tt.fields.namespace, tt.fields.verb, "")
+			got, err := o.RunAccessCheck(context.TODO())
+			if tt.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.want, got)
+			}
+		})
+	}
+}
+
+type ssar struct{}
+
+func (d *ssar) Create(_ context.Context, _ *v1.SelfSubjectAccessReview, _ metav1.CreateOptions) (*v1.SelfSubjectAccessReview, error) {
+	return nil, errors.New("dummy")
+}
+
+func TestCanIOptions_SsarError(t *testing.T) {
+	type fields struct {
+		namespace  string
+		verb       string
+		kind       string
+		discovery  Discovery
+		ssarClient authorizationv1client.SelfSubjectAccessReviewInterface
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		want    bool
+		wantErr bool
+	}{{
+		name: "deployments",
+		fields: fields{
+			discovery:  dclient.NewEmptyFakeClient().Discovery(),
+			ssarClient: &ssar{},
+			kind:       "Deployment",
+			namespace:  "default",
+			verb:       "test",
+		},
+		want:    false,
+		wantErr: true,
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			o := NewCanI(tt.fields.discovery, tt.fields.ssarClient, tt.fields.kind, tt.fields.namespace, tt.fields.verb, "")
+			got, err := o.RunAccessCheck(context.TODO())
+			if tt.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.want, got)
+			}
+		})
+	}
+}
+
 func TestCanIOptions_RunAccessCheck(t *testing.T) {
 	type fields struct {
 		namespace string
@@ -80,7 +173,7 @@ func TestCanIOptions_RunAccessCheck(t *testing.T) {
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			o := NewCanI(tt.fields.client, tt.fields.kind, tt.fields.namespace, tt.fields.verb, "")
+			o := NewCanI(tt.fields.client.Discovery(), tt.fields.client.GetKubeClient().AuthorizationV1().SelfSubjectAccessReviews(), tt.fields.kind, tt.fields.namespace, tt.fields.verb, "")
 			got, err := o.RunAccessCheck(context.TODO())
 			if tt.wantErr {
 				assert.Error(t, err)

pkg/engine/k8smanifest.go

@@ -399,7 +399,7 @@ func checkManifestAnnotations(mnfstAnnotations map[string]string, annotations ma
 }
 
 func checkDryRunPermission(dclient dclient.Interface, kind, namespace string) (bool, error) {
-	canI := auth.NewCanI(dclient, kind, namespace, "create", "")
+	canI := auth.NewCanI(dclient.Discovery(), dclient.GetKubeClient().AuthorizationV1().SelfSubjectAccessReviews(), kind, namespace, "create", "")
 	ok, err := canI.RunAccessCheck(context.TODO())
 	if err != nil {
 		return false, err

pkg/policy/generate/auth.go

@@ -37,7 +37,7 @@ func NewAuth(client dclient.Interface, log logr.Logger) *Auth {
 
 // CanICreate returns 'true' if self can 'create' resource
 func (a *Auth) CanICreate(ctx context.Context, kind, namespace string) (bool, error) {
-	canI := auth.NewCanI(a.client, kind, namespace, "create", "")
+	canI := auth.NewCanI(a.client.Discovery(), a.client.GetKubeClient().AuthorizationV1().SelfSubjectAccessReviews(), kind, namespace, "create", "")
 	ok, err := canI.RunAccessCheck(ctx)
 	if err != nil {
 		return false, err
@@ -47,7 +47,7 @@ func (a *Auth) CanICreate(ctx context.Context, kind, namespace string) (bool, er
 
 // CanIUpdate returns 'true' if self can 'update' resource
 func (a *Auth) CanIUpdate(ctx context.Context, kind, namespace string) (bool, error) {
-	canI := auth.NewCanI(a.client, kind, namespace, "update", "")
+	canI := auth.NewCanI(a.client.Discovery(), a.client.GetKubeClient().AuthorizationV1().SelfSubjectAccessReviews(), kind, namespace, "update", "")
 	ok, err := canI.RunAccessCheck(ctx)
 	if err != nil {
 		return false, err
@@ -57,7 +57,7 @@ func (a *Auth) CanIUpdate(ctx context.Context, kind, namespace string) (bool, er
 
 // CanIDelete returns 'true' if self can 'delete' resource
 func (a *Auth) CanIDelete(ctx context.Context, kind, namespace string) (bool, error) {
-	canI := auth.NewCanI(a.client, kind, namespace, "delete", "")
+	canI := auth.NewCanI(a.client.Discovery(), a.client.GetKubeClient().AuthorizationV1().SelfSubjectAccessReviews(), kind, namespace, "delete", "")
 	ok, err := canI.RunAccessCheck(ctx)
 	if err != nil {
 		return false, err
@@ -67,7 +67,7 @@ func (a *Auth) CanIDelete(ctx context.Context, kind, namespace string) (bool, er
 
 // CanIGet returns 'true' if self can 'get' resource
 func (a *Auth) CanIGet(ctx context.Context, kind, namespace string) (bool, error) {
-	canI := auth.NewCanI(a.client, kind, namespace, "get", "")
+	canI := auth.NewCanI(a.client.Discovery(), a.client.GetKubeClient().AuthorizationV1().SelfSubjectAccessReviews(), kind, namespace, "get", "")
 	ok, err := canI.RunAccessCheck(ctx)
 	if err != nil {
 		return false, err

pkg/validation/cleanuppolicy/validate.go

@@ -63,7 +63,7 @@ func validateAuth(ctx context.Context, client dclient.Interface, policy kyvernov
 	spec := policy.GetSpec()
 	kinds := sets.New(spec.MatchResources.GetKinds()...)
 	for kind := range kinds {
-		checker := auth.NewCanI(client, kind, namespace, "delete", "")
+		checker := auth.NewCanI(client.Discovery(), client.GetKubeClient().AuthorizationV1().SelfSubjectAccessReviews(), kind, namespace, "delete", "")
 		allowed, err := checker.RunAccessCheck(ctx)
 		if err != nil {
 			return err