Skip to content

Commit df3dd55

Browse files
cjc7373cjc7373
authored
chore: rollback service account name when upgrading componentdefinition (#9973)
1 parent ef00ae4 commit df3dd55

6 files changed

Lines changed: 179 additions & 26 deletions

File tree

‎controllers/apps/component/component_controller.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,11 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
168168
&componentReloadSidecarTransformer{Client: r.Client},
169169
// handle restore before workloads transform
170170
&componentRestoreTransformer{Client: r.Client},
171-
// handle the component workload
172-
&componentWorkloadTransformer{Client: r.Client},
173171
// handle RBAC for component workloads
172+
// it should be put before workload transformer, because we modify podSpec's serviceaccount in it
174173
&componentRBACTransformer{},
174+
// handle the component workload
175+
&componentWorkloadTransformer{Client: r.Client},
175176
// handle component postProvision lifecycle action
176177
&componentPostProvisionTransformer{},
177178
// update component status

‎controllers/apps/component/transformer_component_rbac.go‎

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,17 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
2020
package component
2121

2222
import (
23+
"encoding/json"
2324
"fmt"
25+
"hash/fnv"
2426

2527
corev1 "k8s.io/api/core/v1"
2628
rbacv1 "k8s.io/api/rbac/v1"
2729
"k8s.io/apimachinery/pkg/api/equality"
2830
"k8s.io/apimachinery/pkg/api/errors"
2931
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3032
"k8s.io/apimachinery/pkg/types"
33+
"k8s.io/apimachinery/pkg/util/rand"
3134
"k8s.io/klog/v2"
3235
"sigs.k8s.io/controller-runtime/pkg/client"
3336

@@ -50,9 +53,11 @@ type componentRBACTransformer struct{}
5053
var _ graph.Transformer = &componentRBACTransformer{}
5154

5255
const EventReasonRBACManager = "RBACManager"
56+
const EventReasonServiceAccountRollback = "ServiceAccountRollback"
5357

5458
func (t *componentRBACTransformer) Transform(ctx graph.TransformContext, dag *graph.DAG) error {
5559
transCtx, _ := ctx.(*componentTransformContext)
60+
graphCli, _ := transCtx.Client.(model.GraphClient)
5661
synthesizedComp := transCtx.SynthesizeComponent
5762
if isCompDeleting(transCtx.ComponentOrig) {
5863
return nil
@@ -83,17 +88,41 @@ func (t *componentRBACTransformer) Transform(ctx graph.TransformContext, dag *gr
8388
return nil
8489
}
8590

86-
graphCli, _ := transCtx.Client.(model.GraphClient)
87-
8891
var err error
92+
comp := transCtx.Component
93+
lastServiceAccountName := comp.Annotations[constant.ComponentLastServiceAccountNameAnnotationKey]
94+
lastHash := comp.Annotations[constant.ComponentLastServiceAccountRuleHashAnnotationKey]
8995
if serviceAccountName == "" {
96+
rollback, err := needRollbackServiceAccount(transCtx)
97+
if err != nil {
98+
return err
99+
}
100+
90101
serviceAccountName = constant.GenerateDefaultServiceAccountName(synthesizedComp.CompDefName)
102+
if rollback {
103+
transCtx.EventRecorder.Event(comp, corev1.EventTypeNormal, EventReasonServiceAccountRollback, "Change to serviceaccount has been rolled back to prevent pod restart")
104+
serviceAccountName = lastServiceAccountName
105+
}
91106
// if no rolebinding is needed, sa will be created anyway, because other modules may reference it.
92107
sa, err = createOrUpdateServiceAccount(transCtx, serviceAccountName, graphCli, dag)
93108
if err != nil {
94109
return err
95110
}
111+
112+
hash, err := computeServiceAccountRuleHash(transCtx)
113+
if err != nil {
114+
return err
115+
}
116+
117+
if lastServiceAccountName != serviceAccountName || lastHash != hash {
118+
comp.Annotations[constant.ComponentLastServiceAccountNameAnnotationKey] = serviceAccountName
119+
comp.Annotations[constant.ComponentLastServiceAccountRuleHashAnnotationKey] = hash
120+
graphCli.Update(dag, transCtx.ComponentOrig, transCtx.Component)
121+
}
96122
}
123+
124+
synthesizedComp.PodSpec.ServiceAccountName = serviceAccountName
125+
97126
role, err := createOrUpdateRole(transCtx, graphCli, dag)
98127
if err != nil {
99128
return err
@@ -119,6 +148,32 @@ func (t *componentRBACTransformer) Transform(ctx graph.TransformContext, dag *gr
119148
return nil
120149
}
121150

151+
func computeServiceAccountRuleHash(transCtx *componentTransformContext) (string, error) {
152+
hash := fnv.New32a()
153+
data, err := json.Marshal(transCtx.SynthesizeComponent.PolicyRules)
154+
if err != nil {
155+
return "", err
156+
}
157+
hash.Write(data)
158+
enabled := transCtx.SynthesizeComponent.LifecycleActions != nil
159+
fmt.Fprint(hash, enabled)
160+
return rand.SafeEncodeString(fmt.Sprintf("%d", hash.Sum32())), nil
161+
}
162+
163+
func needRollbackServiceAccount(transCtx *componentTransformContext) (rollback bool, err error) {
164+
hash, err := computeServiceAccountRuleHash(transCtx)
165+
if err != nil {
166+
return false, err
167+
}
168+
169+
lastHash, ok := transCtx.Component.Annotations[constant.ComponentLastServiceAccountRuleHashAnnotationKey]
170+
if !ok {
171+
return false, nil
172+
}
173+
174+
return hash == lastHash, nil
175+
}
176+
122177
func labelAndAnnotationEqual(old, new metav1.Object) bool {
123178
// exclude component labels, since they are different for each component
124179
compLabels := constant.GetCompLabels("", "")

‎controllers/apps/component/transformer_component_rbac_test.go‎

Lines changed: 116 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ var _ = Describe("object rbac transformer test.", func() {
189189
// sa should be created
190190
serviceAccount := factory.BuildServiceAccount(synthesizedComp, serviceAccountName)
191191

192-
dagExpected := mockDAG(graphCli, compObj)
192+
dagExpected := mockDAGWithUpdate(graphCli, compObj)
193193
graphCli.Create(dagExpected, serviceAccount)
194194

195195
Expect(dag.Equals(dagExpected, model.DefaultLess)).Should(BeTrue())
@@ -204,7 +204,7 @@ var _ = Describe("object rbac transformer test.", func() {
204204
Name: constant.RBACRoleName,
205205
}, serviceAccountName)
206206
serviceAccount := factory.BuildServiceAccount(synthesizedComp, serviceAccountName)
207-
dagExpected := mockDAG(graphCli, compObj)
207+
dagExpected := mockDAGWithUpdate(graphCli, compObj)
208208
graphCli.Create(dagExpected, serviceAccount)
209209
graphCli.Create(dagExpected, clusterPodRoleBinding)
210210
graphCli.DependOn(dagExpected, clusterPodRoleBinding, serviceAccount)
@@ -225,7 +225,7 @@ var _ = Describe("object rbac transformer test.", func() {
225225
Name: cmpdRole.Name,
226226
}, serviceAccountName)
227227
serviceAccount := factory.BuildServiceAccount(synthesizedComp, serviceAccountName)
228-
dagExpected := mockDAG(graphCli, compObj)
228+
dagExpected := mockDAGWithUpdate(graphCli, compObj)
229229
graphCli.Create(dagExpected, serviceAccount)
230230
graphCli.Create(dagExpected, cmpdRoleBinding)
231231
graphCli.Create(dagExpected, cmpdRole)
@@ -242,6 +242,111 @@ var _ = Describe("object rbac transformer test.", func() {
242242
Expect(reflect.DeepEqual(rb.Subjects, cmpdRoleBinding.Subjects)).To(BeTrue())
243243
Expect(reflect.DeepEqual(rb.RoleRef, cmpdRoleBinding.RoleRef)).To(BeTrue())
244244
})
245+
246+
Context("rollback behavior", func() {
247+
It("tests needRollbackServiceAccount", func() {
248+
init(true, false)
249+
ctx := transCtx.(*componentTransformContext)
250+
251+
By("create another cmpd")
252+
anotherTpl := testapps.NewComponentDefinitionFactory(compDefName).
253+
WithRandomName().
254+
SetDefaultSpec().
255+
Create(&testCtx).
256+
GetObject()
257+
hash, err := computeServiceAccountRuleHash(ctx)
258+
Expect(err).ShouldNot(HaveOccurred())
259+
260+
// Case: No label, should return false
261+
needRollback, err := needRollbackServiceAccount(ctx)
262+
Expect(err).Should(BeNil())
263+
Expect(needRollback).Should(BeFalse())
264+
265+
// Case: With same cmpd
266+
ctx.Component.Annotations[constant.ComponentLastServiceAccountRuleHashAnnotationKey] = hash
267+
ctx.Component.Annotations[constant.ComponentLastServiceAccountNameAnnotationKey] = constant.GenerateDefaultServiceAccountName(synthesizedComp.CompDefName)
268+
needRollback, err = needRollbackServiceAccount(ctx)
269+
Expect(err).Should(BeNil())
270+
Expect(needRollback).Should(BeTrue())
271+
272+
// Case: Different cmpd, same spec
273+
another := anotherTpl.DeepCopy()
274+
ctx.SynthesizeComponent, err = component.BuildSynthesizedComponent(ctx, k8sClient, another, compObj)
275+
Expect(err).Should(Succeed())
276+
needRollback, err = needRollbackServiceAccount(ctx)
277+
Expect(err).Should(BeNil())
278+
Expect(needRollback).Should(BeTrue())
279+
280+
// Case: Different cmpd, different policy rules
281+
another = anotherTpl.DeepCopy()
282+
another.Spec.PolicyRules = []rbacv1.PolicyRule{
283+
{
284+
APIGroups: []string{""},
285+
Resources: []string{"pods"},
286+
Verbs: []string{"get"},
287+
},
288+
}
289+
ctx.SynthesizeComponent, err = component.BuildSynthesizedComponent(ctx, k8sClient, another, compObj)
290+
Expect(err).Should(Succeed())
291+
needRollback, err = needRollbackServiceAccount(ctx)
292+
Expect(err).Should(BeNil())
293+
Expect(needRollback).Should(BeFalse())
294+
295+
// Case: Different cmpd, different lifecycle action
296+
another = anotherTpl.DeepCopy()
297+
another.Spec.PolicyRules = nil
298+
another.Spec.LifecycleActions = nil
299+
ctx.SynthesizeComponent, err = component.BuildSynthesizedComponent(ctx, k8sClient, another, compObj)
300+
Expect(err).Should(Succeed())
301+
needRollback, err = needRollbackServiceAccount(ctx)
302+
Expect(err).Should(BeNil())
303+
Expect(needRollback).Should(BeFalse())
304+
})
305+
306+
mockDAGWithUpdate := func(graphCli model.GraphClient, comp *appsv1.Component) *graph.DAG {
307+
d := graph.NewDAG()
308+
graphCli.Root(d, comp, comp, model.ActionUpdatePtr())
309+
its := &workloads.InstanceSet{}
310+
graphCli.Create(d, its)
311+
return d
312+
}
313+
314+
less := func(v1, v2 graph.Vertex) bool {
315+
o1, ok1 := v1.(*model.ObjectVertex)
316+
o2, ok2 := v2.(*model.ObjectVertex)
317+
if !ok1 || !ok2 {
318+
return false
319+
}
320+
if o1.String() != o2.String() {
321+
return o1.String() < o2.String()
322+
}
323+
if !reflect.DeepEqual(o1.Obj.GetLabels(), o2.Obj.GetLabels()) {
324+
return true
325+
}
326+
return !reflect.DeepEqual(o1.Obj.GetAnnotations(), o2.Obj.GetAnnotations())
327+
}
328+
329+
It("adds labels for an old component", func() {
330+
init(false, false)
331+
// mock a running workload
332+
ctx := transCtx.(*componentTransformContext)
333+
ctx.RunningWorkload = &workloads.InstanceSet{}
334+
expectedComp := compObj.DeepCopy()
335+
Expect(transformer.Transform(transCtx, dag)).Should(BeNil())
336+
// sa should be created
337+
oldSAName := constant.GenerateDefaultServiceAccountName(compDefObj.Name)
338+
serviceAccount := factory.BuildServiceAccount(synthesizedComp, oldSAName)
339+
340+
hash, err := computeServiceAccountRuleHash(ctx)
341+
Expect(err).ShouldNot(HaveOccurred())
342+
expectedComp.Annotations[constant.ComponentLastServiceAccountRuleHashAnnotationKey] = hash
343+
expectedComp.Annotations[constant.ComponentLastServiceAccountNameAnnotationKey] = constant.GenerateDefaultServiceAccountName(synthesizedComp.CompDefName)
344+
dagExpected := mockDAGWithUpdate(graphCli, expectedComp)
345+
graphCli.Create(dagExpected, serviceAccount)
346+
347+
Expect(dag.Equals(dagExpected, less)).Should(BeTrue())
348+
})
349+
})
245350
})
246351
})
247352

@@ -252,3 +357,11 @@ func mockDAG(graphCli model.GraphClient, comp *appsv1.Component) *graph.DAG {
252357
graphCli.Create(d, its)
253358
return d
254359
}
360+
361+
func mockDAGWithUpdate(graphCli model.GraphClient, comp *appsv1.Component) *graph.DAG {
362+
d := graph.NewDAG()
363+
graphCli.Root(d, comp, comp, model.ActionUpdatePtr())
364+
its := &workloads.InstanceSet{}
365+
graphCli.Create(d, its)
366+
return d
367+
}

‎pkg/constant/annotations.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ const (
4949

5050
// NodeSelectorOnceAnnotationKey adds nodeSelector in podSpec for one pod exactly once
5151
NodeSelectorOnceAnnotationKey = "workloads.kubeblocks.io/node-selector-once"
52+
53+
ComponentLastServiceAccountNameAnnotationKey = "component.kubeblocks.io/last-service-account-name"
54+
ComponentLastServiceAccountRuleHashAnnotationKey = "component.kubeblocks.io/last-service-account-rule-hash"
5255
)
5356

5457
const (

‎pkg/controller/component/synthesize_component.go‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import (
3737
"github.com/apecloud/kubeblocks/pkg/controller/scheduling"
3838
intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil"
3939
"github.com/apecloud/kubeblocks/pkg/generics"
40-
viper "github.com/apecloud/kubeblocks/pkg/viperx"
4140
)
4241

4342
var (
@@ -107,7 +106,6 @@ func BuildSynthesizedComponent(ctx context.Context, cli client.Reader,
107106
Replicas: comp.Spec.Replicas,
108107
Resources: comp.Spec.Resources,
109108
TLSConfig: comp.Spec.TLSConfig,
110-
ServiceAccountName: comp.Spec.ServiceAccountName,
111109
Instances: comp.Spec.Instances,
112110
OfflineInstances: comp.Spec.OfflineInstances,
113111
DisableExporter: comp.Spec.DisableExporter,
@@ -142,9 +140,6 @@ func BuildSynthesizedComponent(ctx context.Context, cli client.Reader,
142140
// override componentService
143141
overrideComponentServices(synthesizeComp, comp)
144142

145-
// build serviceAccountName
146-
buildServiceAccountName(synthesizeComp)
147-
148143
// build runtimeClassName
149144
buildRuntimeClassName(synthesizeComp, comp)
150145

@@ -480,19 +475,6 @@ func synthesizeFileTemplate(comp *appsv1.Component, tpl appsv1.ComponentFileTemp
480475
return stpl
481476
}
482477

483-
// buildServiceAccountName builds serviceAccountName for component and podSpec.
484-
func buildServiceAccountName(synthesizeComp *SynthesizedComponent) {
485-
if synthesizeComp.ServiceAccountName != "" {
486-
synthesizeComp.PodSpec.ServiceAccountName = synthesizeComp.ServiceAccountName
487-
return
488-
}
489-
if !viper.GetBool(constant.EnableRBACManager) {
490-
return
491-
}
492-
synthesizeComp.ServiceAccountName = constant.GenerateDefaultServiceAccountName(synthesizeComp.CompDefName)
493-
synthesizeComp.PodSpec.ServiceAccountName = synthesizeComp.ServiceAccountName
494-
}
495-
496478
func buildRuntimeClassName(synthesizeComp *SynthesizedComponent, comp *appsv1.Component) {
497479
if comp.Spec.RuntimeClassName == nil {
498480
return

‎pkg/controller/component/type.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ type SynthesizedComponent struct {
4949
FileTemplates []SynthesizedFileTemplate
5050
LogConfigs []kbappsv1.LogConfig `json:"logConfigs,omitempty"`
5151
TLSConfig *kbappsv1.TLSConfig `json:"tlsConfig"`
52-
ServiceAccountName string `json:"serviceAccountName,omitempty"`
5352
ServiceReferences map[string]*kbappsv1.ServiceDescriptor `json:"serviceReferences,omitempty"`
5453
Labels map[string]string `json:"labels,omitempty"`
5554
StaticLabels map[string]string // labels defined by the component definition

0 commit comments

Comments
 (0)