From 40d756bc0af9ab804c37c1f5b66c16e40838ad73 Mon Sep 17 00:00:00 2001 From: ldpliu Date: Thu, 8 Apr 2021 12:17:38 +0800 Subject: [PATCH] update clusterrole func --- .../clusterrole/clusterrole_controller.go | 8 ++-- .../clusterrole_controller_test.go | 2 +- .../clusterrole/clusterrole_controller.go | 46 ++++--------------- .../clusterset/clusterrole/rbac.go | 7 +++ .../syncclusterrolebinding_controller.go | 2 +- pkg/utils/role.go | 13 ++++-- pkg/utils/role_test.go | 7 +++ test/e2e/clusterset_test.go | 16 +++---- 8 files changed, 47 insertions(+), 54 deletions(-) diff --git a/pkg/controllers/clusterrole/clusterrole_controller.go b/pkg/controllers/clusterrole/clusterrole_controller.go index d8bca9bc6..ce9bb56e1 100644 --- a/pkg/controllers/clusterrole/clusterrole_controller.go +++ b/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 diff --git a/pkg/controllers/clusterrole/clusterrole_controller_test.go b/pkg/controllers/clusterrole/clusterrole_controller_test.go index 733e085af..ea09a86d9 100644 --- a/pkg/controllers/clusterrole/clusterrole_controller_test.go +++ b/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, }, diff --git a/pkg/controllers/clusterset/clusterrole/clusterrole_controller.go b/pkg/controllers/clusterset/clusterrole/clusterrole_controller.go index e6028d78d..4f53db28e 100644 --- a/pkg/controllers/clusterset/clusterrole/clusterrole_controller.go +++ b/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 diff --git a/pkg/controllers/clusterset/clusterrole/rbac.go b/pkg/controllers/clusterset/clusterrole/rbac.go index 903dd5e80..3628dc059 100644 --- a/pkg/controllers/clusterset/clusterrole/rbac.go +++ b/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"). diff --git a/pkg/controllers/clusterset/syncclusterrolebinding/syncclusterrolebinding_controller.go b/pkg/controllers/clusterset/syncclusterrolebinding/syncclusterrolebinding_controller.go index 616addc13..952c29590 100644 --- a/pkg/controllers/clusterset/syncclusterrolebinding/syncclusterrolebinding_controller.go +++ b/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" diff --git a/pkg/utils/role.go b/pkg/utils/role.go index b999e252f..53c146b39 100644 --- a/pkg/utils/role.go +++ b/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) } diff --git a/pkg/utils/role_test.go b/pkg/utils/role_test.go index 1a90b2cf5..8aee69b28 100644 --- a/pkg/utils/role_test.go +++ b/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) + } +} diff --git a/test/e2e/clusterset_test.go b/test/e2e/clusterset_test.go index 484c7658a..50a36ffa9 100644 --- a/test/e2e/clusterset_test.go +++ b/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