From 9b1f75b56d12a0736dda813682007c8f797779c0 Mon Sep 17 00:00:00 2001
From: Soumya Ghosh Dastidar <gdsoumya@gmail.com>
Date: Mon, 10 Jul 2023 20:56:59 +0530
Subject: [PATCH] feat: resolve review comments

Signed-off-by: Soumya Ghosh Dastidar <gdsoumya@gmail.com>
---
 pkg/cache/cluster.go | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/pkg/cache/cluster.go b/pkg/cache/cluster.go
index 1b820a678..7e15f89f8 100644
--- a/pkg/cache/cluster.go
+++ b/pkg/cache/cluster.go
@@ -491,7 +491,7 @@ func (c *clusterCache) startMissingWatches() error {
 
 			err := c.processApi(client, api, func(resClient dynamic.ResourceInterface, ns string) error {
 				resourceVersion, err := c.loadInitialState(ctx, api, resClient, ns)
-				if err != nil && c.respectRBAC != RespectRbacDisabled && (k8sErrors.IsForbidden(err) || k8sErrors.IsUnauthorized(err)) {
+				if err != nil && c.isRestrictedResource(err) {
 					keep := false
 					if c.respectRBAC == RespectRbacStrict {
 						k, permErr := c.checkPermission(ctx, clientset.AuthorizationV1().SelfSubjectAccessReviews(), api)
@@ -726,7 +726,7 @@ func (c *clusterCache) processApi(client dynamic.Interface, api kube.APIResource
 	resClient := client.Resource(api.GroupVersionResource)
 	switch {
 	// if manage whole cluster or resource is cluster level and cluster resources enabled
-	case len(c.namespaces) == 0 || !api.Meta.Namespaced && c.clusterResources:
+	case len(c.namespaces) == 0 || (!api.Meta.Namespaced && c.clusterResources):
 		return callback(resClient, "")
 	// if manage some namespaces and resource is namespaced
 	case len(c.namespaces) != 0 && api.Meta.Namespaced:
@@ -741,6 +741,11 @@ func (c *clusterCache) processApi(client dynamic.Interface, api kube.APIResource
 	return nil
 }
 
+// isRestrictedResource checks if the kube api call is unauthorized or forbidden
+func (c *clusterCache) isRestrictedResource(err error) bool {
+	return c.respectRBAC != RespectRbacDisabled && (k8sErrors.IsForbidden(err) || k8sErrors.IsUnauthorized(err))
+}
+
 // checkPermission runs a self subject access review to check if the controller has permissions to list the resource
 func (c *clusterCache) checkPermission(ctx context.Context, reviewInterface authType1.SelfSubjectAccessReviewInterface, api kube.APIResourceInfo) (keep bool, err error) {
 	sar := &authorizationv1.SelfSubjectAccessReview{
@@ -755,10 +760,10 @@ func (c *clusterCache) checkPermission(ctx context.Context, reviewInterface auth
 
 	switch {
 	// if manage whole cluster or resource is cluster level and cluster resources enabled
-	case len(c.namespaces) == 0 || !api.Meta.Namespaced && c.clusterResources:
+	case len(c.namespaces) == 0 || (!api.Meta.Namespaced && c.clusterResources):
 		resp, err := reviewInterface.Create(ctx, sar, metav1.CreateOptions{})
 		if err != nil {
-			return true, err
+			return false, err
 		}
 		if resp != nil && resp.Status.Allowed {
 			return true, nil
@@ -771,7 +776,7 @@ func (c *clusterCache) checkPermission(ctx context.Context, reviewInterface auth
 			sar.Spec.ResourceAttributes.Namespace = ns
 			resp, err := reviewInterface.Create(ctx, sar, metav1.CreateOptions{})
 			if err != nil {
-				return true, err
+				return false, err
 			}
 			if resp != nil && resp.Status.Allowed {
 				return true, nil
@@ -861,7 +866,7 @@ func (c *clusterCache) sync() error {
 				})
 			})
 			if err != nil {
-				if c.respectRBAC != RespectRbacDisabled && (k8sErrors.IsForbidden(err) || k8sErrors.IsUnauthorized(err)) {
+				if c.isRestrictedResource(err) {
 					keep := false
 					if c.respectRBAC == RespectRbacStrict {
 						k, permErr := c.checkPermission(ctx, clientset.AuthorizationV1().SelfSubjectAccessReviews(), api)