evaluate user access to managed cluster by selfSubjectAccess with Imp…

…ersonate user kube client (#140)

Signed-off-by: Xiangjing Li <xiangli@redhat.com>

pkg/controller/mcmhub/hook.go

@@ -658,7 +658,7 @@ func GetClustersByPlacement(instance *subv1.Subscription, kubeclient client.Clie
 		if instance.Spec.Placement.PlacementRef != nil {
 			clusters, err = getClustersFromPlacementRef(instance, kubeclient, logger)
 		} else {
-			clustermap, err := placementutils.PlaceByGenericPlacmentFields(kubeclient, instance.Spec.Placement.GenericPlacementFields, nil, instance)
+			clustermap, err := placementutils.PlaceByGenericPlacmentFields(kubeclient, instance.Spec.Placement.GenericPlacementFields, instance)
 			if err != nil {
 				logger.Error(err, " - Failed to get clusters from generic fields with error.")
 				return nil, err

pkg/controller/mcmhub/placement.go

@@ -58,7 +58,7 @@ func (r *ReconcileSubscription) getClustersByPlacement(instance *appSubV1.Subscr
 	if instance.Spec.Placement.PlacementRef != nil {
 		clusters, err = r.getClustersFromPlacementRef(instance)
 	} else {
-		clustermap, err := placementutils.PlaceByGenericPlacmentFields(r.Client, instance.Spec.Placement.GenericPlacementFields, nil, instance)
+		clustermap, err := placementutils.PlaceByGenericPlacmentFields(r.Client, instance.Spec.Placement.GenericPlacementFields, instance)
 		if err != nil {
 			klog.Error("Failed to get clusters from generic fields with error: ", err)
 			return nil, err

pkg/placementrule/controller/placementrule/placement.go

@@ -23,21 +23,25 @@ import (
 	rbacv1 "k8s.io/api/authorization/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
 	spokeClusterV1 "open-cluster-management.io/api/cluster/v1"
 	appv1alpha1 "open-cluster-management.io/multicloud-operators-subscription/pkg/apis/apps/placementrule/v1"
 	"open-cluster-management.io/multicloud-operators-subscription/pkg/placementrule/utils"
 )
 
 func (r *ReconcilePlacementRule) hubReconcile(instance *appv1alpha1.PlacementRule) error {
-	clmap, err := utils.PlaceByGenericPlacmentFields(r.Client, instance.Spec.GenericPlacementFields, r.authClient, instance)
+	clmap, err := utils.PlaceByGenericPlacmentFields(r.Client, instance.Spec.GenericPlacementFields, instance)
 	if err != nil {
 		klog.Error("Error in preparing clusters by status:", err)
+
 		return err
 	}
 
 	err = r.filteClustersByStatus(instance, clmap /* , clstatusmap */)
 	if err != nil {
 		klog.Error("Error in filtering clusters by status:", err)
+
 		return err
 	}
 
@@ -290,9 +294,19 @@ func (r *ReconcilePlacementRule) filteClustersByIdentityAnno(instance *appv1alph
 		return nil
 	}
 
+	user, groups := utils.ExtractUserAndGroup(objanno)
+
+	userKubeConfig := r.authConfig
+	userKubeConfig.Impersonate = rest.ImpersonationConfig{
+		UserName: user,
+		Groups:   groups,
+	}
+
+	userKubeClient := kubernetes.NewForConfigOrDie(userKubeConfig)
+
 	for clusterName, cl := range clmap {
 		manageCluster := cl.DeepCopy()
-		if !r.checkUserPermission(objanno, manageCluster) {
+		if !r.checkUserPermission(userKubeClient, manageCluster, user, groups) {
 			delete(clmap, clusterName)
 		}
 	}
@@ -301,37 +315,28 @@ func (r *ReconcilePlacementRule) filteClustersByIdentityAnno(instance *appv1alph
 }
 
 // checkUserPermission checks if user can get managedCluster KIND resource.
-func (r *ReconcilePlacementRule) checkUserPermission(annotations map[string]string, managedCluster *spokeClusterV1.ManagedCluster) bool {
-	user, groups := utils.ExtractUserAndGroup(annotations)
+func (r *ReconcilePlacementRule) checkUserPermission(userKubeClient *kubernetes.Clientset, managedCluster *spokeClusterV1.ManagedCluster,
+	user string, groups []string) bool {
 	clusterName := managedCluster.GetName()
 
-	sar := &rbacv1.SubjectAccessReview{
-		Spec: rbacv1.SubjectAccessReviewSpec{
+	selfSAR := &rbacv1.SelfSubjectAccessReview{
+		Spec: rbacv1.SelfSubjectAccessReviewSpec{
 			ResourceAttributes: &rbacv1.ResourceAttributes{
 				Name:     clusterName,
 				Group:    "cluster.open-cluster-management.io",
 				Verb:     "get",
 				Resource: "managedclusters",
 			},
-			User:   user,
-			Groups: groups,
 		},
 	}
 
-	result, err := r.authClient.AuthorizationV1().SubjectAccessReviews().Create(context.TODO(), sar, v1.CreateOptions{})
+	result, err := userKubeClient.AuthorizationV1().SelfSubjectAccessReviews().Create(context.TODO(), selfSAR, v1.CreateOptions{})
 	klog.Infof("user: %v, groups: %v, cluster:%v, result:%v, err:%v", user, groups, clusterName, result, err)
 
 	if err != nil {
 		return false
 	}
 
-	// According to https://docs.openshift.com/container-platform/4.9/rest_api/authorization_apis/subjectaccessreview-authorization-k8s-io-v1.html
-	// If both allowed is false and denied is false, then the authorizer has no opinion on whether to authorize the action
-	// We should return true in this case
-	if !result.Status.Allowed && !result.Status.Denied {
-		return true
-	}
-
 	if !result.Status.Allowed {
 		return false
 	}

pkg/placementrule/controller/placementrule/placementrule_controller.go

@@ -24,7 +24,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -49,11 +49,8 @@ func Add(mgr manager.Manager) error {
 // newReconciler returns a new reconcile.Reconciler
 func newReconciler(mgr manager.Manager) reconcile.Reconciler {
 	authCfg := mgr.GetConfig()
-	authCfg.QPS = 100.0
-	authCfg.Burst = 200
-	kubeClient := kubernetes.NewForConfigOrDie(authCfg)
 
-	return &ReconcilePlacementRule{Client: mgr.GetClient(), scheme: mgr.GetScheme(), authClient: kubeClient}
+	return &ReconcilePlacementRule{Client: mgr.GetClient(), scheme: mgr.GetScheme(), authConfig: authCfg}
 }
 
 // add adds a new Controller to mgr with r as the reconcile.Reconciler
@@ -94,7 +91,7 @@ var _ reconcile.Reconciler = &ReconcilePlacementRule{}
 // ReconcilePlacementRule reconciles a PlacementRule object
 type ReconcilePlacementRule struct {
 	client.Client
-	authClient kubernetes.Interface
+	authConfig *rest.Config
 	scheme     *runtime.Scheme
 }
 

pkg/placementrule/utils/placement.go

@@ -24,7 +24,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
 	spokeClusterV1 "open-cluster-management.io/api/cluster/v1"
 	appv1alpha1 "open-cluster-management.io/multicloud-operators-subscription/pkg/apis/apps/placementrule/v1"
@@ -43,7 +42,7 @@ func ToPlaceLocal(placement *appv1alpha1.Placement) bool {
 // Top priority: clusterNames, ignore selector
 // Bottomline: Use label selector
 func PlaceByGenericPlacmentFields(kubeclient client.Client, placement appv1alpha1.GenericPlacementFields,
-	authclient kubernetes.Interface, object runtime.Object) (map[string]*spokeClusterV1.ManagedCluster, error) {
+	object runtime.Object) (map[string]*spokeClusterV1.ManagedCluster, error) {
 	clmap := make(map[string]*spokeClusterV1.ManagedCluster)
 
 	var labelSelector *metav1.LabelSelector

pkg/placementrule/utils/utils_test.go

@@ -159,7 +159,7 @@ func TestPlacementRule(t *testing.T) {
 	g.Expect(err).NotTo(gomega.HaveOccurred())
 
 	// test PlaceByGenericPlacmentFields
-	clmap, err := PlaceByGenericPlacmentFields(c, placementrule1.Spec.GenericPlacementFields, kubeClient, placementrule1)
+	clmap, err := PlaceByGenericPlacmentFields(c, placementrule1.Spec.GenericPlacementFields, placementrule1)
 
 	g.Expect(err).NotTo(gomega.HaveOccurred())
 	g.Expect(len(clmap)).To(gomega.Equal(1))