chore: rollback service account name when upgrading componentdefinition

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

@@ -21,6 +21,7 @@ package component
 
 import (
 	"fmt"
+	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -45,11 +46,14 @@ import (
 )
 
 // componentRBACTransformer puts the RBAC objects at the beginning of the DAG
+// Note: rbac objects created in this transformer are not necessarily used in workload objects,
+// as when updating componentdefition, old serviceaccount may be retained to prevent pod restart.
 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)
@@ -77,6 +81,7 @@ func (t *componentRBACTransformer) Transform(ctx graph.TransformContext, dag *gr
 			}
 			return err
 		}
+		synthesizedComp.PodSpec.ServiceAccountName = serviceAccountName
 	}
 	if !viper.GetBool(constant.EnableRBACManager) {
 		transCtx.EventRecorder.Event(transCtx.Component, corev1.EventTypeNormal, EventReasonRBACManager, "RBAC manager is disabled")
@@ -88,11 +93,22 @@ func (t *componentRBACTransformer) Transform(ctx graph.TransformContext, dag *gr
 	var err error
 	if serviceAccountName == "" {
 		serviceAccountName = constant.GenerateDefaultServiceAccountName(synthesizedComp.CompDefName)
+		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
+		}
 		// if no rolebinding is needed, sa will be created anyway, because other modules may reference it.
 		sa, err = createOrUpdateServiceAccount(transCtx, serviceAccountName, graphCli, dag)
 		if err != nil {
 			return err
 		}
+		synthesizedComp.PodSpec.ServiceAccountName = serviceAccountName
+		transCtx.Component.Labels[constant.ComponentLastServiceAccountNameLabelKey] = serviceAccountName
 	}
 	role, err := createOrUpdateRole(transCtx, graphCli, dag)
 	if err != nil {
@@ -123,6 +139,32 @@ 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
+	}
+
+	// this logic is in line with GenerateDefaultServiceAccountName
+	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, client.IgnoreNotFound(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,66 @@ var _ = Describe("object rbac transformer test.", func() {
 			Expect(reflect.DeepEqual(rb.RoleRef, cmpdRoleBinding.RoleRef)).To(BeTrue())
 		})
 	})
+
+	Context("tests serviceaccount rollback", func() {
+		It("tests needRollbackServiceAccount", func() {
+			init(true, false)
+			ctx := transCtx.(*componentTransformContext)
+
+			By("create another cmpd")
+			anotherCompDef := testapps.NewComponentDefinitionFactory(compDefName).
+				WithRandomName().
+				SetDefaultSpec().
+				Create(&testCtx).
+				GetObject()
+
+			// Case: No label, should return false
+			needRollback, err := needRollbackServiceAccount(ctx)
+			Expect(err).Should(BeNil())
+			Expect(needRollback).Should(BeFalse())
+
+			// Case: 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: with a non-exist cmpd
+			ctx.Component.Labels[constant.ComponentLastServiceAccountNameLabelKey] = "kb-non-existent"
+			needRollback, err = needRollbackServiceAccount(ctx)
+			Expect(err).Should(BeNil())
+			Expect(needRollback).Should(BeFalse())
+
+			// Case: 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: 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: 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())
+		})
+	})
 })
 
 func mockDAG(graphCli model.GraphClient, comp *appsv1.Component) *graph.DAG {

pkg/constant/labels.go

@@ -33,13 +33,14 @@ const (
 
 // labels defined by KubeBlocks
 const (
-	ClusterDefLabelKey            = "clusterdefinition.kubeblocks.io/name"
-	ShardingDefLabelKey           = "shardingdefinition.kubeblocks.io/name"
-	ComponentDefinitionLabelKey   = "componentdefinition.kubeblocks.io/name"
-	ComponentVersionLabelKey      = "componentversion.kubeblocks.io/name"
-	SidecarDefLabelKey            = "sidecardefinition.kubeblocks.io/name"
-	ServiceDescriptorNameLabelKey = "servicedescriptor.kubeblocks.io/name"
-	AddonNameLabelKey             = "extensions.kubeblocks.io/addon-name"
+	ClusterDefLabelKey                      = "clusterdefinition.kubeblocks.io/name"
+	ShardingDefLabelKey                     = "shardingdefinition.kubeblocks.io/name"
+	ComponentDefinitionLabelKey             = "componentdefinition.kubeblocks.io/name"
+	ComponentVersionLabelKey                = "componentversion.kubeblocks.io/name"
+	SidecarDefLabelKey                      = "sidecardefinition.kubeblocks.io/name"
+	ServiceDescriptorNameLabelKey           = "servicedescriptor.kubeblocks.io/name"
+	AddonNameLabelKey                       = "extensions.kubeblocks.io/addon-name"
+	ComponentLastServiceAccountNameLabelKey = "component.kubeblocks.io/last-service-account-name"
 
 	KBAppShardingNameLabelKey       = "apps.kubeblocks.io/sharding-name"
 	KBAppShardTemplateLabelKey      = "apps.kubeblocks.io/shard-template"

pkg/controller/component/synthesize_component.go

@@ -37,7 +37,6 @@ import (
 	"github.com/apecloud/kubeblocks/pkg/controller/scheduling"
 	intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil"
 	"github.com/apecloud/kubeblocks/pkg/generics"
-	viper "github.com/apecloud/kubeblocks/pkg/viperx"
 )
 
 var (
@@ -107,7 +106,6 @@ func BuildSynthesizedComponent(ctx context.Context, cli client.Reader,
 		Replicas:                         comp.Spec.Replicas,
 		Resources:                        comp.Spec.Resources,
 		TLSConfig:                        comp.Spec.TLSConfig,
-		ServiceAccountName:               comp.Spec.ServiceAccountName,
 		Instances:                        comp.Spec.Instances,
 		FlatInstanceOrdinal:              comp.Spec.FlatInstanceOrdinal,
 		InstanceImages:                   make(map[string]map[string]string),
@@ -146,9 +144,6 @@ func BuildSynthesizedComponent(ctx context.Context, cli client.Reader,
 	// override componentService
 	overrideComponentServices(synthesizeComp, comp)
 
-	// build serviceAccountName
-	buildServiceAccountName(synthesizeComp)
-
 	// build runtimeClassName
 	buildRuntimeClassName(synthesizeComp, comp)
 
@@ -510,19 +505,6 @@ func synthesizeFileTemplate(comp *appsv1.Component, tpl appsv1.ComponentFileTemp
 	return stpl
 }
 
-// buildServiceAccountName builds serviceAccountName for component and podSpec.
-func buildServiceAccountName(synthesizeComp *SynthesizedComponent) {
-	if synthesizeComp.ServiceAccountName != "" {
-		synthesizeComp.PodSpec.ServiceAccountName = synthesizeComp.ServiceAccountName
-		return
-	}
-	if !viper.GetBool(constant.EnableRBACManager) {
-		return
-	}
-	synthesizeComp.ServiceAccountName = constant.GenerateDefaultServiceAccountName(synthesizeComp.CompDefName)
-	synthesizeComp.PodSpec.ServiceAccountName = synthesizeComp.ServiceAccountName
-}
-
 func buildRuntimeClassName(synthesizeComp *SynthesizedComponent, comp *appsv1.Component) {
 	if comp.Spec.RuntimeClassName == nil {
 		return

pkg/controller/component/type.go

@@ -49,7 +49,6 @@ type SynthesizedComponent struct {
 	FileTemplates                    []SynthesizedFileTemplate
 	LogConfigs                       []kbappsv1.LogConfig                   `json:"logConfigs,omitempty"`
 	TLSConfig                        *kbappsv1.TLSConfig                    `json:"tlsConfig"`
-	ServiceAccountName               string                                 `json:"serviceAccountName,omitempty"`
 	ServiceReferences                map[string]*kbappsv1.ServiceDescriptor `json:"serviceReferences,omitempty"`
 	Labels                           map[string]string                      `json:"labels,omitempty"`
 	StaticLabels                     map[string]string                      // labels defined by the component definition