Merge pull request stolostron#320 from ldpliu/clusterrole

update clusterrole

pkg/controllers/clusterrole/clusterrole_controller.go

@@ -77,12 +77,12 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 			if klog.V(4) {
 				klog.Infof("deleting ManagedClusterRole %v", cluster.Name)
 			}
-			err := utils.DeleteClusterRole(r.kubeClient, utils.BuildClusterRoleName(cluster.Name, "admin"))
+			err := utils.DeleteClusterRole(r.kubeClient, utils.GenerateClusterRoleName(cluster.Name, "admin"))
 			if err != nil {
 				klog.Warningf("will reconcile since failed to delete clusterrole %v : %v", cluster.Name, err)
 				return reconcile.Result{}, err
 			}
-			err = utils.DeleteClusterRole(r.kubeClient, utils.BuildClusterRoleName(cluster.Name, "view"))
+			err = utils.DeleteClusterRole(r.kubeClient, utils.GenerateClusterRoleName(cluster.Name, "view"))
 			if err != nil {
 				klog.Warningf("will reconcile since failed to delete clusterrole %v : %v", cluster.Name, err)
 				return reconcile.Result{}, err
@@ -112,13 +112,13 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 
 	//add clusterrole
 	adminRules := buildAdminRoleRules(cluster.Name)
-	err = utils.ApplyClusterRole(r.kubeClient, utils.BuildClusterRoleName(cluster.Name, "admin"), adminRules)
+	err = utils.ApplyClusterRole(r.kubeClient, utils.GenerateClusterRoleName(cluster.Name, "admin"), adminRules)
 	if err != nil {
 		klog.Warningf("will reconcile since failed to create/update clusterrole %v, %v", cluster.Name, err)
 		return ctrl.Result{}, err
 	}
 	viewRules := buildViewRoleRules(cluster.Name)
-	err = utils.ApplyClusterRole(r.kubeClient, utils.BuildClusterRoleName(cluster.Name, "view"), viewRules)
+	err = utils.ApplyClusterRole(r.kubeClient, utils.GenerateClusterRoleName(cluster.Name, "view"), viewRules)
 	if err != nil {
 		klog.Warningf("will reconcile since failed to create/update clusterrole %v, %v", cluster.Name, err)
 		return ctrl.Result{}, err

pkg/controllers/clusterrole/clusterrole_controller_test.go

@@ -131,7 +131,7 @@ func newAdminRoleObjs() []runtime.Object {
 	return []runtime.Object{
 		&rbacv1.ClusterRole{
 			ObjectMeta: metav1.ObjectMeta{
-				Name: utils.BuildClusterRoleName(ManagedClusterName, "admin"),
+				Name: utils.GenerateClusterRoleName(ManagedClusterName, "admin"),
 			},
 			Rules: nil,
 		},

pkg/controllers/clusterset/clusterrole/clusterrole_controller.go

@@ -5,6 +5,7 @@ import (
 
 	clusterv1alpha1 "github.com/open-cluster-management/api/cluster/v1alpha1"
 	"github.com/open-cluster-management/multicloud-operators-foundation/pkg/utils"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog"
@@ -65,58 +66,31 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 
 	err := r.client.Get(ctx, req.NamespacedName, clusterset)
 	if err != nil {
-		return ctrl.Result{}, client.IgnoreNotFound(err)
-	}
-
-	// Check DeletionTimestamp to determine if object is under deletion
-	if !clusterset.GetDeletionTimestamp().IsZero() {
-		// The object is being deleted
-		if utils.ContainsString(clusterset.GetFinalizers(), clustersetRoleFinalizerName) {
-			if klog.V(4) {
-				klog.Infof("deleting ManagedClusterSetRole %v", clusterset.Name)
-			}
-			err := utils.DeleteClusterRole(r.kubeClient, utils.BuildClusterRoleName(clusterset.Name, "clusterset-admin"))
+		if errors.IsNotFound(err) {
+			err := utils.DeleteClusterRole(r.kubeClient, utils.GenerateClustersetClusterroleName(req.Name, "admin"))
 			if err != nil {
-				klog.Warningf("will reconcile since failed to delete clusterrole %v : %v", clusterset.Name, err)
+				klog.Warningf("will reconcile since failed to delete clusterrole. clusterset: %v, err: %v", req.Name, err)
 				return reconcile.Result{}, err
 			}
-			err = utils.DeleteClusterRole(r.kubeClient, utils.BuildClusterRoleName(clusterset.Name, "clusterset-view"))
+			err = utils.DeleteClusterRole(r.kubeClient, utils.GenerateClustersetClusterroleName(req.Name, "view"))
 			if err != nil {
-				klog.Warningf("will reconcile since failed to delete clusterrole %v : %v", clusterset.Name, err)
-				return reconcile.Result{}, err
-			}
-			if klog.V(4) {
-				klog.Infof("removing ManagedClusterSet Finalizer in ManagedCluster %v", clusterset.Name)
-			}
-			clusterset.ObjectMeta.Finalizers = utils.RemoveString(clusterset.ObjectMeta.Finalizers, clustersetRoleFinalizerName)
-			if err := r.client.Update(context.TODO(), clusterset); err != nil {
-				klog.Warningf("will reconcile since failed to remove Finalizer from ManagedClusterSet %v, %v", clusterset.Name, err)
+				klog.Warningf("will reconcile since failed to delete clusterrole. clusterset: %v, err: %v", req.Name, err)
 				return reconcile.Result{}, err
 			}
+			return ctrl.Result{}, nil
 		}
-		return reconcile.Result{}, nil
-	}
-
-	if !utils.ContainsString(clusterset.GetFinalizers(), clustersetRoleFinalizerName) {
-		if klog.V(4) {
-			klog.Infof("adding ManagedClusterSetRole Finalizer to ManagedClusterSet %v", clusterset.Name)
-		}
-		clusterset.ObjectMeta.Finalizers = append(clusterset.ObjectMeta.Finalizers, clustersetRoleFinalizerName)
-		if err := r.client.Update(context.TODO(), clusterset); err != nil {
-			klog.Warningf("will reconcile since failed to add finalizer to ManagedClusterSet %v, %v", clusterset.Name, err)
-			return reconcile.Result{}, err
-		}
+		return ctrl.Result{}, err
 	}
 
 	//add clusterrole
 	adminRules := buildAdminRoleRules(clusterset.Name)
-	err = utils.ApplyClusterRole(r.kubeClient, utils.BuildClusterRoleName(clusterset.Name, "clusterset-admin"), adminRules)
+	err = utils.ApplyClusterRole(r.kubeClient, utils.GenerateClustersetClusterroleName(clusterset.Name, "admin"), adminRules)
 	if err != nil {
 		klog.Warningf("will reconcile since failed to create/update clusterrole %v, %v", clusterset.Name, err)
 		return ctrl.Result{}, err
 	}
 	viewRules := buildViewRoleRules(clusterset.Name)
-	err = utils.ApplyClusterRole(r.kubeClient, utils.BuildClusterRoleName(clusterset.Name, "clusterset-view"), viewRules)
+	err = utils.ApplyClusterRole(r.kubeClient, utils.GenerateClustersetClusterroleName(clusterset.Name, "view"), viewRules)
 	if err != nil {
 		klog.Warningf("will reconcile since failed to create/update clusterrole %v, %v", clusterset.Name, err)
 		return ctrl.Result{}, err

pkg/controllers/clusterset/clusterrole/rbac.go

@@ -13,6 +13,7 @@ import (
 var managedclusterGroup = "cluster.open-cluster-management.io"
 var hiveGroup = "hive.openshift.io"
 var managedClusterViewGroup = "clusterview.open-cluster-management.io"
+var registerGroup = "register.open-cluster-management.io"
 
 // buildAdminRoleRules builds the clustesetadminroles
 func buildAdminRoleRules(clustersetName string) []rbacv1.PolicyRule {
@@ -36,6 +37,12 @@ func buildAdminRoleRules(clustersetName string) []rbacv1.PolicyRule {
 			Groups(managedclusterGroup).
 			Resources("managedclusters").
 			RuleOrDie(),
+		//TODO
+		// We will restrict the update permission only for authenticated clusterset in another pr
+		clusterrbac.NewRule("update").
+			Groups(registerGroup).
+			Resources("managedclusters/accept").
+			RuleOrDie(),
 		clusterrbac.NewRule("get", "list", "watch").
 			Groups(managedClusterViewGroup).
 			Resources("managedclustersets").

pkg/controllers/clusterset/syncclusterrolebinding/syncclusterrolebinding_controller.go

@@ -153,7 +153,7 @@ func generateClusterSubjectMap(clustersetToClusters *helpers.ClusterSetMapper, c
 
 func generateRequiredClusterRoleBinding(clusterName string, subjects []rbacv1.Subject) *rbacv1.ClusterRoleBinding {
 	clusterRoleBindingName := utils.GenerateClusterRoleBindingName(clusterName)
-	clusterRoleName := utils.GenerateClusterRoleName(clusterName)
+	clusterRoleName := utils.GenerateClusterRoleName(clusterName, "admin")
 
 	var labels = make(map[string]string)
 	labels[clusterrolebinding.ClusterSetLabel] = "true"

pkg/utils/role.go

@@ -2,6 +2,7 @@ package utils
 
 import (
 	"context"
+	"fmt"
 	"reflect"
 
 	clusterv1alpha1 "github.com/open-cluster-management/api/cluster/v1alpha1"
@@ -102,13 +103,17 @@ func ApplyClusterRoleBinding(ctx context.Context, client client.Client, required
 }
 
 //managedcluster admin role
-func GenerateClusterRoleName(clusterName string) string {
-	return "open-cluster-management:admin:" + clusterName
+func GenerateClusterRoleName(clusterName, role string) string {
+	return fmt.Sprintf("open-cluster-management:%s:%s", role, clusterName)
+}
+
+func GenerateClustersetClusterroleName(clustersetName, role string) string {
+	return fmt.Sprintf("open-cluster-management:managedclusterset:%s:%s", role, clustersetName)
 }
 
 //clusterset clusterrolebinding
 func GenerateClusterRoleBindingName(clusterName string) string {
-	return "open-cluster-management:clusterset:managedcluster:" + clusterName
+	return fmt.Sprintf("open-cluster-management:clusterset:managedcluster:%s", clusterName)
 }
 
 //Delete cluster role
@@ -148,5 +153,5 @@ func ApplyClusterRole(kubeClient kubernetes.Interface, clusterRoleName string, r
 }
 
 func BuildClusterRoleName(objName, rule string) string {
-	return "open-cluster-management:" + rule + ":" + objName
+	return fmt.Sprintf("open-cluster-management:%s:%s", rule, objName)
 }

pkg/utils/role_test.go

@@ -161,3 +161,10 @@ func verifyApply(ctx context.Context, client client.Client, required *rbacv1.Clu
 	}
 	return true
 }
+
+func TestBuildClusterRoleName(t *testing.T) {
+	roleName := BuildClusterRoleName("obj", "admin")
+	if roleName != "open-cluster-management:admin:obj" {
+		t.Errorf("Failed to generate clusterroleName: %v", roleName)
+	}
+}

test/e2e/clusterset_test.go

@@ -155,8 +155,8 @@ var _ = ginkgo.Describe("Testing ManagedClusterSet", func() {
 			gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
 
 			gomega.Eventually(func() (interface{}, error) {
-				//clusterset-admin clusterrole should be auto created
-				adminClustersetRole := utils.BuildClusterRoleName(clusterset.GetName(), "clusterset-admin")
+				//admin clusterrole should be auto created
+				adminClustersetRole := utils.GenerateClustersetClusterroleName(clusterset.GetName(), "admin")
 				_, err = util.GetClusterResource(dynamicClient, clusterRoleGVR, adminClustersetRole)
 				if err != nil {
 					return false, nil
@@ -165,8 +165,8 @@ var _ = ginkgo.Describe("Testing ManagedClusterSet", func() {
 			}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue())
 
 			gomega.Eventually(func() (interface{}, error) {
-				//clusterset-view clusterrole should be auto created
-				viewClustersetRole := utils.BuildClusterRoleName(clusterset.GetName(), "clusterset-view")
+				//view clusterrole should be auto created
+				viewClustersetRole := utils.GenerateClustersetClusterroleName(clusterset.GetName(), "view")
 				_, err = util.GetClusterResource(dynamicClient, clusterRoleGVR, viewClustersetRole)
 				if err != nil {
 					return false, nil
@@ -179,8 +179,8 @@ var _ = ginkgo.Describe("Testing ManagedClusterSet", func() {
 			gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
 
 			gomega.Eventually(func() (interface{}, error) {
-				//clusterset-admin clusterrole should be auto deleted
-				adminClustersetRole := utils.BuildClusterRoleName(clusterset.GetName(), "clusterset-admin")
+				//admin clusterrole should be auto deleted
+				adminClustersetRole := utils.GenerateClustersetClusterroleName(clusterset.GetName(), "admin")
 				_, err = util.GetClusterResource(dynamicClient, clusterRoleGVR, adminClustersetRole)
 				if err != nil {
 					return false, nil
@@ -189,8 +189,8 @@ var _ = ginkgo.Describe("Testing ManagedClusterSet", func() {
 			}, eventuallyTimeout, eventuallyInterval).Should(gomega.BeFalse())
 
 			gomega.Eventually(func() (interface{}, error) {
-				//clusterset-view clusterrole should be auto deleted
-				viewClustersetRole := utils.BuildClusterRoleName(clusterset.GetName(), "clusterset-view")
+				//view clusterrole should be auto deleted
+				viewClustersetRole := utils.GenerateClustersetClusterroleName(clusterset.GetName(), "view")
 				_, err = util.GetClusterResource(dynamicClient, clusterRoleGVR, viewClustersetRole)
 				if err != nil {
 					return false, nil