refactor: use typed client in auth (#5743)

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

@@ -7,8 +7,9 @@ import (
 
 	"github.com/kyverno/kyverno/pkg/clients/dclient"
 	authorizationv1 "k8s.io/api/authorization/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/kubernetes"
 )
 
 // CanIOptions provides utility to check if user has authorization for the given operation
@@ -23,19 +24,23 @@ type CanIOptions interface {
 }
 
 type canIOptions struct {
-	namespace string
+	namespace   string
-	verb      string
+	verb        string
-	kind      string
+	kind        string
-	client    dclient.Interface
+	subresource string
+	discovery   dclient.IDiscovery
+	client      kubernetes.Interface
 }
 
 // NewCanI returns a new instance of operation access controller evaluator
-func NewCanI(client dclient.Interface, kind, namespace, verb string) CanIOptions {
+func NewCanI(client dclient.Interface, kind, namespace, verb, subresource string) CanIOptions {
 	return &canIOptions{
-		namespace: namespace,
+		namespace:   namespace,
-		kind:      kind,
+		verb:        verb,
-		verb:      verb,
+		kind:        kind,
-		client:    client,
+		subresource: subresource,
+		discovery:   client.Discovery(),
+		client:      client.GetKubeClient(),
 	}
 }
 
@@ -48,7 +53,7 @@ func NewCanI(client dclient.Interface, kind, namespace, verb string) CanIOptions
 func (o *canIOptions) RunAccessCheck(ctx context.Context) (bool, error) {
 	// get GroupVersionResource from RESTMapper
 	// get GVR from kind
-	gvr, err := o.client.Discovery().GetGVRFromKind(o.kind)
+	gvr, err := o.discovery.GetGVRFromKind(o.kind)
 	if err != nil {
 		return false, fmt.Errorf("failed to get GVR for kind %s", o.kind)
 	}
@@ -61,10 +66,11 @@ func (o *canIOptions) RunAccessCheck(ctx context.Context) (bool, error) {
 	sar := &authorizationv1.SelfSubjectAccessReview{
 		Spec: authorizationv1.SelfSubjectAccessReviewSpec{
 			ResourceAttributes: &authorizationv1.ResourceAttributes{
-				Namespace: o.namespace,
+				Namespace:   o.namespace,
-				Verb:      o.verb,
+				Verb:        o.verb,
-				Group:     gvr.Group,
+				Group:       gvr.Group,
-				Resource:  gvr.Resource,
+				Resource:    gvr.Resource,
+				Subresource: o.subresource,
 			},
 		},
 	}
@@ -76,42 +82,18 @@ 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.CreateResource(ctx, "", "SelfSubjectAccessReview", "", sar, false)
+	resp, err := o.client.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, sar, metav1.CreateOptions{})
 	if err != nil {
 		logger.Error(err, "failed to create resource")
 		return false, err
 	}
 
-	// status.allowed
+	if !resp.Status.Allowed {
-	allowed, ok, err := unstructured.NestedBool(resp.Object, "status", "allowed")
+		reason := resp.Status.Reason
-	if !ok {
+		evaluationError := resp.Status.EvaluationError
-		if err != nil {
-			logger.Error(err, "failed to get the field", "field", "status.allowed")
-		}
-		logger.Info("field not found", "field", "status.allowed")
-	}
-
-	if !allowed {
-		// status.reason
-		reason, ok, err := unstructured.NestedString(resp.Object, "status", "reason")
-		if !ok {
-			if err != nil {
-				logger.Error(err, "failed to get the field", "field", "status.reason")
-			}
-			logger.Info("field not found", "field", "status.reason")
-		}
-		// status.evaluationError
-		evaluationError, ok, err := unstructured.NestedString(resp.Object, "status", "evaluationError")
-		if !ok {
-			if err != nil {
-				logger.Error(err, "failed to get the field", "field", "status.evaluationError")
-			}
-			logger.Info("field not found", "field", "status.evaluationError")
-		}
-
 		// Reporting ? (just logs)
 		logger.Info("disallowed operation", "reason", reason, "evaluationError", evaluationError)
 	}
 
-	return allowed, nil
+	return resp.Status.Allowed, nil
 }

pkg/auth/auth_test.go

@@ -29,7 +29,7 @@ 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, tt.args.kind, tt.args.namespace, tt.args.verb, "")
 			assert.NotNil(t, got)
 		})
 	}
@@ -80,7 +80,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, tt.fields.kind, tt.fields.namespace, tt.fields.verb, "")
 			got, err := o.RunAccessCheck(context.TODO())
 			if tt.wantErr {
 				assert.Error(t, err)

pkg/clients/dclient/client.go

@@ -20,6 +20,8 @@ import (
 )
 
 type Interface interface {
+	// GetKubeClient provides typed kube client
+	GetKubeClient() kubernetes.Interface
 	// GetEventsInterface provides typed interface for events
 	GetEventsInterface() corev1.EventInterface
 	// GetDynamicInterface fetches underlying dynamic interface
@@ -87,6 +89,11 @@ func (c *client) NewDynamicSharedInformerFactory(defaultResync time.Duration) dy
 	return dynamicinformer.NewDynamicSharedInformerFactory(c.dyn, defaultResync)
 }
 
+// GetKubeClient provides typed kube client
+func (c *client) GetKubeClient() kubernetes.Interface {
+	return c.kube
+}
+
 // GetEventsInterface provides typed interface for events
 func (c *client) GetEventsInterface() corev1.EventInterface {
 	return c.kube.CoreV1().Events(metav1.NamespaceAll)

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, 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, 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, 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, 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, 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.NewString(spec.MatchResources.GetKinds()...)
 	for kind := range kinds {
-		checker := auth.NewCanI(client, kind, namespace, "delete")
+		checker := auth.NewCanI(client, kind, namespace, "delete", "")
 		allowed, err := checker.RunAccessCheck(ctx)
 		if err != nil {
 			return err