review comments

controllers/apps/component/transformer_component_rbac.go

@@ -26,6 +26,7 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -62,8 +63,8 @@ func (t *componentRBACTransformer) Transform(ctx graph.TransformContext, dag *gr
 
 	var serviceAccountName string
 	var sa *corev1.ServiceAccount
-	// If the user has disabled rbac manager or specified comp.Spec.ServiceAccountName, it is now the user's responsibility to
-	// provide appropriate serviceaccount.
+	// If the user has disabled rbac manager or specified comp.Spec.ServiceAccountName, it is now
+	// the user's responsibility to provide appropriate serviceaccount.
 	if serviceAccountName = transCtx.Component.Spec.ServiceAccountName; serviceAccountName != "" {
 		// if user provided serviceaccount does not exist, raise error
 		sa := &corev1.ServiceAccount{}
@@ -119,6 +120,11 @@ func isLifecycleActionsEnabled(compDef *appsv1.ComponentDefinition) bool {
 	return compDef.Spec.LifecycleActions != nil
 }
 
+func labelAndAnnotationEqual(old, new metav1.Object) bool {
+	return equality.Semantic.DeepEqual(old.GetLabels(), new.GetLabels()) &&
+		equality.Semantic.DeepEqual(old.GetAnnotations(), new.GetAnnotations())
+}
+
 func createOrUpdate[T any, PT generics.PObject[T]](
 	transCtx *componentTransformContext, obj PT, graphCli model.GraphClient, dag *graph.DAG, cmpFn func(oldObj, newObj PT) bool,
 ) (PT, error) {
@@ -146,7 +152,8 @@ func createOrUpdateServiceAccount(transCtx *componentTransformContext, serviceAc
 	}
 
 	return createOrUpdate(transCtx, sa, graphCli, dag, func(old, new *corev1.ServiceAccount) bool {
-		return equality.Semantic.DeepEqual(old.ImagePullSecrets, new.ImagePullSecrets) &&
+		return labelAndAnnotationEqual(old, new) &&
+			equality.Semantic.DeepEqual(old.ImagePullSecrets, new.ImagePullSecrets) &&
 			equality.Semantic.DeepEqual(old.Secrets, new.Secrets) &&
 			equality.Semantic.DeepEqual(old.AutomountServiceAccountToken, new.AutomountServiceAccountToken)
 	})
@@ -155,23 +162,26 @@ func createOrUpdateServiceAccount(transCtx *componentTransformContext, serviceAc
 func createOrUpdateRole(
 	transCtx *componentTransformContext, graphCli model.GraphClient, dag *graph.DAG,
 ) (*rbacv1.Role, error) {
-	role := factory.BuildComponentRole(transCtx.SynthesizeComponent, transCtx.CompDef)
+	role := factory.BuildRole(transCtx.SynthesizeComponent, transCtx.CompDef)
 	if role == nil {
 		return nil, nil
 	}
 	if err := setCompOwnershipNFinalizer(transCtx.Component, role); err != nil {
 		return nil, err
 	}
 	return createOrUpdate(transCtx, role, graphCli, dag, func(old, new *rbacv1.Role) bool {
-		return equality.Semantic.DeepEqual(old.Rules, new.Rules)
+		return labelAndAnnotationEqual(old, new) &&
+			equality.Semantic.DeepEqual(old.Rules, new.Rules)
 	})
 }
 
 func createOrUpdateRoleBinding(
 	transCtx *componentTransformContext, cmpdRole *rbacv1.Role, serviceAccountName string, graphCli model.GraphClient, dag *graph.DAG,
 ) ([]*rbacv1.RoleBinding, error) {
 	cmpRoleBinding := func(old, new *rbacv1.RoleBinding) bool {
-		return equality.Semantic.DeepEqual(old.Subjects, new.Subjects) && equality.Semantic.DeepEqual(old.RoleRef, new.RoleRef)
+		return labelAndAnnotationEqual(old, new) &&
+			equality.Semantic.DeepEqual(old.Subjects, new.Subjects) &&
+			equality.Semantic.DeepEqual(old.RoleRef, new.RoleRef)
 	}
 	res := make([]*rbacv1.RoleBinding, 0)
 

controllers/apps/component/transformer_component_rbac_test.go

@@ -29,6 +29,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 
 	appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
@@ -132,6 +133,59 @@ var _ = Describe("object rbac transformer test.", func() {
 	}
 
 	Context("transformer rbac manager", func() {
+		It("tests labelAndAnnotationEqual", func() {
+			// nil and not nil
+			Expect(labelAndAnnotationEqual(
+				&corev1.ServiceAccount{
+					ObjectMeta: metav1.ObjectMeta{
+						Labels: map[string]string{},
+					},
+				}, &corev1.ServiceAccount{}),
+			).Should(BeTrue())
+			// labels are equal
+			Expect(labelAndAnnotationEqual(&corev1.ServiceAccount{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{
+						"test": "test",
+					},
+				},
+			}, &corev1.ServiceAccount{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{
+						"test": "test",
+					},
+				},
+			})).Should(BeTrue())
+			// labels are not equal
+			Expect(labelAndAnnotationEqual(&corev1.ServiceAccount{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{
+						"test": "test",
+					},
+				},
+			}, &corev1.ServiceAccount{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{
+						"test": "test1",
+					},
+				},
+			})).Should(BeFalse())
+			// annotations are not equal
+			Expect(labelAndAnnotationEqual(&corev1.ServiceAccount{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"test": "test",
+					},
+				},
+			}, &corev1.ServiceAccount{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"test": "test1",
+					},
+				},
+			})).Should(BeFalse())
+		})
+
 		It("w/o any rolebindings", func() {
 			init(false, false)
 			Expect(transformer.Transform(transCtx, dag)).Should(BeNil())
@@ -166,7 +220,7 @@ var _ = Describe("object rbac transformer test.", func() {
 		It("w/ cmpd's PolicyRules", func() {
 			init(false, true)
 			Expect(transformer.Transform(transCtx, dag)).Should(BeNil())
-			cmpdRole := factory.BuildComponentRole(synthesizedComp, compDefObj)
+			cmpdRole := factory.BuildRole(synthesizedComp, compDefObj)
 			cmpdRoleBinding := factory.BuildRoleBinding(synthesizedComp, serviceAccountName, &rbacv1.RoleRef{
 				APIGroup: rbacv1.GroupName,
 				Kind:     "Role",

pkg/constant/pattern.go

@@ -83,6 +83,11 @@ func GenerateDefaultServiceAccountName(cmpdName string) string {
 	return fmt.Sprintf("%s-%s", KBLowerPrefix, cmpdName)
 }
 
+// GenerateDefaultRoleName generates default role name for a component.
+func GenerateDefaultRoleName(cmpdName string) string {
+	return fmt.Sprintf("%s-%s", KBLowerPrefix, cmpdName)
+}
+
 // GenerateWorkloadNamePattern generates the workload name pattern
 func GenerateWorkloadNamePattern(clusterName, compName string) string {
 	return fmt.Sprintf("%s-%s", clusterName, compName)

pkg/controller/factory/builder.go

@@ -21,7 +21,6 @@ package factory
 
 import (
 	"encoding/json"
-	"fmt"
 	"path/filepath"
 	"strconv"
 
@@ -353,8 +352,8 @@ func BuildServiceAccount(synthesizedComp *component.SynthesizedComponent, saName
 
 func BuildRoleBinding(synthesizedComp *component.SynthesizedComponent, name string, roleRef *rbacv1.RoleRef, saName string) *rbacv1.RoleBinding {
 	return builder.NewRoleBindingBuilder(synthesizedComp.Namespace, name).
-		AddLabelsInMap(synthesizedComp.StaticLabels).
 		AddLabelsInMap(constant.GetCompLabels(synthesizedComp.ClusterName, synthesizedComp.Name)).
+		AddLabelsInMap(synthesizedComp.StaticLabels).
 		AddAnnotationsInMap(synthesizedComp.StaticAnnotations).
 		SetRoleRef(*roleRef).
 		AddSubjects(rbacv1.Subject{
@@ -365,13 +364,12 @@ func BuildRoleBinding(synthesizedComp *component.SynthesizedComponent, name stri
 		GetObject()
 }
 
-func BuildComponentRole(synthesizedComp *component.SynthesizedComponent, cmpd *appsv1.ComponentDefinition) *rbacv1.Role {
+func BuildRole(synthesizedComp *component.SynthesizedComponent, cmpd *appsv1.ComponentDefinition) *rbacv1.Role {
 	rules := cmpd.Spec.PolicyRules
 	if len(rules) == 0 {
 		return nil
 	}
-	roleName := fmt.Sprintf("%s-%s", constant.KBLowerPrefix, cmpd.Name)
-	return builder.NewRoleBuilder(synthesizedComp.Namespace, roleName).
+	return builder.NewRoleBuilder(synthesizedComp.Namespace, constant.GenerateDefaultRoleName(cmpd.Name)).
 		AddLabelsInMap(constant.GetCompLabels(synthesizedComp.ClusterName, synthesizedComp.Name)).
 		AddLabelsInMap(synthesizedComp.StaticLabels).
 		AddAnnotationsInMap(synthesizedComp.StaticAnnotations).