diff --git a/controllers/apps/component/transformer_component_rbac.go b/controllers/apps/component/transformer_component_rbac.go index 844a854b0d6..9ccee3595cf 100644 --- a/controllers/apps/component/transformer_component_rbac.go +++ b/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,7 +162,7 @@ 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 } @@ -163,7 +170,8 @@ func createOrUpdateRole( 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) }) } @@ -171,7 +179,9 @@ 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) diff --git a/controllers/apps/component/transformer_component_rbac_test.go b/controllers/apps/component/transformer_component_rbac_test.go index 194e79dc2bc..cfab0daad3a 100644 --- a/controllers/apps/component/transformer_component_rbac_test.go +++ b/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", diff --git a/pkg/constant/pattern.go b/pkg/constant/pattern.go index 2d5ace4a5ad..e31d3c0604c 100644 --- a/pkg/constant/pattern.go +++ b/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) diff --git a/pkg/controller/factory/builder.go b/pkg/controller/factory/builder.go index 1c21461536b..c54d6fe2de3 100644 --- a/pkg/controller/factory/builder.go +++ b/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).