From cf9d59bbfa4e51cdb3b61270f8648f8f7767bb08 Mon Sep 17 00:00:00 2001 From: cjc7373 Date: Mon, 20 Jan 2025 17:44:10 +0800 Subject: [PATCH] feat: implement cmpd's PolicyRules (#8328) --- apis/apps/v1/cluster_types.go | 20 +- apis/apps/v1/component_types.go | 19 +- apis/apps/v1/componentdefinition_types.go | 2 - .../bases/apps.kubeblocks.io_clusters.yaml | 38 ++- ...ps.kubeblocks.io_componentdefinitions.yaml | 3 - .../bases/apps.kubeblocks.io_components.yaml | 18 +- .../cluster/transformer_cluster_deletion.go | 37 +-- .../apps/component/component_controller.go | 4 +- .../component/component_controller_test.go | 140 +++++++--- .../transformer_component_deletion.go | 69 +++++ .../component/transformer_component_rbac.go | 245 ++++++++++-------- .../transformer_component_rbac_test.go | 173 ++++++++++--- .../component/transformer_component_utils.go | 33 +-- .../crds/apps.kubeblocks.io_clusters.yaml | 38 ++- ...ps.kubeblocks.io_componentdefinitions.yaml | 3 - .../crds/apps.kubeblocks.io_components.yaml | 18 +- .../rbac/cluster_pod_required_role.yaml | 88 +------ docs/developer_docs/api-reference/cluster.md | 51 ++-- pkg/constant/pattern.go | 11 +- pkg/controller/builder/builder_role.go | 39 +++ pkg/controller/builder/builder_role_test.go | 58 +++++ .../component/synthesize_component.go | 6 +- pkg/controller/factory/builder.go | 23 +- pkg/controller/factory/builder_test.go | 7 +- pkg/controller/model/graph_client.go | 5 +- pkg/generics/type.go | 7 + pkg/operations/custom/action_workload.go | 2 +- 27 files changed, 692 insertions(+), 465 deletions(-) create mode 100644 pkg/controller/builder/builder_role.go create mode 100644 pkg/controller/builder/builder_role_test.go diff --git a/apis/apps/v1/cluster_types.go b/apis/apps/v1/cluster_types.go index 49c15b4e0ba..f5455f590a6 100644 --- a/apis/apps/v1/cluster_types.go +++ b/apis/apps/v1/cluster_types.go @@ -404,17 +404,15 @@ type ClusterComponentSpec struct { // This ServiceAccount is used to grant necessary permissions for the Component's Pods to interact // with other Kubernetes resources, such as modifying Pod labels or sending events. // - // Defaults: - // To perform certain operational tasks, agent sidecars running in Pods require specific RBAC permissions. - // The service account will be bound to a default role named "kubeblocks-cluster-pod-role" which is installed together with KubeBlocks. - // If not specified, KubeBlocks automatically assigns a default ServiceAccount named "kb-{cluster.name}" - // - // Future Changes: - // Future versions might change the default ServiceAccount creation strategy to one per Component, - // potentially revising the naming to "kb-{cluster.name}-{component.name}". - // - // Users can override the automatic ServiceAccount assignment by explicitly setting the name of - // an existed ServiceAccount in this field. + // If not specified, KubeBlocks automatically creates a default ServiceAccount named + // "kb-{componentdefinition.name}", bound to a role with rules defined in ComponentDefinition's + // `policyRules` field. If needed (currently this means if any lifecycleAction is enabled), + // it will also be bound to a default role named + // "kubeblocks-cluster-pod-role", which is installed together with KubeBlocks. + // If multiple components use the same ComponentDefinition, they will share one ServiceAccount. + // + // If the field is not empty, the specified ServiceAccount will be used, and KubeBlocks will not + // create a ServiceAccount. But KubeBlocks does create RoleBindings for the specified ServiceAccount. // // +optional ServiceAccountName string `json:"serviceAccountName,omitempty"` diff --git a/apis/apps/v1/component_types.go b/apis/apps/v1/component_types.go index cd6aec7d54d..bdb8c4ea52e 100644 --- a/apis/apps/v1/component_types.go +++ b/apis/apps/v1/component_types.go @@ -175,16 +175,15 @@ type ComponentSpec struct { // This ServiceAccount is used to grant necessary permissions for the Component's Pods to interact // with other Kubernetes resources, such as modifying Pod labels or sending events. // - // Defaults: - // If not specified, KubeBlocks automatically assigns a default ServiceAccount named "kb-{cluster.name}", - // bound to a default role defined during KubeBlocks installation. - // - // Future Changes: - // Future versions might change the default ServiceAccount creation strategy to one per Component, - // potentially revising the naming to "kb-{cluster.name}-{component.name}". - // - // Users can override the automatic ServiceAccount assignment by explicitly setting the name of - // an existed ServiceAccount in this field. + // If not specified, KubeBlocks automatically creates a default ServiceAccount named + // "kb-{componentdefinition.name}", bound to a role with rules defined in ComponentDefinition's + // `policyRules` field. If needed (currently this means if any lifecycleAction is enabled), + // it will also be bound to a default role named + // "kubeblocks-cluster-pod-role", which is installed together with KubeBlocks. + // If multiple components use the same ComponentDefinition, they will share one ServiceAccount. + // + // If the field is not empty, the specified ServiceAccount will be used, and KubeBlocks will not + // create a ServiceAccount. But KubeBlocks does create RoleBindings for the specified ServiceAccount. // // +optional ServiceAccountName string `json:"serviceAccountName,omitempty"` diff --git a/apis/apps/v1/componentdefinition_types.go b/apis/apps/v1/componentdefinition_types.go index 90aabaf6dca..38d00c9aa20 100644 --- a/apis/apps/v1/componentdefinition_types.go +++ b/apis/apps/v1/componentdefinition_types.go @@ -489,8 +489,6 @@ type ComponentDefinitionSpec struct { // for the Component based on the specified policy rules. // This ensures that the Pods in the Component has appropriate permissions to function. // - // Note: This field is currently non-functional and is reserved for future implementation. - // // This field is immutable. // // +optional diff --git a/config/crd/bases/apps.kubeblocks.io_clusters.yaml b/config/crd/bases/apps.kubeblocks.io_clusters.yaml index 7ac44eebc41..4eef6fc3bfb 100644 --- a/config/crd/bases/apps.kubeblocks.io_clusters.yaml +++ b/config/crd/bases/apps.kubeblocks.io_clusters.yaml @@ -5077,19 +5077,16 @@ spec: with other Kubernetes resources, such as modifying Pod labels or sending events. - Defaults: - To perform certain operational tasks, agent sidecars running in Pods require specific RBAC permissions. - The service account will be bound to a default role named "kubeblocks-cluster-pod-role" which is installed together with KubeBlocks. - If not specified, KubeBlocks automatically assigns a default ServiceAccount named "kb-{cluster.name}" + If not specified, KubeBlocks automatically creates a default ServiceAccount named + "kb-{componentdefinition.name}", bound to a role with rules defined in ComponentDefinition's + `policyRules` field. If needed (currently this means if any lifecycleAction is enabled), + it will also be bound to a default role named + "kubeblocks-cluster-pod-role", which is installed together with KubeBlocks. + If multiple components use the same ComponentDefinition, they will share one ServiceAccount. - Future Changes: - Future versions might change the default ServiceAccount creation strategy to one per Component, - potentially revising the naming to "kb-{cluster.name}-{component.name}". - - - Users can override the automatic ServiceAccount assignment by explicitly setting the name of - an existed ServiceAccount in this field. + If the field is not empty, the specified ServiceAccount will be used, and KubeBlocks will not + create a ServiceAccount. But KubeBlocks does create RoleBindings for the specified ServiceAccount. type: string serviceRefs: description: |- @@ -13827,19 +13824,16 @@ spec: with other Kubernetes resources, such as modifying Pod labels or sending events. - Defaults: - To perform certain operational tasks, agent sidecars running in Pods require specific RBAC permissions. - The service account will be bound to a default role named "kubeblocks-cluster-pod-role" which is installed together with KubeBlocks. - If not specified, KubeBlocks automatically assigns a default ServiceAccount named "kb-{cluster.name}" + If not specified, KubeBlocks automatically creates a default ServiceAccount named + "kb-{componentdefinition.name}", bound to a role with rules defined in ComponentDefinition's + `policyRules` field. If needed (currently this means if any lifecycleAction is enabled), + it will also be bound to a default role named + "kubeblocks-cluster-pod-role", which is installed together with KubeBlocks. + If multiple components use the same ComponentDefinition, they will share one ServiceAccount. - Future Changes: - Future versions might change the default ServiceAccount creation strategy to one per Component, - potentially revising the naming to "kb-{cluster.name}-{component.name}". - - - Users can override the automatic ServiceAccount assignment by explicitly setting the name of - an existed ServiceAccount in this field. + If the field is not empty, the specified ServiceAccount will be used, and KubeBlocks will not + create a ServiceAccount. But KubeBlocks does create RoleBindings for the specified ServiceAccount. type: string serviceRefs: description: |- diff --git a/config/crd/bases/apps.kubeblocks.io_componentdefinitions.yaml b/config/crd/bases/apps.kubeblocks.io_componentdefinitions.yaml index e9349ebd2aa..cbd0b5f6e57 100644 --- a/config/crd/bases/apps.kubeblocks.io_componentdefinitions.yaml +++ b/config/crd/bases/apps.kubeblocks.io_componentdefinitions.yaml @@ -8331,9 +8331,6 @@ spec: This ensures that the Pods in the Component has appropriate permissions to function. - Note: This field is currently non-functional and is reserved for future implementation. - - This field is immutable. items: description: |- diff --git a/config/crd/bases/apps.kubeblocks.io_components.yaml b/config/crd/bases/apps.kubeblocks.io_components.yaml index b2a53efd985..2cc1b07a038 100644 --- a/config/crd/bases/apps.kubeblocks.io_components.yaml +++ b/config/crd/bases/apps.kubeblocks.io_components.yaml @@ -4841,18 +4841,16 @@ spec: with other Kubernetes resources, such as modifying Pod labels or sending events. - Defaults: - If not specified, KubeBlocks automatically assigns a default ServiceAccount named "kb-{cluster.name}", - bound to a default role defined during KubeBlocks installation. - - - Future Changes: - Future versions might change the default ServiceAccount creation strategy to one per Component, - potentially revising the naming to "kb-{cluster.name}-{component.name}". + If not specified, KubeBlocks automatically creates a default ServiceAccount named + "kb-{componentdefinition.name}", bound to a role with rules defined in ComponentDefinition's + `policyRules` field. If needed (currently this means if any lifecycleAction is enabled), + it will also be bound to a default role named + "kubeblocks-cluster-pod-role", which is installed together with KubeBlocks. + If multiple components use the same ComponentDefinition, they will share one ServiceAccount. - Users can override the automatic ServiceAccount assignment by explicitly setting the name of - an existed ServiceAccount in this field. + If the field is not empty, the specified ServiceAccount will be used, and KubeBlocks will not + create a ServiceAccount. But KubeBlocks does create RoleBindings for the specified ServiceAccount. type: string serviceRefs: description: |- diff --git a/controllers/apps/cluster/transformer_cluster_deletion.go b/controllers/apps/cluster/transformer_cluster_deletion.go index b51c3ffd571..fae53ff4745 100644 --- a/controllers/apps/cluster/transformer_cluster_deletion.go +++ b/controllers/apps/cluster/transformer_cluster_deletion.go @@ -21,14 +21,13 @@ package cluster import ( "fmt" - "reflect" "strings" "time" "golang.org/x/exp/maps" corev1 "k8s.io/api/core/v1" - rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" @@ -129,7 +128,7 @@ func (t *clusterDeletionTransformer) Transform(ctx graph.TransformContext, dag * delKindMap := map[string]sets.Empty{} for _, o := range delObjs { // skip the objects owned by the component and InstanceSet controller - if shouldSkipObjOwnedByComp(o, *cluster) || isOwnedByInstanceSet(o) { + if isOwnedByComp(o) || isOwnedByInstanceSet(o) { continue } graphCli.Delete(dag, o, inUniversalContext4G()) @@ -138,6 +137,7 @@ func (t *clusterDeletionTransformer) Transform(ctx graph.TransformContext, dag * // set cluster action to status until all the sub-resources deleted if len(delObjs) == 0 { + transCtx.Logger.Info(fmt.Sprintf("deleting cluster %v", klog.KObj(cluster))) graphCli.Delete(dag, cluster) } else { transCtx.Logger.Info(fmt.Sprintf("deleting the sub-resource kinds: %v", maps.Keys(delKindMap))) @@ -174,37 +174,6 @@ func kindsForWipeOut() ([]client.ObjectList, []client.ObjectList) { return append(namespacedKinds, namespacedKindsPlus...), nonNamespacedKinds } -// shouldSkipObjOwnedByComp is used to judge whether the object owned by component should be skipped when deleting the cluster -func shouldSkipObjOwnedByComp(obj client.Object, cluster appsv1.Cluster) bool { - ownByComp := isOwnedByComp(obj) - if !ownByComp { - // if the object is not owned by component, it should not be skipped - return false - } - - // Due to compatibility reasons, the component controller creates cluster-scoped RoleBinding and ServiceAccount objects in the following two scenarios: - // 1. When the user does not specify a ServiceAccount, KubeBlocks automatically creates a ServiceAccount and a RoleBinding with named pattern kb-{cluster.Name}. - // 2. When the user specifies a ServiceAccount that does not exist, KubeBlocks will automatically create a ServiceAccount and a RoleBinding with the same name. - // In both cases, the lifecycle of the RoleBinding and ServiceAccount should not be tied to the component. They should be deleted when the cluster is deleted. - doNotSkipTypes := []interface{}{ - &rbacv1.RoleBinding{}, - &corev1.ServiceAccount{}, - } - for _, t := range doNotSkipTypes { - if objType, ok := obj.(interface{ GetName() string }); ok && reflect.TypeOf(obj) == reflect.TypeOf(t) { - if strings.EqualFold(objType.GetName(), constant.GenerateDefaultServiceAccountName(cluster.GetName())) { - return false - } - labels := obj.GetLabels() - value, ok := labels[constant.AppManagedByLabelKey] - if ok && value == constant.AppName { - return false - } - } - } - return true -} - func deleteCompNShardingInOrder4Terminate(transCtx *clusterTransformContext, dag *graph.DAG) (sets.Set[string], error) { nameSet, err := clusterRunningCompNShardingSet(transCtx.Context, transCtx.Client, transCtx.Cluster) if err != nil { diff --git a/controllers/apps/component/component_controller.go b/controllers/apps/component/component_controller.go index c097ea79abf..e51fd464402 100644 --- a/controllers/apps/component/component_controller.go +++ b/controllers/apps/component/component_controller.go @@ -212,10 +212,8 @@ func (r *ComponentReconciler) setupWithManager(mgr ctrl.Manager) error { if viper.GetBool(constant.EnableRBACManager) { b.Owns(&rbacv1.RoleBinding{}). + Owns(&rbacv1.Role{}). Owns(&corev1.ServiceAccount{}) - } else { - b.Watches(&rbacv1.RoleBinding{}, handler.EnqueueRequestsFromMapFunc(r.filterComponentResources)). - Watches(&corev1.ServiceAccount{}, handler.EnqueueRequestsFromMapFunc(r.filterComponentResources)) } return b.Complete(r) diff --git a/controllers/apps/component/component_controller_test.go b/controllers/apps/component/component_controller_test.go index a04abeb486c..c658e773b23 100644 --- a/controllers/apps/component/component_controller_test.go +++ b/controllers/apps/component/component_controller_test.go @@ -48,6 +48,7 @@ import ( dpv1alpha1 "github.com/apecloud/kubeblocks/apis/dataprotection/v1alpha1" workloads "github.com/apecloud/kubeblocks/apis/workloads/v1" "github.com/apecloud/kubeblocks/pkg/constant" + "github.com/apecloud/kubeblocks/pkg/controller/builder" "github.com/apecloud/kubeblocks/pkg/controller/component" "github.com/apecloud/kubeblocks/pkg/controller/plan" intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" @@ -108,7 +109,10 @@ var _ = Describe("Component Controller", func() { inNS := client.InNamespace(testCtx.DefaultNamespace) ml := client.HasLabels{testCtx.TestObjLabelKey} // namespaced - testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.ComponentSignature, true, inNS, ml) + testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.ComponentSignature, true, inNS) + testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.ServiceAccountSignature, true, inNS) + testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.RoleSignature, true, inNS) + testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.RoleBindingSignature, true, inNS) testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.PersistentVolumeClaimSignature, true, inNS, ml) testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.PodSignature, true, inNS, ml) testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.BackupSignature, true, inNS, ml) @@ -138,7 +142,6 @@ var _ = Describe("Component Controller", func() { createAllDefinitionObjects := func() { By("Create a componentDefinition obj") compDefObj = testapps.NewComponentDefinitionFactory(compDefName). - WithRandomName(). AddAnnotations(constant.SkipImmutableCheckAnnotationKey, "true"). SetDefaultSpec(). Create(&testCtx). @@ -1338,33 +1341,29 @@ var _ = Describe("Component Controller", func() { })).Should(Succeed()) } - checkRBACResourcesExistence := func(saName string, expectExisted bool) { + checkRBACResourcesExistence := func(saName, rbName string, expectExisted bool) { saKey := types.NamespacedName{ Namespace: compObj.Namespace, Name: saName, } rbKey := types.NamespacedName{ Namespace: compObj.Namespace, - Name: saName, + Name: rbName, } Eventually(testapps.CheckObjExists(&testCtx, saKey, &corev1.ServiceAccount{}, expectExisted)).Should(Succeed()) Eventually(testapps.CheckObjExists(&testCtx, rbKey, &rbacv1.RoleBinding{}, expectExisted)).Should(Succeed()) } testCompRBAC := func(compName, compDefName, saName string) { - By("update comp definition to enable lifecycle actions") - Expect(testapps.GetAndChangeObj(&testCtx, client.ObjectKeyFromObject(compDefObj), func(compDef *kbappsv1.ComponentDefinition) { - compDef.Spec.LifecycleActions.Readonly = testapps.NewLifecycleAction("readonly") - compDef.Spec.LifecycleActions.Readwrite = testapps.NewLifecycleAction("readwrite") - })()).Should(Succeed()) - By("creating a component with target service account name") if len(saName) == 0 { - saName = "test-sa-" + randomStr() + createClusterObj(compName, compDefName, nil) + saName = constant.GenerateDefaultServiceAccountName(compDefName) + } else { + createClusterObj(compName, compDefName, func(f *testapps.MockClusterFactory) { + f.SetServiceAccountName(saName) + }) } - createClusterObj(compName, compDefName, func(f *testapps.MockClusterFactory) { - f.SetServiceAccountName(saName) - }) By("check the service account used in Pod") itsKey := types.NamespacedName{ @@ -1375,52 +1374,107 @@ var _ = Describe("Component Controller", func() { g.Expect(its.Spec.Template.Spec.ServiceAccountName).To(Equal(saName)) })).Should(Succeed()) - By("check the RBAC resources created") - checkRBACResourcesExistence(saName, true) + By("check the RBAC resources status") + checkRBACResourcesExistence(saName, fmt.Sprintf("%v-pod", saName), true) + } + + testCompWithRBAC := func(compName, compDefName string) { + testCompRBAC(compName, compDefName, "") + By("delete the cluster(component)") + testapps.DeleteObject(&testCtx, clusterKey, &kbappsv1.Cluster{}) + Eventually(testapps.CheckObjExists(&testCtx, clusterKey, &kbappsv1.Cluster{}, false)).Should(Succeed()) + Eventually(testapps.CheckObjExists(&testCtx, compKey, &kbappsv1.Component{}, false)).Should(Succeed()) + + By("check the RBAC resources deleted") + saName := constant.GenerateDefaultServiceAccountName(compDefName) + checkRBACResourcesExistence(saName, fmt.Sprintf("%v-pod", saName), false) } testRecreateCompWithRBACCreateByKubeBlocks := func(compName, compDefName string) { - saName := "test-sa-" + randomStr() - testCompRBAC(compName, compDefName, saName) + testCompRBAC(compName, compDefName, "") By("delete the cluster(component)") testapps.DeleteObject(&testCtx, clusterKey, &kbappsv1.Cluster{}) Eventually(testapps.CheckObjExists(&testCtx, clusterKey, &kbappsv1.Cluster{}, false)).Should(Succeed()) By("check the RBAC resources deleted") - checkRBACResourcesExistence(saName, false) + saName := constant.GenerateDefaultServiceAccountName(compDefName) + checkRBACResourcesExistence(saName, fmt.Sprintf("%v-pod", saName), false) By("re-create cluster(component) with same name") - testCompRBAC(compName, compDefName, saName) + testCompRBAC(compName, compDefName, "") } - tesCreateCompWithRBACCreateByUser := func(compName, compDefName string) { - saName := "test-sa-exist" + randomStr() - - testCompRBAC(compName, compDefName, saName) + testSharedRBACResoucreDeletion := func() { + By("create first cluster") + createClusterObj(defaultCompName+"-comp1", compDefName, nil) + comp1Key := compKey + cluster1Key := clusterKey + By("check rbac resources owner") + saName := constant.GenerateDefaultServiceAccountName(compDefName) saKey := types.NamespacedName{ Namespace: compObj.Namespace, Name: saName, } - By("mock the ServiceAccount and RoleBinding created by user by setting annotations to nil") - Eventually(testapps.GetAndChangeObj(&testCtx, saKey, func(sa *corev1.ServiceAccount) { - sa.Annotations = nil + Eventually(testapps.CheckObj(&testCtx, saKey, func(g Gomega, sa *corev1.ServiceAccount) { + refs := sa.GetOwnerReferences() + g.Expect(refs).Should(HaveLen(1)) + owner := refs[0] + g.Expect(owner.Name).Should(Equal(comp1Key.Name)) })).Should(Succeed()) - rbKey := types.NamespacedName{ - Namespace: compObj.Namespace, - Name: saName, - } - Eventually(testapps.GetAndChangeObj(&testCtx, rbKey, func(rb *rbacv1.RoleBinding) { - rb.Annotations = nil + + checkRBACResourcesExistence(saName, fmt.Sprintf("%v-pod", saName), true) + + By("create second cluster") + createClusterObj(defaultCompName+"-comp2", compDefName, nil) + comp2Key := compKey + By("check rbac resources owner not modified") + Consistently(testapps.CheckObj(&testCtx, saKey, func(g Gomega, sa *corev1.ServiceAccount) { + refs := sa.GetOwnerReferences() + g.Expect(refs).Should(HaveLen(1)) + owner := refs[0] + g.Expect(owner.Name).Should(Equal(comp1Key.Name)) + })).Should(Succeed()) + + By("delete first cluster") + testapps.DeleteObject(&testCtx, cluster1Key, &kbappsv1.Cluster{}) + Eventually(testapps.CheckObjExists(&testCtx, cluster1Key, &kbappsv1.Cluster{}, false)).Should(Succeed()) + + By("check rbac resources owner transferred") + Eventually(testapps.CheckObj(&testCtx, saKey, func(g Gomega, sa *corev1.ServiceAccount) { + refs := sa.GetOwnerReferences() + g.Expect(refs).Should(HaveLen(1)) + owner := refs[0] + g.Expect(owner.Name).Should(Equal(comp2Key.Name)) })).Should(Succeed()) + } + + testCreateCompWithNonExistRBAC := func(compName, compDefName string) { + saName := "test-sa-non-exist" + randomStr() + + // component controller won't complete reconciliation, so the phase will be empty + createClusterObjWithPhase(compName, compDefName, func(f *testapps.MockClusterFactory) { + f.SetServiceAccountName(saName) + }, kbappsv1.ClusterPhase("")) + Consistently(testapps.GetComponentPhase(&testCtx, compKey)).Should(Equal(kbappsv1.ComponentPhase(""))) + } + + testCreateCompWithRBACCreateByUser := func(compName, compDefName string) { + saName := "test-sa-exist" + randomStr() + + By("user manually creates ServiceAccount and RoleBinding") + sa := builder.NewServiceAccountBuilder(testCtx.DefaultNamespace, saName).GetObject() + testapps.CheckedCreateK8sResource(&testCtx, sa) + + testCompRBAC(compName, compDefName, saName) By("delete the cluster(component)") testapps.DeleteObject(&testCtx, clusterKey, &kbappsv1.Cluster{}) Eventually(testapps.CheckObjExists(&testCtx, clusterKey, &kbappsv1.Cluster{}, true)).Should(Succeed()) - By("check the RBAC resources deleted") - checkRBACResourcesExistence(saName, true) + By("check the serviceaccount not deleted") + Eventually(testapps.CheckObjExists(&testCtx, client.ObjectKeyFromObject(sa), &corev1.ServiceAccount{}, true)).Should(Succeed()) } testThreeReplicas := func(compName, compDefName string) { @@ -1646,16 +1700,24 @@ var _ = Describe("Component Controller", func() { testCompTLSConfig(defaultCompName, compDefName) }) - It("with component RBAC set", func() { - testCompRBAC(defaultCompName, compDefName, "") + It("creates component RBAC resources", func() { + testCompWithRBAC(defaultCompName, compDefName) }) - It("re-create component with custom RBAC which is not exist and auto created by KubeBlocks", func() { + It("re-creates component with custom RBAC which is not exist and auto created by KubeBlocks", func() { testRecreateCompWithRBACCreateByKubeBlocks(defaultCompName, compDefName) }) + It("transfers rbac resources' ownership when multiple components share them", func() { + testSharedRBACResoucreDeletion() + }) + + It("creates component with non-exist serviceaccount", func() { + testCreateCompWithNonExistRBAC(defaultCompName, compDefName) + }) + It("create component with custom RBAC which is already exist created by User", func() { - tesCreateCompWithRBACCreateByUser(defaultCompName, compDefName) + testCreateCompWithRBACCreateByUser(defaultCompName, compDefName) }) }) diff --git a/controllers/apps/component/transformer_component_deletion.go b/controllers/apps/component/transformer_component_deletion.go index 2c35763798b..fe9ea579989 100644 --- a/controllers/apps/component/transformer_component_deletion.go +++ b/controllers/apps/component/transformer_component_deletion.go @@ -27,7 +27,9 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" @@ -37,6 +39,7 @@ import ( "github.com/apecloud/kubeblocks/pkg/controller/graph" "github.com/apecloud/kubeblocks/pkg/controller/model" intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) // componentDeletionTransformer handles component deletion @@ -125,6 +128,13 @@ func (t *componentDeletionTransformer) deleteCompResources(transCtx *componentTr if isOwnedByInstanceSet(object) { continue } + skipDeletion, err := handleRBACResourceDeletion(object, transCtx, comp, graphCli, dag) + if err != nil { + return fmt.Errorf("handle rbac deletion failed: %w", err) + } + if skipDeletion { + continue + } graphCli.Delete(dag, object) } graphCli.Status(dag, comp, transCtx.Component) @@ -143,6 +153,64 @@ func (t *componentDeletionTransformer) deleteCompResources(transCtx *componentTr return graph.ErrPrematureStop } +func handleRBACResourceDeletion(obj client.Object, transCtx *componentTransformContext, comp *appsv1.Component, + graphCli model.GraphClient, dag *graph.DAG) (skipDeletion bool, err error) { + switch v := obj.(type) { + case *corev1.ServiceAccount, *rbacv1.Role, *rbacv1.RoleBinding: + // list other components that reference the same componentdefinition + transCtx.Logger.V(1).Info("handling rbac resources deletion", + "comp", comp.Name, "name", klog.KObj(v).String()) + compDefName := comp.Spec.CompDef + compList := &appsv1.ComponentList{} + if err := transCtx.Client.List(transCtx.Context, compList, client.InNamespace(comp.Namespace), + client.MatchingLabels{constant.ComponentDefinitionLabelKey: compDefName}); err != nil { + return false, err + } + // if any, transfer ownership to any other component + for _, otherComp := range compList.Items { + // skip current component + if otherComp.Name == comp.Name { + continue + } + // skip deleting component + if !otherComp.DeletionTimestamp.IsZero() { + continue + } + + if err := controllerutil.RemoveControllerReference(comp, v, rscheme); err != nil { + return false, err + } + if err := controllerutil.SetControllerReference(&otherComp, v, rscheme); err != nil { + return false, err + } + // component controller selects a comp's subresource by labels, so change them too + clusterName, err := component.GetClusterName(&otherComp) + if err != nil { + return false, err + } + compShortName, err := component.ShortName(clusterName, otherComp.Name) + if err != nil { + return false, err + } + newLabels := constant.GetCompLabels(clusterName, compShortName) + for k, val := range newLabels { + v.GetLabels()[k] = val + } + graphCli.Update(dag, nil, v) + gvk, err := apiutil.GVKForObject(v, rscheme) + if err != nil { + return false, err + } + transCtx.Logger.V(1).Info("rbac resources owner transferred, skip deletion", + "fromComp", comp.Name, "toComp", otherComp.Name, "name", klog.KObj(v).String(), "gvk", gvk) + return true, nil + } + return false, nil + default: + return false, nil + } +} + func (t *componentDeletionTransformer) getCluster(transCtx *componentTransformContext, comp *appsv1.Component) (*appsv1.Cluster, error) { clusterName, err := component.GetClusterName(comp) if err != nil { @@ -171,6 +239,7 @@ func compOwnedKinds() []client.ObjectList { &corev1.PersistentVolumeClaimList{}, &appsv1alpha1.ConfigurationList{}, &corev1.ServiceAccountList{}, + &rbacv1.RoleList{}, &rbacv1.RoleBindingList{}, } } diff --git a/controllers/apps/component/transformer_component_rbac.go b/controllers/apps/component/transformer_component_rbac.go index eef1d491498..83247b00215 100644 --- a/controllers/apps/component/transformer_component_rbac.go +++ b/controllers/apps/component/transformer_component_rbac.go @@ -21,23 +21,24 @@ package component import ( "fmt" - "time" corev1 "k8s.io/api/core/v1" 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" appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" workloads "github.com/apecloud/kubeblocks/apis/workloads/v1" "github.com/apecloud/kubeblocks/pkg/common" "github.com/apecloud/kubeblocks/pkg/constant" - "github.com/apecloud/kubeblocks/pkg/controller/component" "github.com/apecloud/kubeblocks/pkg/controller/factory" "github.com/apecloud/kubeblocks/pkg/controller/graph" "github.com/apecloud/kubeblocks/pkg/controller/model" - ictrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" + "github.com/apecloud/kubeblocks/pkg/generics" viper "github.com/apecloud/kubeblocks/pkg/viperx" ) @@ -46,8 +47,11 @@ type componentRBACTransformer struct{} var _ graph.Transformer = &componentRBACTransformer{} +const EventReasonRBACManager = "RBACManager" + func (t *componentRBACTransformer) Transform(ctx graph.TransformContext, dag *graph.DAG) error { transCtx, _ := ctx.(*componentTransformContext) + synthesizedComp := transCtx.SynthesizeComponent if model.IsObjectDeleting(transCtx.ComponentOrig) { return nil } @@ -57,39 +61,56 @@ func (t *componentRBACTransformer) Transform(ctx graph.TransformContext, dag *gr return nil } - graphCli, _ := transCtx.Client.(model.GraphClient) - - serviceAccount, err := buildServiceAccount(transCtx) - if err != nil { - return err + 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 serviceAccountName = transCtx.Component.Spec.ServiceAccountName; serviceAccountName != "" { + // if user provided serviceaccount does not exist, raise error + sa := &corev1.ServiceAccount{} + if err := transCtx.Client.Get(transCtx.Context, types.NamespacedName{Namespace: synthesizedComp.Namespace, Name: serviceAccountName}, sa); err != nil { + if errors.IsNotFound(err) { + transCtx.EventRecorder.Event(transCtx.Cluster, corev1.EventTypeWarning, EventReasonRBACManager, fmt.Sprintf("serviceaccount %v not found", serviceAccountName)) + } + return err + } } - if serviceAccount == nil { - transCtx.Logger.V(1).Info("buildServiceAccounts returns serviceAccount nil") + if !viper.GetBool(constant.EnableRBACManager) { + transCtx.EventRecorder.Event(transCtx.Cluster, corev1.EventTypeNormal, EventReasonRBACManager, "RBAC manager is disabled") return nil } - if isServiceAccountExist(transCtx, serviceAccount.Name) { - return nil - } + graphCli, _ := transCtx.Client.(model.GraphClient) - if !viper.GetBool(constant.EnableRBACManager) { - transCtx.Logger.V(1).Info("rbac manager is disabled") - transCtx.EventRecorder.Event(transCtx.Cluster, corev1.EventTypeWarning, - string(ictrlutil.ErrorTypeNotFound), fmt.Sprintf("ServiceAccount %s is not exist", serviceAccount.Name)) - return ictrlutil.NewRequeueError(time.Second, "RBAC manager is disabled, but service account is not exist") + var err error + if serviceAccountName == "" { + serviceAccountName = constant.GenerateDefaultServiceAccountName(synthesizedComp.CompDefName) + // 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 + } + } + role, err := createOrUpdateRole(transCtx, graphCli, dag) + if err != nil { + return err } - rb, err := buildRoleBinding(transCtx.SynthesizeComponent, transCtx.Component, serviceAccount.Name) + rbs, err := createOrUpdateRoleBinding(transCtx, role, serviceAccountName, graphCli, dag) if err != nil { return err } - graphCli.Create(dag, rb, inDataContext4G()) - createServiceAccount(serviceAccount, graphCli, dag, rb) - itsList := graphCli.FindAll(dag, &workloads.InstanceSet{}) - for _, its := range itsList { - // serviceAccount must be created before workload - graphCli.DependOn(dag, its, serviceAccount) + if sa != nil { + // serviceAccount should be created before roleBinding and role + for _, rb := range rbs { + graphCli.DependOn(dag, rb, sa, role) + } + // serviceAccount should be created before workload + itsList := graphCli.FindAll(dag, &workloads.InstanceSet{}) + for _, its := range itsList { + graphCli.DependOn(dag, its, sa) + } } return nil @@ -99,103 +120,121 @@ func isLifecycleActionsEnabled(compDef *appsv1.ComponentDefinition) bool { return compDef.Spec.LifecycleActions != nil } -func isServiceAccountExist(transCtx *componentTransformContext, serviceAccountName string) bool { - synthesizedComp := transCtx.SynthesizeComponent - namespaceName := types.NamespacedName{ - Namespace: synthesizedComp.Namespace, - Name: serviceAccountName, - } - sa := &corev1.ServiceAccount{} - if err := transCtx.Client.Get(transCtx.Context, namespaceName, sa, inDataContext4C()); err != nil { - // KubeBlocks will create a rolebinding only if it has RBAC access priority and - // the rolebinding is not already present. - if errors.IsNotFound(err) { - transCtx.Logger.V(1).Info("ServiceAccount not exists", "namespaceName", namespaceName) - return false +func labelAndAnnotationEqual(old, new metav1.Object) bool { + // exclude component labels, since they are different for each component + compLabels := constant.GetCompLabels("", "") + oldLabels := make(map[string]string) + for k, v := range old.GetLabels() { + if _, ok := compLabels[k]; !ok { + oldLabels[k] = v } - transCtx.Logger.Error(err, "get ServiceAccount failed") - return false } - return true + newLabels := make(map[string]string) + for k, v := range new.GetLabels() { + if _, ok := compLabels[k]; !ok { + newLabels[k] = v + } + } + return equality.Semantic.DeepEqual(oldLabels, newLabels) && + equality.Semantic.DeepEqual(old.GetAnnotations(), new.GetAnnotations()) } -func isRoleBindingExist(transCtx *componentTransformContext, serviceAccountName string) bool { - synthesizedComp := transCtx.SynthesizeComponent - namespaceName := types.NamespacedName{ - Namespace: synthesizedComp.Namespace, - Name: constant.GenerateDefaultServiceAccountName(synthesizedComp.ClusterName), - } - rb := &rbacv1.RoleBinding{} - if err := transCtx.Client.Get(transCtx.Context, namespaceName, rb, inDataContext4C()); err != nil { - // KubeBlocks will create a role binding only if it has RBAC access priority and - // the role binding is not already present. +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) { + oldObj := PT(new(T)) + if err := transCtx.Client.Get(transCtx.Context, client.ObjectKeyFromObject(obj), oldObj); err != nil { if errors.IsNotFound(err) { - transCtx.Logger.V(1).Info("RoleBinding not exists", "namespaceName", namespaceName) - return false + graphCli.Create(dag, obj, inDataContext4G()) + return obj, nil } - transCtx.Logger.Error(err, fmt.Sprintf("get role binding failed: %s", namespaceName)) - return false + return nil, err } - - if rb.RoleRef.Name != constant.RBACRoleName { - transCtx.Logger.V(1).Info("rbac manager: ClusterRole not match", "ClusterRole", - constant.RBACRoleName, "rolebinding.RoleRef", rb.RoleRef.Name) + if !cmpFn(oldObj, obj) { + transCtx.Logger.V(1).Info("updating rbac resources", "name", klog.KObj(obj).String(), "obj", fmt.Sprintf("%#v", obj)) + graphCli.Update(dag, oldObj, obj, inDataContext4G()) } + return obj, nil +} - isServiceAccountMatch := false - for _, sub := range rb.Subjects { - if sub.Kind == rbacv1.ServiceAccountKind && sub.Name == serviceAccountName { - isServiceAccountMatch = true - break - } - } +func createOrUpdateServiceAccount(transCtx *componentTransformContext, serviceAccountName string, graphCli model.GraphClient, dag *graph.DAG) (*corev1.ServiceAccount, error) { + synthesizedComp := transCtx.SynthesizeComponent - if !isServiceAccountMatch { - transCtx.Logger.V(1).Info("rbac manager: ServiceAccount not match", "ServiceAccount", - serviceAccountName, "rolebinding.Subjects", rb.Subjects) + sa := factory.BuildServiceAccount(synthesizedComp, serviceAccountName) + if err := setCompOwnershipNFinalizer(transCtx.Component, sa); err != nil { + return nil, err } - return true -} -// buildServiceAccount builds the service account for the component. -func buildServiceAccount(transCtx *componentTransformContext) (*corev1.ServiceAccount, error) { - var ( - cluster = transCtx.Cluster - comp = transCtx.Component - compDef = transCtx.CompDef - synthesizedComp = transCtx.SynthesizeComponent - ) - serviceAccountName := comp.Spec.ServiceAccountName - if serviceAccountName == "" { - // If lifecycle actions are disabled, then do not create a service account. - if !isLifecycleActionsEnabled(compDef) { - return nil, nil - } - // use cluster.name to keep compatible with existed clusters - serviceAccountName = constant.GenerateDefaultServiceAccountName(cluster.Name) - } + return createOrUpdate(transCtx, sa, graphCli, dag, func(old, new *corev1.ServiceAccount) bool { + 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) + }) +} - if isRoleBindingExist(transCtx, serviceAccountName) && isServiceAccountExist(transCtx, serviceAccountName) { +func createOrUpdateRole( + transCtx *componentTransformContext, graphCli model.GraphClient, dag *graph.DAG, +) (*rbacv1.Role, error) { + role := factory.BuildRole(transCtx.SynthesizeComponent, transCtx.CompDef) + if role == nil { return nil, nil } - - saObj := factory.BuildServiceAccount(synthesizedComp, serviceAccountName) - if err := setCompOwnershipNFinalizer(comp, saObj); err != nil { + if err := setCompOwnershipNFinalizer(transCtx.Component, role); err != nil { return nil, err } - return saObj, nil + return createOrUpdate(transCtx, role, graphCli, dag, func(old, new *rbacv1.Role) bool { + return labelAndAnnotationEqual(old, new) && + equality.Semantic.DeepEqual(old.Rules, new.Rules) + }) } -func buildRoleBinding(synthesizedComp *component.SynthesizedComponent, comp *appsv1.Component, serviceAccountName string) (*rbacv1.RoleBinding, error) { - roleBinding := factory.BuildRoleBinding(synthesizedComp, serviceAccountName) - if err := setCompOwnershipNFinalizer(comp, roleBinding); err != nil { - return nil, err +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 labelAndAnnotationEqual(old, new) && + equality.Semantic.DeepEqual(old.Subjects, new.Subjects) && + equality.Semantic.DeepEqual(old.RoleRef, new.RoleRef) + } + res := make([]*rbacv1.RoleBinding, 0) + + if cmpdRole != nil { + cmpdRoleBinding := factory.BuildRoleBinding(transCtx.SynthesizeComponent, serviceAccountName, &rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "Role", + Name: cmpdRole.Name, + }, serviceAccountName) + if err := setCompOwnershipNFinalizer(transCtx.Component, cmpdRoleBinding); err != nil { + return nil, err + } + rb, err := createOrUpdate(transCtx, cmpdRoleBinding, graphCli, dag, cmpRoleBinding) + if err != nil { + return nil, err + } + res = append(res, rb) + } + + if isLifecycleActionsEnabled(transCtx.CompDef) { + clusterPodRoleBinding := factory.BuildRoleBinding( + transCtx.SynthesizeComponent, + fmt.Sprintf("%v-pod", serviceAccountName), + &rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: constant.RBACRoleName, + }, + serviceAccountName, + ) + if err := setCompOwnershipNFinalizer(transCtx.Component, clusterPodRoleBinding); err != nil { + return nil, err + } + rb, err := createOrUpdate(transCtx, clusterPodRoleBinding, graphCli, dag, cmpRoleBinding) + if err != nil { + return nil, err + } + res = append(res, rb) } - return roleBinding, nil -} -func createServiceAccount(serviceAccount *corev1.ServiceAccount, graphCli model.GraphClient, dag *graph.DAG, parent client.Object) { - // serviceAccount must be created before roleBinding - graphCli.Create(dag, serviceAccount, inDataContext4G()) - graphCli.DependOn(dag, parent, serviceAccount) + return res, nil } diff --git a/controllers/apps/component/transformer_component_rbac_test.go b/controllers/apps/component/transformer_component_rbac_test.go index db8116b1ec8..cfab0daad3a 100644 --- a/controllers/apps/component/transformer_component_rbac_test.go +++ b/controllers/apps/component/transformer_component_rbac_test.go @@ -20,10 +20,16 @@ along with this program. If not, see . package component import ( + "fmt" + "reflect" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/client" 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" @@ -33,15 +39,14 @@ import ( "github.com/apecloud/kubeblocks/pkg/controller/factory" "github.com/apecloud/kubeblocks/pkg/controller/graph" "github.com/apecloud/kubeblocks/pkg/controller/model" + "github.com/apecloud/kubeblocks/pkg/generics" testapps "github.com/apecloud/kubeblocks/pkg/testutil/apps" - viper "github.com/apecloud/kubeblocks/pkg/viperx" ) var _ = Describe("object rbac transformer test.", func() { - const compDefName = "test-compdef" const clusterName = "test-cluster" const compName = "default" - var serviceAccountName = constant.GenerateDefaultServiceAccountName(clusterName) + var serviceAccountName = "" var transCtx graph.TransformContext var dag *graph.DAG @@ -51,23 +56,42 @@ var _ = Describe("object rbac transformer test.", func() { var compDefObj *appsv1.ComponentDefinition var compObj *appsv1.Component var synthesizedComp *component.SynthesizedComponent - var saKey types.NamespacedName - var allSettings map[string]interface{} - BeforeEach(func() { + AfterEach(func() { + inNS := client.InNamespace(testCtx.DefaultNamespace) + testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.ServiceAccountSignature, true, inNS) + testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.RoleBindingSignature, true, inNS) + }) + + init := func(enableLifecycleAction bool, enablePolicyRules bool) { By("Create a component definition") - compDefObj = testapps.NewComponentDefinitionFactory(compDefName). + compDefName := "test-compdef" + compDefFactory := testapps.NewComponentDefinitionFactory(compDefName). WithRandomName(). SetDefaultSpec(). - Create(&testCtx). - GetObject() + Create(&testCtx) + + // default spec has some lifecycle actions + if !enableLifecycleAction { + compDefFactory.Get().Spec.LifecycleActions = nil + } + + if enablePolicyRules { + compDefFactory.SetPolicyRules([]rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pod"}, + Verbs: []string{"get", "list", "watch"}, + }, + }) + } + compDefObj = compDefFactory.GetObject() By("Creating a cluster") cluster = testapps.NewClusterFactory(testCtx.DefaultNamespace, clusterName, ""). WithRandomName(). AddComponent(compName, compDefName). SetReplicas(1). - SetServiceAccountName(serviceAccountName). GetObject() By("Creating a component") @@ -76,24 +100,19 @@ var _ = Describe("object rbac transformer test.", func() { AddAnnotations(constant.KBAppClusterUIDKey, string(cluster.UID)). AddLabels(constant.AppInstanceLabelKey, cluster.Name). SetReplicas(1). - SetServiceAccountName(serviceAccountName). GetObject() - saKey = types.NamespacedName{ - Namespace: testCtx.DefaultNamespace, - Name: serviceAccountName, - } - graphCli = model.NewGraphClient(k8sClient) var err error + serviceAccountName = constant.GenerateDefaultServiceAccountName(compDefFactory.Get().Name) synthesizedComp, err = component.BuildSynthesizedComponent(ctx, k8sClient, compDefObj, compObj, cluster) Expect(err).Should(Succeed()) transCtx = &componentTransformContext{ Context: ctx, Client: graphCli, - EventRecorder: nil, + EventRecorder: clusterRecorder, Logger: logger, Cluster: cluster, CompDef: compDefObj, @@ -104,36 +123,126 @@ var _ = Describe("object rbac transformer test.", func() { dag = mockDAG(graphCli, cluster) transformer = &componentRBACTransformer{} - allSettings = viper.AllSettings() - viper.SetDefault(constant.EnableRBACManager, true) - }) - AfterEach(func() { - viper.SetDefault(constant.EnableRBACManager, false) - if allSettings != nil { - Expect(viper.MergeConfigMap(allSettings)).ShouldNot(HaveOccurred()) - allSettings = nil + saKey := types.NamespacedName{ + Namespace: testCtx.DefaultNamespace, + Name: serviceAccountName, } - }) + Eventually(testapps.CheckObjExists(&testCtx, saKey, + &corev1.ServiceAccount{}, false)).Should(Succeed()) + } Context("transformer rbac manager", func() { - It("create serviceaccount, rolebinding if not exist", func() { - Eventually(testapps.CheckObjExists(&testCtx, saKey, - &corev1.ServiceAccount{}, false)).Should(Succeed()) + 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()) + // sa should be created + serviceAccount := factory.BuildServiceAccount(synthesizedComp, serviceAccountName) + dagExpected := mockDAG(graphCli, cluster) + graphCli.Create(dagExpected, serviceAccount) + Expect(dag.Equals(dagExpected, model.DefaultLess)).Should(BeTrue()) + }) + + It("w/ lifecycle actions", func() { + init(true, false) + Expect(transformer.Transform(transCtx, dag)).Should(BeNil()) + clusterPodRoleBinding := factory.BuildRoleBinding(synthesizedComp, fmt.Sprintf("%v-pod", serviceAccountName), &rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: constant.RBACRoleName, + }, serviceAccountName) serviceAccount := factory.BuildServiceAccount(synthesizedComp, serviceAccountName) - roleBinding := factory.BuildRoleBinding(synthesizedComp, serviceAccount.Name) + dagExpected := mockDAG(graphCli, cluster) + graphCli.Create(dagExpected, serviceAccount) + graphCli.Create(dagExpected, clusterPodRoleBinding) + graphCli.DependOn(dagExpected, clusterPodRoleBinding, serviceAccount) + itsList := graphCli.FindAll(dagExpected, &workloads.InstanceSet{}) + for i := range itsList { + graphCli.DependOn(dagExpected, itsList[i], serviceAccount) + } + Expect(dag.Equals(dagExpected, model.DefaultLess)).Should(BeTrue()) + }) + It("w/ cmpd's PolicyRules", func() { + init(false, true) + Expect(transformer.Transform(transCtx, dag)).Should(BeNil()) + cmpdRole := factory.BuildRole(synthesizedComp, compDefObj) + cmpdRoleBinding := factory.BuildRoleBinding(synthesizedComp, serviceAccountName, &rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "Role", + Name: cmpdRole.Name, + }, serviceAccountName) + serviceAccount := factory.BuildServiceAccount(synthesizedComp, serviceAccountName) dagExpected := mockDAG(graphCli, cluster) graphCli.Create(dagExpected, serviceAccount) - graphCli.Create(dagExpected, roleBinding) - graphCli.DependOn(dagExpected, roleBinding, serviceAccount) + graphCli.Create(dagExpected, cmpdRoleBinding) + graphCli.Create(dagExpected, cmpdRole) + graphCli.DependOn(dagExpected, cmpdRoleBinding, serviceAccount, cmpdRole) itsList := graphCli.FindAll(dagExpected, &workloads.InstanceSet{}) for i := range itsList { graphCli.DependOn(dagExpected, itsList[i], serviceAccount) } Expect(dag.Equals(dagExpected, model.DefaultLess)).Should(BeTrue()) + // DefaultLess doesn't compare objs' contents + actualRoleBinding := graphCli.FindAll(dag, &rbacv1.RoleBinding{}) + Expect(actualRoleBinding).To(HaveLen(1)) + rb := actualRoleBinding[0].(*rbacv1.RoleBinding) + Expect(reflect.DeepEqual(rb.Subjects, cmpdRoleBinding.Subjects)).To(BeTrue()) + Expect(reflect.DeepEqual(rb.RoleRef, cmpdRoleBinding.RoleRef)).To(BeTrue()) }) }) }) diff --git a/controllers/apps/component/transformer_component_utils.go b/controllers/apps/component/transformer_component_utils.go index 80ac1eee756..3e152a10665 100644 --- a/controllers/apps/component/transformer_component_utils.go +++ b/controllers/apps/component/transformer_component_utils.go @@ -20,11 +20,7 @@ along with this program. If not, see . package component import ( - "reflect" - "strings" - corev1 "k8s.io/api/core/v1" - rbacv1 "k8s.io/api/rbac/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -38,7 +34,7 @@ func setCompOwnershipNFinalizer(comp *appsv1.Component, object client.Object) er return nil } // add finalizer to the object - addFinalizer(object, comp) + controllerutil.AddFinalizer(object, constant.DBComponentFinalizerName) if err := intctrlutil.SetOwnership(comp, object, rscheme, ""); err != nil { if _, ok := err.(*controllerutil.AlreadyOwnedError); ok { return nil @@ -57,30 +53,3 @@ func skipSetCompOwnershipNFinalizer(obj client.Object) bool { return false } } - -func addFinalizer(obj client.Object, comp *appsv1.Component) { - if skipAddCompFinalizer(obj, comp) { - return - } - controllerutil.AddFinalizer(obj, constant.DBComponentFinalizerName) -} - -func skipAddCompFinalizer(obj client.Object, comp *appsv1.Component) bool { - // Due to compatibility reasons, the component controller creates cluster-scoped RoleBinding and ServiceAccount objects in the following two scenarios: - // 1. When the user does not specify a ServiceAccount, KubeBlocks automatically creates a ServiceAccount and a RoleBinding with named pattern kb-{cluster.Name}. - // 2. When the user specifies a ServiceAccount that does not exist, KubeBlocks will automatically create a ServiceAccount and a RoleBinding with the same name. - // In both cases, the lifecycle of the RoleBinding and ServiceAccount should not be tied to the component. - skipTypes := []interface{}{ - &rbacv1.RoleBinding{}, - &corev1.ServiceAccount{}, - } - - for _, t := range skipTypes { - if objType, ok := obj.(interface{ GetName() string }); ok && reflect.TypeOf(obj) == reflect.TypeOf(t) { - if !strings.HasPrefix(objType.GetName(), constant.GenerateDefaultServiceAccountName(comp.GetName())) { - return true - } - } - } - return false -} diff --git a/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml b/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml index 7ac44eebc41..4eef6fc3bfb 100644 --- a/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml +++ b/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml @@ -5077,19 +5077,16 @@ spec: with other Kubernetes resources, such as modifying Pod labels or sending events. - Defaults: - To perform certain operational tasks, agent sidecars running in Pods require specific RBAC permissions. - The service account will be bound to a default role named "kubeblocks-cluster-pod-role" which is installed together with KubeBlocks. - If not specified, KubeBlocks automatically assigns a default ServiceAccount named "kb-{cluster.name}" + If not specified, KubeBlocks automatically creates a default ServiceAccount named + "kb-{componentdefinition.name}", bound to a role with rules defined in ComponentDefinition's + `policyRules` field. If needed (currently this means if any lifecycleAction is enabled), + it will also be bound to a default role named + "kubeblocks-cluster-pod-role", which is installed together with KubeBlocks. + If multiple components use the same ComponentDefinition, they will share one ServiceAccount. - Future Changes: - Future versions might change the default ServiceAccount creation strategy to one per Component, - potentially revising the naming to "kb-{cluster.name}-{component.name}". - - - Users can override the automatic ServiceAccount assignment by explicitly setting the name of - an existed ServiceAccount in this field. + If the field is not empty, the specified ServiceAccount will be used, and KubeBlocks will not + create a ServiceAccount. But KubeBlocks does create RoleBindings for the specified ServiceAccount. type: string serviceRefs: description: |- @@ -13827,19 +13824,16 @@ spec: with other Kubernetes resources, such as modifying Pod labels or sending events. - Defaults: - To perform certain operational tasks, agent sidecars running in Pods require specific RBAC permissions. - The service account will be bound to a default role named "kubeblocks-cluster-pod-role" which is installed together with KubeBlocks. - If not specified, KubeBlocks automatically assigns a default ServiceAccount named "kb-{cluster.name}" + If not specified, KubeBlocks automatically creates a default ServiceAccount named + "kb-{componentdefinition.name}", bound to a role with rules defined in ComponentDefinition's + `policyRules` field. If needed (currently this means if any lifecycleAction is enabled), + it will also be bound to a default role named + "kubeblocks-cluster-pod-role", which is installed together with KubeBlocks. + If multiple components use the same ComponentDefinition, they will share one ServiceAccount. - Future Changes: - Future versions might change the default ServiceAccount creation strategy to one per Component, - potentially revising the naming to "kb-{cluster.name}-{component.name}". - - - Users can override the automatic ServiceAccount assignment by explicitly setting the name of - an existed ServiceAccount in this field. + If the field is not empty, the specified ServiceAccount will be used, and KubeBlocks will not + create a ServiceAccount. But KubeBlocks does create RoleBindings for the specified ServiceAccount. type: string serviceRefs: description: |- diff --git a/deploy/helm/crds/apps.kubeblocks.io_componentdefinitions.yaml b/deploy/helm/crds/apps.kubeblocks.io_componentdefinitions.yaml index e9349ebd2aa..cbd0b5f6e57 100644 --- a/deploy/helm/crds/apps.kubeblocks.io_componentdefinitions.yaml +++ b/deploy/helm/crds/apps.kubeblocks.io_componentdefinitions.yaml @@ -8331,9 +8331,6 @@ spec: This ensures that the Pods in the Component has appropriate permissions to function. - Note: This field is currently non-functional and is reserved for future implementation. - - This field is immutable. items: description: |- diff --git a/deploy/helm/crds/apps.kubeblocks.io_components.yaml b/deploy/helm/crds/apps.kubeblocks.io_components.yaml index b2a53efd985..2cc1b07a038 100644 --- a/deploy/helm/crds/apps.kubeblocks.io_components.yaml +++ b/deploy/helm/crds/apps.kubeblocks.io_components.yaml @@ -4841,18 +4841,16 @@ spec: with other Kubernetes resources, such as modifying Pod labels or sending events. - Defaults: - If not specified, KubeBlocks automatically assigns a default ServiceAccount named "kb-{cluster.name}", - bound to a default role defined during KubeBlocks installation. - - - Future Changes: - Future versions might change the default ServiceAccount creation strategy to one per Component, - potentially revising the naming to "kb-{cluster.name}-{component.name}". + If not specified, KubeBlocks automatically creates a default ServiceAccount named + "kb-{componentdefinition.name}", bound to a role with rules defined in ComponentDefinition's + `policyRules` field. If needed (currently this means if any lifecycleAction is enabled), + it will also be bound to a default role named + "kubeblocks-cluster-pod-role", which is installed together with KubeBlocks. + If multiple components use the same ComponentDefinition, they will share one ServiceAccount. - Users can override the automatic ServiceAccount assignment by explicitly setting the name of - an existed ServiceAccount in this field. + If the field is not empty, the specified ServiceAccount will be used, and KubeBlocks will not + create a ServiceAccount. But KubeBlocks does create RoleBindings for the specified ServiceAccount. type: string serviceRefs: description: |- diff --git a/deploy/helm/templates/rbac/cluster_pod_required_role.yaml b/deploy/helm/templates/rbac/cluster_pod_required_role.yaml index b1a0a8e37d1..c60d78048cf 100644 --- a/deploy/helm/templates/rbac/cluster_pod_required_role.yaml +++ b/deploy/helm/templates/rbac/cluster_pod_required_role.yaml @@ -14,11 +14,12 @@ aggregationRule: apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - name: {{ include "kubeblocks.fullname" . }}-agent-pod-role + name: {{ include "kubeblocks.fullname" . }}-kbagent-pod-role labels: {{- include "kubeblocks.labels" . | nindent 4 }} app.kubernetes.io/required-by: pod rules: +# this is needed to create role probe events - apiGroups: - "" resources: @@ -27,88 +28,3 @@ rules: - create - get - update -- apiGroups: - - "" - resources: - - configmaps - verbs: - - create - - get - - list - - patch - - update - - delete -- apiGroups: - - apps.kubeblocks.io - resources: - - clusters - verbs: - - get - - list -- apiGroups: - - apps.kubeblocks.io - resources: - - clusters/status - verbs: - - get -- apiGroups: - - "" - resources: - - pods - verbs: - - get - - list -- apiGroups: - - apps.kubeblocks.io - resources: - - opsrequests/status - verbs: - - get - - patch ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: {{ include "kubeblocks.fullname" . }}-patroni-pod-role - labels: - {{- include "kubeblocks.labels" . | nindent 4 }} - app.kubernetes.io/required-by: pod -rules: -- apiGroups: - - "" - resources: - - configmaps - verbs: - - create - - get - - list - - patch - - update - - watch - # delete is required only for 'patronictl remove' - - delete -- apiGroups: - - "" - resources: - - endpoints - verbs: - - get - - patch - - update - - create - - list - - watch - # delete is required only for 'patronictl remove' - - delete -- apiGroups: - - "" - resources: - - pods - verbs: - - create - - delete - - get - - list - - patch - - update - - watch \ No newline at end of file diff --git a/docs/developer_docs/api-reference/cluster.md b/docs/developer_docs/api-reference/cluster.md index a1df0105db3..5eaa7aa5eb9 100644 --- a/docs/developer_docs/api-reference/cluster.md +++ b/docs/developer_docs/api-reference/cluster.md @@ -663,14 +663,14 @@ string

Specifies the name of the ServiceAccount required by the running Component. This ServiceAccount is used to grant necessary permissions for the Component’s Pods to interact with other Kubernetes resources, such as modifying Pod labels or sending events.

-

Defaults: -If not specified, KubeBlocks automatically assigns a default ServiceAccount named “kb-{cluster.name}”, -bound to a default role defined during KubeBlocks installation.

-

Future Changes: -Future versions might change the default ServiceAccount creation strategy to one per Component, -potentially revising the naming to “kb-{cluster.name}-{component.name}”.

-

Users can override the automatic ServiceAccount assignment by explicitly setting the name of -an existed ServiceAccount in this field.

+

If not specified, KubeBlocks automatically creates a default ServiceAccount named +“kb-{componentdefinition.name}”, bound to a role with rules defined in ComponentDefinition’s +policyRules field. If needed (currently this means if any lifecycleAction is enabled), +it will also be bound to a default role named +“kubeblocks-cluster-pod-role”, which is installed together with KubeBlocks. +If multiple components use the same ComponentDefinition, they will share one ServiceAccount.

+

If the field is not empty, the specified ServiceAccount will be used, and KubeBlocks will not +create a ServiceAccount. But KubeBlocks does create RoleBindings for the specified ServiceAccount.

@@ -1478,7 +1478,6 @@ Kubernetes resources within the namespace.

The purpose of this field is to automatically generate the necessary RBAC roles for the Component based on the specified policy rules. This ensures that the Pods in the Component has appropriate permissions to function.

-

Note: This field is currently non-functional and is reserved for future implementation.

This field is immutable.

@@ -3013,15 +3012,14 @@ string

Specifies the name of the ServiceAccount required by the running Component. This ServiceAccount is used to grant necessary permissions for the Component’s Pods to interact with other Kubernetes resources, such as modifying Pod labels or sending events.

-

Defaults: -To perform certain operational tasks, agent sidecars running in Pods require specific RBAC permissions. -The service account will be bound to a default role named “kubeblocks-cluster-pod-role” which is installed together with KubeBlocks. -If not specified, KubeBlocks automatically assigns a default ServiceAccount named “kb-{cluster.name}”

-

Future Changes: -Future versions might change the default ServiceAccount creation strategy to one per Component, -potentially revising the naming to “kb-{cluster.name}-{component.name}”.

-

Users can override the automatic ServiceAccount assignment by explicitly setting the name of -an existed ServiceAccount in this field.

+

If not specified, KubeBlocks automatically creates a default ServiceAccount named +“kb-{componentdefinition.name}”, bound to a role with rules defined in ComponentDefinition’s +policyRules field. If needed (currently this means if any lifecycleAction is enabled), +it will also be bound to a default role named +“kubeblocks-cluster-pod-role”, which is installed together with KubeBlocks. +If multiple components use the same ComponentDefinition, they will share one ServiceAccount.

+

If the field is not empty, the specified ServiceAccount will be used, and KubeBlocks will not +create a ServiceAccount. But KubeBlocks does create RoleBindings for the specified ServiceAccount.

@@ -5350,7 +5348,6 @@ Kubernetes resources within the namespace.

The purpose of this field is to automatically generate the necessary RBAC roles for the Component based on the specified policy rules. This ensures that the Pods in the Component has appropriate permissions to function.

-

Note: This field is currently non-functional and is reserved for future implementation.

This field is immutable.

@@ -6166,14 +6163,14 @@ string

Specifies the name of the ServiceAccount required by the running Component. This ServiceAccount is used to grant necessary permissions for the Component’s Pods to interact with other Kubernetes resources, such as modifying Pod labels or sending events.

-

Defaults: -If not specified, KubeBlocks automatically assigns a default ServiceAccount named “kb-{cluster.name}”, -bound to a default role defined during KubeBlocks installation.

-

Future Changes: -Future versions might change the default ServiceAccount creation strategy to one per Component, -potentially revising the naming to “kb-{cluster.name}-{component.name}”.

-

Users can override the automatic ServiceAccount assignment by explicitly setting the name of -an existed ServiceAccount in this field.

+

If not specified, KubeBlocks automatically creates a default ServiceAccount named +“kb-{componentdefinition.name}”, bound to a role with rules defined in ComponentDefinition’s +policyRules field. If needed (currently this means if any lifecycleAction is enabled), +it will also be bound to a default role named +“kubeblocks-cluster-pod-role”, which is installed together with KubeBlocks. +If multiple components use the same ComponentDefinition, they will share one ServiceAccount.

+

If the field is not empty, the specified ServiceAccount will be used, and KubeBlocks will not +create a ServiceAccount. But KubeBlocks does create RoleBindings for the specified ServiceAccount.

diff --git a/pkg/constant/pattern.go b/pkg/constant/pattern.go index 6ea94657907..e31d3c0604c 100644 --- a/pkg/constant/pattern.go +++ b/pkg/constant/pattern.go @@ -78,9 +78,14 @@ func GetCompEnvCMName(compObjName string) string { return fmt.Sprintf("%s-env", compObjName) } -// GenerateDefaultServiceAccountName generates default service account name for a cluster. -func GenerateDefaultServiceAccountName(name string) string { - return fmt.Sprintf("%s-%s", KBLowerPrefix, name) +// GenerateDefaultServiceAccountName generates default service account name for a component. +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 diff --git a/pkg/controller/builder/builder_role.go b/pkg/controller/builder/builder_role.go new file mode 100644 index 00000000000..dfacc0df388 --- /dev/null +++ b/pkg/controller/builder/builder_role.go @@ -0,0 +1,39 @@ +/* +Copyright (C) 2022-2024 ApeCloud Co., Ltd + +This file is part of KubeBlocks project + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . +*/ + +package builder + +import ( + rbacv1 "k8s.io/api/rbac/v1" +) + +type RoleBuilder struct { + BaseBuilder[rbacv1.Role, *rbacv1.Role, RoleBuilder] +} + +func NewRoleBuilder(namespace, name string) *RoleBuilder { + builder := &RoleBuilder{} + builder.init(namespace, name, &rbacv1.Role{}, builder) + return builder +} + +func (builder *RoleBuilder) AddPolicyRules(rules []rbacv1.PolicyRule) *RoleBuilder { + builder.get().Rules = append(builder.get().Rules, rules...) + return builder +} diff --git a/pkg/controller/builder/builder_role_test.go b/pkg/controller/builder/builder_role_test.go new file mode 100644 index 00000000000..95fe8544d85 --- /dev/null +++ b/pkg/controller/builder/builder_role_test.go @@ -0,0 +1,58 @@ +/* +Copyright (C) 2022-2024 ApeCloud Co., Ltd + +This file is part of KubeBlocks project + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . +*/ + +package builder + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + rbacv1 "k8s.io/api/rbac/v1" +) + +var _ = Describe("role builder", func() { + It("should build a role", func() { + const ( + name = "foo" + ns = "default" + ) + + rules := []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pod"}, + Verbs: []string{"get", "list", "watch"}, + }, + { + APIGroups: []string{""}, + Resources: []string{"pod/status"}, + Verbs: []string{"get"}, + }, + } + + role := NewRoleBuilder(ns, name). + AddPolicyRules(rules). + GetObject() + + Expect(role.Name).Should(Equal(name)) + Expect(role.Namespace).Should(Equal(ns)) + Expect(role.Rules).Should(HaveLen(2)) + Expect(role.Rules[0]).Should(Equal(rules[0])) + }) +}) diff --git a/pkg/controller/component/synthesize_component.go b/pkg/controller/component/synthesize_component.go index 9e81dc006a5..f0e408d13ef 100644 --- a/pkg/controller/component/synthesize_component.go +++ b/pkg/controller/component/synthesize_component.go @@ -34,6 +34,7 @@ import ( appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" "github.com/apecloud/kubeblocks/pkg/constant" intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" + viper "github.com/apecloud/kubeblocks/pkg/viperx" ) var ( @@ -399,11 +400,10 @@ func buildServiceAccountName(synthesizeComp *SynthesizedComponent) { synthesizeComp.PodSpec.ServiceAccountName = synthesizeComp.ServiceAccountName return } - if synthesizeComp.LifecycleActions == nil || synthesizeComp.LifecycleActions.RoleProbe == nil { + if !viper.GetBool(constant.EnableRBACManager) { return } - synthesizeComp.ServiceAccountName = constant.GenerateDefaultServiceAccountName(synthesizeComp.ClusterName) - // set component.PodSpec.ServiceAccountName + synthesizeComp.ServiceAccountName = constant.GenerateDefaultServiceAccountName(synthesizeComp.CompDefName) synthesizeComp.PodSpec.ServiceAccountName = synthesizeComp.ServiceAccountName } diff --git a/pkg/controller/factory/builder.go b/pkg/controller/factory/builder.go index 3f7c2c92b1a..ab2569944e7 100644 --- a/pkg/controller/factory/builder.go +++ b/pkg/controller/factory/builder.go @@ -350,16 +350,12 @@ func BuildServiceAccount(synthesizedComp *component.SynthesizedComponent, saName GetObject() } -func BuildRoleBinding(synthesizedComp *component.SynthesizedComponent, saName string) *rbacv1.RoleBinding { - return builder.NewRoleBindingBuilder(synthesizedComp.Namespace, 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)). AddAnnotationsInMap(synthesizedComp.StaticAnnotations). - SetRoleRef(rbacv1.RoleRef{ - APIGroup: rbacv1.GroupName, - Kind: "ClusterRole", - Name: constant.RBACRoleName, - }). + SetRoleRef(*roleRef). AddSubjects(rbacv1.Subject{ Kind: rbacv1.ServiceAccountKind, Namespace: synthesizedComp.Namespace, @@ -367,3 +363,16 @@ func BuildRoleBinding(synthesizedComp *component.SynthesizedComponent, saName st }). GetObject() } + +func BuildRole(synthesizedComp *component.SynthesizedComponent, cmpd *appsv1.ComponentDefinition) *rbacv1.Role { + rules := cmpd.Spec.PolicyRules + if len(rules) == 0 { + return nil + } + return builder.NewRoleBuilder(synthesizedComp.Namespace, constant.GenerateDefaultRoleName(cmpd.Name)). + AddLabelsInMap(synthesizedComp.StaticLabels). + AddLabelsInMap(constant.GetCompLabels(synthesizedComp.ClusterName, synthesizedComp.Name)). + AddAnnotationsInMap(synthesizedComp.StaticAnnotations). + AddPolicyRules(rules). + GetObject() +} diff --git a/pkg/controller/factory/builder_test.go b/pkg/controller/factory/builder_test.go index 7af11a7e784..dbb19fd46fc 100644 --- a/pkg/controller/factory/builder_test.go +++ b/pkg/controller/factory/builder_test.go @@ -26,6 +26,7 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -207,7 +208,11 @@ var _ = Describe("builder", func() { It("builds rolebinding correctly", func() { _, cluster, synthesizedComp := newClusterObjs(nil) expectName := fmt.Sprintf("kb-%s", cluster.Name) - rb := BuildRoleBinding(synthesizedComp, expectName) + rb := BuildRoleBinding(synthesizedComp, expectName, &rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: constant.RBACRoleName, + }, expectName) Expect(rb).ShouldNot(BeNil()) Expect(rb.Name).Should(Equal(expectName)) }) diff --git a/pkg/controller/model/graph_client.go b/pkg/controller/model/graph_client.go index fbde5a7ddaf..4b05950d499 100644 --- a/pkg/controller/model/graph_client.go +++ b/pkg/controller/model/graph_client.go @@ -167,7 +167,10 @@ func (r *realGraphClient) DependOn(dag *graph.DAG, object client.Object, depende return } for _, d := range dependency { - if d == nil { + // d == nil can't tell if d is (*T)(nil) + // e.g. `var d *corev1.Pod` + value := reflect.ValueOf(d) + if d == nil || (value.Kind() == reflect.Ptr && value.IsNil()) { continue } v := r.FindMatchedVertex(dag, d) diff --git a/pkg/generics/type.go b/pkg/generics/type.go index be9534330e0..e51fef26916 100644 --- a/pkg/generics/type.go +++ b/pkg/generics/type.go @@ -25,6 +25,7 @@ import ( snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" storagev1 "k8s.io/api/storage/v1" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" @@ -70,6 +71,12 @@ var PersistentVolumeSignature = func(_ corev1.PersistentVolume, _ *corev1.Persis var PodSignature = func(_ corev1.Pod, _ *corev1.Pod, _ corev1.PodList, _ *corev1.PodList) {} var EventSignature = func(_ corev1.Event, _ *corev1.Event, _ corev1.EventList, _ *corev1.EventList) {} var ConfigMapSignature = func(_ corev1.ConfigMap, _ *corev1.ConfigMap, _ corev1.ConfigMapList, _ *corev1.ConfigMapList) {} +var ServiceAccountSignature = func(_ corev1.ServiceAccount, _ *corev1.ServiceAccount, _ corev1.ServiceAccountList, _ *corev1.ServiceAccountList) { +} +var RoleBindingSignature = func(_ rbacv1.RoleBinding, _ *rbacv1.RoleBinding, _ rbacv1.RoleBindingList, _ *rbacv1.RoleBindingList) { +} +var RoleSignature = func(_ rbacv1.Role, _ *rbacv1.Role, _ rbacv1.RoleList, _ *rbacv1.RoleList) { +} var JobSignature = func(_ batchv1.Job, _ *batchv1.Job, _ batchv1.JobList, _ *batchv1.JobList) {} var CronJobSignature = func(_ batchv1.CronJob, _ *batchv1.CronJob, _ batchv1.CronJobList, _ *batchv1.CronJobList) {} diff --git a/pkg/operations/custom/action_workload.go b/pkg/operations/custom/action_workload.go index 9329b90d056..4a8cacb7222 100644 --- a/pkg/operations/custom/action_workload.go +++ b/pkg/operations/custom/action_workload.go @@ -198,7 +198,7 @@ func (w *WorkloadAction) buildPodSpec(actionCtx ActionContext, podSpec.ServiceAccountName = w.Comp.ServiceAccountName default: saKey := client.ObjectKey{Namespace: w.Cluster.Namespace, - Name: constant.GenerateDefaultServiceAccountName(w.Cluster.Name)} + Name: constant.GenerateDefaultServiceAccountName(w.Comp.ComponentDef)} if exists, _ := intctrlutil.CheckResourceExists(actionCtx.ReqCtx.Ctx, actionCtx.Client, saKey, &corev1.ServiceAccount{}); exists { podSpec.ServiceAccountName = saKey.Name }