add tests

Author: cjc7373

controllers/apps/component/component_controller.go

@@ -164,10 +164,10 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			&componentReloadSidecarTransformer{Client: r.Client},
 			// handle restore before workloads transform
 			&componentRestoreTransformer{Client: r.Client},
+			// handle RBAC for component workloads, it should be put before workload transformer
+			&componentRBACTransformer{},
 			// handle the component workload
 			&componentWorkloadTransformer{Client: r.Client},
-			// handle RBAC for component workloads
-			&componentRBACTransformer{},
 			// handle component postProvision lifecycle action
 			&componentPostProvisionTransformer{},
 			// update component status

controllers/apps/component/transformer_component_rbac.go

@@ -53,6 +53,7 @@ type componentRBACTransformer struct{}
 var _ graph.Transformer = &componentRBACTransformer{}
 
 const EventReasonRBACManager = "RBACManager"
+const EventReasonServiceAccountRollback = "ServiceAccountRollback"
 
 func (t *componentRBACTransformer) Transform(ctx graph.TransformContext, dag *graph.DAG) error {
 	transCtx, _ := ctx.(*componentTransformContext)
@@ -89,35 +90,15 @@ func (t *componentRBACTransformer) Transform(ctx graph.TransformContext, dag *gr
 
 	graphCli, _ := transCtx.Client.(model.GraphClient)
 
-	needRollbackServiceAccount := func() (bool, error) {
-		lastName, ok := transCtx.Component.Labels[constant.ComponentLastServiceAccountNameLabelKey]
-		if !ok {
-			return false, nil
-		}
-
-		lastCmpdName := strings.Join(strings.Split(lastName, "-")[1:], "-")
-		lastCmpd, err := component.GetCompDefByName(transCtx.Context, transCtx.Client, lastCmpdName)
-		if err != nil {
-			return false, err
-		}
-
-		curLifecycleActionEnabled := synthesizedComp.LifecycleActions != nil
-		lastLifecycleActionEnabled := lastCmpd.Spec.LifecycleActions != nil
-		if equality.Semantic.DeepEqual(synthesizedComp.PolicyRules, lastCmpd.Spec.PolicyRules) &&
-			curLifecycleActionEnabled == lastLifecycleActionEnabled {
-			return true, nil
-		}
-		return false, nil
-	}
-
 	var err error
 	if serviceAccountName == "" {
 		serviceAccountName = constant.GenerateDefaultServiceAccountName(synthesizedComp.CompDefName)
-		rollback, err := needRollbackServiceAccount()
+		rollback, err := needRollbackServiceAccount(transCtx)
 		if err != nil {
 			return err
 		}
 		if rollback {
+			transCtx.EventRecorder.Event(transCtx.Component, corev1.EventTypeNormal, EventReasonServiceAccountRollback, "Change to serviceaccount has rolled back to prevent pod restart")
 			// don't change anything, just return
 			return nil
 		}
@@ -158,6 +139,31 @@ func (t *componentRBACTransformer) Transform(ctx graph.TransformContext, dag *gr
 	return nil
 }
 
+func needRollbackServiceAccount(transCtx *componentTransformContext) (bool, error) {
+	lastName, ok := transCtx.Component.Labels[constant.ComponentLastServiceAccountNameLabelKey]
+	if !ok {
+		return false, nil
+	}
+
+	lastCmpdName := strings.Join(strings.Split(lastName, "-")[1:], "-")
+	if lastCmpdName == transCtx.CompDef.Name {
+		return false, nil
+	}
+
+	lastCmpd, err := component.GetCompDefByName(transCtx.Context, transCtx.Client, lastCmpdName)
+	if err != nil {
+		return false, err
+	}
+
+	curLifecycleActionEnabled := transCtx.SynthesizeComponent.LifecycleActions != nil
+	lastLifecycleActionEnabled := lastCmpd.Spec.LifecycleActions != nil
+	if equality.Semantic.DeepEqual(transCtx.SynthesizeComponent.PolicyRules, lastCmpd.Spec.PolicyRules) &&
+		curLifecycleActionEnabled == lastLifecycleActionEnabled {
+		return true, nil
+	}
+	return false, nil
+}
+
 func (t *componentRBACTransformer) rbacInstanceAssistantObjects(graphCli model.GraphClient, dag *graph.DAG, objs []client.Object) {
 	itsList := graphCli.FindAll(dag, &workloads.InstanceSet{})
 	for _, itsObj := range itsList {

controllers/apps/component/transformer_component_rbac_test.go

@@ -243,6 +243,79 @@ var _ = Describe("object rbac transformer test.", func() {
 			Expect(reflect.DeepEqual(rb.RoleRef, cmpdRoleBinding.RoleRef)).To(BeTrue())
 		})
 	})
+
+	Context("tests serviceaccount rollback", func() {
+		It("tests needRollbackServiceAccount basic cases", func() {
+			init(true, false)
+			ctx := transCtx.(*componentTransformContext)
+
+			By("create another cmpd")
+			anotherCompDef := testapps.NewComponentDefinitionFactory(compDefName).
+				WithRandomName().
+				SetDefaultSpec().
+				Create(&testCtx).
+				GetObject()
+
+			// Case 1: No label, should return false
+			needRollback, err := needRollbackServiceAccount(ctx)
+			Expect(err).Should(BeNil())
+			Expect(needRollback).Should(BeFalse())
+
+			// Case 2: With label but component definitions are the same
+			ctx.Component.Labels[constant.ComponentLastServiceAccountNameLabelKey] = "kb-" + compDefObj.Name
+			needRollback, err = needRollbackServiceAccount(ctx)
+			Expect(err).Should(BeNil())
+			Expect(needRollback).Should(BeFalse())
+
+			// Case 3: Different cmpd, same spec
+			ctx.Component.Labels[constant.ComponentLastServiceAccountNameLabelKey] = "kb-" + anotherCompDef.Name
+			needRollback, err = needRollbackServiceAccount(ctx)
+			Expect(err).Should(BeNil())
+			Expect(needRollback).Should(BeTrue())
+
+			// Case 4: Different cmpd, different policy rules
+			Expect(testapps.ChangeObj(&testCtx, anotherCompDef, func(cmpd *appsv1.ComponentDefinition) {
+				cmpd.Spec.PolicyRules = []rbacv1.PolicyRule{
+					{
+						APIGroups: []string{""},
+						Resources: []string{"pods"},
+						Verbs:     []string{"get"},
+					},
+				}
+			})).Should(Succeed())
+			needRollback, err = needRollbackServiceAccount(ctx)
+			Expect(err).Should(BeNil())
+			Expect(needRollback).Should(BeFalse())
+
+			// Case 5: Different cmpd, different lifecycle action
+			Expect(testapps.ChangeObj(&testCtx, anotherCompDef, func(cmpd *appsv1.ComponentDefinition) {
+				cmpd.Spec.PolicyRules = nil
+				cmpd.Spec.LifecycleActions = nil
+			})).Should(Succeed())
+			needRollback, err = needRollbackServiceAccount(ctx)
+			Expect(err).Should(BeNil())
+			Expect(needRollback).Should(BeFalse())
+		})
+
+		It("tests needRollbackServiceAccount error cases", func() {
+			init(false, false)
+			ctx := transCtx.(*componentTransformContext)
+
+			// Case 1: Invalid label format
+			ctx.Component.Labels = map[string]string{
+				constant.ComponentLastServiceAccountNameLabelKey: "invalid-format",
+			}
+			_, err := needRollbackServiceAccount(ctx)
+			Expect(err).Should(HaveOccurred())
+
+			// Case 2: Component definition not found
+			ctx.Component.Labels = map[string]string{
+				constant.ComponentLastServiceAccountNameLabelKey: "kb-non-existent",
+			}
+			_, err = needRollbackServiceAccount(ctx)
+			Expect(err).Should(HaveOccurred())
+		})
+	})
 })
 
 func mockDAG(graphCli model.GraphClient, comp *appsv1.Component) *graph.DAG {