diff --git a/apis/apps/v1/cluster_types.go b/apis/apps/v1/cluster_types.go index fe8b5eb5236..abf2778adb1 100644 --- a/apis/apps/v1/cluster_types.go +++ b/apis/apps/v1/cluster_types.go @@ -405,17 +405,13 @@ 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}" + // If not specified, KubeBlocks automatically creates a default ServiceAccount named + // "kb-{cluster.name}-{component.name}", bound to a role with rules defined in ComponentDefinition's + // `policyRules` field. It will also be bound to a default role named "kubeblocks-cluster-pod-role" + // which is installed together with KubeBlocks. // - // 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, nor create RoleBinding accordingly. // // +optional ServiceAccountName string `json:"serviceAccountName,omitempty"` diff --git a/apis/apps/v1/component_types.go b/apis/apps/v1/component_types.go index d0dcc15ebd5..4096e3de81c 100644 --- a/apis/apps/v1/component_types.go +++ b/apis/apps/v1/component_types.go @@ -175,16 +175,13 @@ 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. + // If not specified, KubeBlocks automatically creates a default ServiceAccount named + // "kb-{cluster.name}-{component.name}", bound to a role with rules defined in ComponentDefinition's + // `policyRules` field. It will also be bound to a default role named "kubeblocks-cluster-pod-role" + // which is installed together with KubeBlocks. // - // 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, nor create RoleBinding accordingly. // // +optional ServiceAccountName string `json:"serviceAccountName,omitempty"` diff --git a/apis/apps/v1/componentdefinition_types.go b/apis/apps/v1/componentdefinition_types.go index f9bb51652d0..8b60d37fbb9 100644 --- a/apis/apps/v1/componentdefinition_types.go +++ b/apis/apps/v1/componentdefinition_types.go @@ -469,8 +469,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 f6ff2897e8e..e742c2e68e2 100644 --- a/config/crd/bases/apps.kubeblocks.io_clusters.yaml +++ b/config/crd/bases/apps.kubeblocks.io_clusters.yaml @@ -5032,19 +5032,14 @@ 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-{cluster.name}-{component.name}", bound to a role with rules defined in ComponentDefinition's + `policyRules` field. It will also be bound to a default role named "kubeblocks-cluster-pod-role" + which is installed together with KubeBlocks. - 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, nor create RoleBinding accordingly. type: string serviceRefs: description: |- @@ -13734,19 +13729,14 @@ 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-{cluster.name}-{component.name}", bound to a role with rules defined in ComponentDefinition's + `policyRules` field. It will also be bound to a default role named "kubeblocks-cluster-pod-role" + which is installed together with KubeBlocks. - 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, nor create RoleBinding accordingly. 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 54164157d4a..3a7325a9b99 100644 --- a/config/crd/bases/apps.kubeblocks.io_componentdefinitions.yaml +++ b/config/crd/bases/apps.kubeblocks.io_componentdefinitions.yaml @@ -3949,9 +3949,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 d2bc9d66174..eceb1e27938 100644 --- a/config/crd/bases/apps.kubeblocks.io_components.yaml +++ b/config/crd/bases/apps.kubeblocks.io_components.yaml @@ -4800,18 +4800,14 @@ 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-{cluster.name}-{component.name}", bound to a role with rules defined in ComponentDefinition's + `policyRules` field. It will also be bound to a default role named "kubeblocks-cluster-pod-role" + which is installed together with KubeBlocks. - 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, nor create RoleBinding accordingly. type: string serviceRefs: description: |- diff --git a/controllers/apps/component_controller.go b/controllers/apps/component_controller.go index 62d957195ca..66cd78553b1 100644 --- a/controllers/apps/component_controller.go +++ b/controllers/apps/component_controller.go @@ -214,9 +214,6 @@ func (r *ComponentReconciler) setupWithManager(mgr ctrl.Manager) error { if viper.GetBool(constant.EnableRBACManager) { b.Owns(&rbacv1.RoleBinding{}). 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_controller_test.go b/controllers/apps/component_controller_test.go index 355e9a5e8f4..f681341ae79 100644 --- a/controllers/apps/component_controller_test.go +++ b/controllers/apps/component_controller_test.go @@ -50,6 +50,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" @@ -1408,33 +1409,29 @@ var _ = Describe("Component Controller", func() { testCompConfiguration := func(compName, compDefName string) { } - 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()) - + testCompRBAC := func(compName, compDefName, saName string, rbacResourceCreated bool) { By("creating a component with target service account name") if len(saName) == 0 { - saName = "test-sa-" + randomStr() + createClusterObj(compName, compDefName, nil) + saName = constant.GenerateDefaultServiceAccountName(clusterObj.Name, compName) + } 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{ @@ -1445,52 +1442,53 @@ 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), rbacResourceCreated) } testRecreateCompWithRBACCreateByKubeBlocks := func(compName, compDefName string) { - saName := "test-sa-" + randomStr() - testCompRBAC(compName, compDefName, saName) + testCompRBAC(compName, compDefName, "", true) 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(clusterObj.Name, compName) + checkRBACResourcesExistence(saName, fmt.Sprintf("%v-pod", saName), false) By("re-create cluster(component) with same name") - testCompRBAC(compName, compDefName, saName) + testCompRBAC(compName, compDefName, "", true) } tesCreateCompWithRBACCreateByUser := func(compName, compDefName string) { saName := "test-sa-exist" + randomStr() - testCompRBAC(compName, compDefName, saName) + testCompRBAC(compName, compDefName, saName, false) - 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 - })).Should(Succeed()) - rbKey := types.NamespacedName{ - Namespace: compObj.Namespace, - Name: saName, - } - Eventually(testapps.GetAndChangeObj(&testCtx, rbKey, func(rb *rbacv1.RoleBinding) { - rb.Annotations = nil - })).Should(Succeed()) + By("user manually creates ServiceAccount and RoleBinding") + sa := builder.NewServiceAccountBuilder(compObj.Namespace, saName).GetObject() + testapps.CheckedCreateK8sResource(&testCtx, sa) + rb := builder.NewRoleBindingBuilder(compObj.Namespace, saName). + SetRoleRef(rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "Role", + Name: constant.RBACRoleName, + }). + AddSubjects(rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Namespace: compObj.Namespace, + Name: saName, + }). + GetObject() + testapps.CheckedCreateK8sResource(&testCtx, rb) 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 RBAC resources not deleted") + checkRBACResourcesExistence(saName, saName, true) } testThreeReplicas := func(compName, compDefName string) { @@ -1793,7 +1791,7 @@ var _ = Describe("Component Controller", func() { }) It("with component RBAC set", func() { - testCompRBAC(defaultCompName, compDefName, "") + testCompRBAC(defaultCompName, compDefName, "", true) }) It("re-create component with custom RBAC which is not exist and auto created by KubeBlocks", func() { diff --git a/controllers/apps/transformer_cluster_deletion.go b/controllers/apps/transformer_cluster_deletion.go index f0790b312bb..b5f827eccb1 100644 --- a/controllers/apps/transformer_cluster_deletion.go +++ b/controllers/apps/transformer_cluster_deletion.go @@ -21,13 +21,11 @@ package apps 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" "sigs.k8s.io/controller-runtime/pkg/client" @@ -166,33 +164,9 @@ func kindsForWipeOut() ([]client.ObjectList, []client.ObjectList) { // shouldSkipObjOwnedByComp is used to judge whether the object owned by component should be skipped when deleting the cluster func shouldSkipObjOwnedByComp(obj client.Object, cluster kbappsv1.Cluster) bool { + // if the object is not owned by component, it should not be skipped 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 + return ownByComp } func deleteCompNShardingInOrder4Terminate(transCtx *clusterTransformContext, dag *graph.DAG) (sets.Set[string], error) { diff --git a/controllers/apps/transformer_component_rbac.go b/controllers/apps/transformer_component_rbac.go index 46e9e4ed74a..aef556c0961 100644 --- a/controllers/apps/transformer_component_rbac.go +++ b/controllers/apps/transformer_component_rbac.go @@ -21,23 +21,21 @@ package apps import ( "fmt" - "time" + "reflect" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/types" "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,6 +44,8 @@ type componentRBACTransformer struct{} var _ graph.Transformer = &componentRBACTransformer{} +const EventReasonRBACManager = "RBACManager" + func (t *componentRBACTransformer) Transform(ctx graph.TransformContext, dag *graph.DAG) error { transCtx, _ := ctx.(*componentTransformContext) if model.IsObjectDeleting(transCtx.ComponentOrig) { @@ -57,39 +57,44 @@ 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 + // If the user has disabled rbac manager or specified comp.Spec.ServiceAccountName, it is now the user's responsibility to + // provide appropriate serviceaccount, roles and rolebindings. + if transCtx.Component.Spec.ServiceAccountName != "" { + return nil } - if serviceAccount == nil { - transCtx.Logger.V(1).Info("buildServiceAccounts returns serviceAccount nil") + if !viper.GetBool(constant.EnableRBACManager) { + transCtx.EventRecorder.Event(transCtx.Cluster, corev1.EventTypeWarning, EventReasonRBACManager, "RBAC manager is disabled") return nil } - if isServiceAccountExist(transCtx, serviceAccount.Name) { - return nil + graphCli, _ := transCtx.Client.(model.GraphClient) + synthesizedComp := transCtx.SynthesizeComponent + serviceAccountName := constant.GenerateDefaultServiceAccountName(synthesizedComp.ClusterName, synthesizedComp.Name) + + role, err := createOrUpdateRole(transCtx, serviceAccountName, graphCli, dag) + if err != nil { + return err } - 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") + rbs, err := createOrUpdateRoleBinding(transCtx, role, serviceAccountName, graphCli, dag) + if err != nil { + return err } - rb, err := buildRoleBinding(transCtx.SynthesizeComponent, transCtx.Component, serviceAccount.Name) + // 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 } - graphCli.Create(dag, rb, inDataContext4G()) - createServiceAccount(serviceAccount, graphCli, dag, rb) + // 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 { - // serviceAccount must be created before workload - graphCli.DependOn(dag, its, serviceAccount) + graphCli.DependOn(dag, its, sa) } return nil @@ -99,103 +104,91 @@ 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. +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("ServiceAccount not exists", "namespaceName", namespaceName) - return false + graphCli.Create(dag, obj, inDataContext4G()) + return obj, nil } - transCtx.Logger.Error(err, "get ServiceAccount failed") - return false + return nil, err + } + if !cmpFn(oldObj, obj) { + graphCli.Update(dag, oldObj, obj, inDataContext4G()) } - return true + return obj, nil } -func isRoleBindingExist(transCtx *componentTransformContext, serviceAccountName string) bool { +func createOrUpdateServiceAccount(transCtx *componentTransformContext, serviceAccountName string, graphCli model.GraphClient, dag *graph.DAG) (*corev1.ServiceAccount, error) { 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. - if errors.IsNotFound(err) { - transCtx.Logger.V(1).Info("RoleBinding not exists", "namespaceName", namespaceName) - return false - } - transCtx.Logger.Error(err, fmt.Sprintf("get role binding failed: %s", namespaceName)) - return false - } - if rb.RoleRef.Name != constant.RBACRoleName { - transCtx.Logger.V(1).Info("rbac manager: ClusterRole not match", "ClusterRole", - constant.RBACRoleName, "rolebinding.RoleRef", rb.RoleRef.Name) - } - - isServiceAccountMatch := false - for _, sub := range rb.Subjects { - if sub.Kind == rbacv1.ServiceAccountKind && sub.Name == serviceAccountName { - isServiceAccountMatch = true - break - } + sa := factory.BuildServiceAccount(synthesizedComp, serviceAccountName) + if err := setCompOwnershipNFinalizer(transCtx.Component, sa); err != nil { + return nil, err } - if !isServiceAccountMatch { - transCtx.Logger.V(1).Info("rbac manager: ServiceAccount not match", "ServiceAccount", - serviceAccountName, "rolebinding.Subjects", rb.Subjects) - } - return true + return createOrUpdate(transCtx, sa, graphCli, dag, func(old, new *corev1.ServiceAccount) bool { + return reflect.DeepEqual(old.ImagePullSecrets, new.ImagePullSecrets) && + reflect.DeepEqual(old.Secrets, new.Secrets) && + *old.AutomountServiceAccountToken == *new.AutomountServiceAccountToken + }) } -// 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) - } - - if isRoleBindingExist(transCtx, serviceAccountName) && isServiceAccountExist(transCtx, serviceAccountName) { +func createOrUpdateRole( + transCtx *componentTransformContext, serviceAccountName string, graphCli model.GraphClient, dag *graph.DAG, +) (*rbacv1.Role, error) { + role := factory.BuildComponentRole(transCtx.SynthesizeComponent, transCtx.CompDef, serviceAccountName) + 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 reflect.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 reflect.DeepEqual(old.Subjects, new.Subjects) && reflect.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) + 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, + ) + 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/transformer_component_rbac_test.go b/controllers/apps/transformer_component_rbac_test.go index 34fbc4afdc4..120713aa6b0 100644 --- a/controllers/apps/transformer_component_rbac_test.go +++ b/controllers/apps/transformer_component_rbac_test.go @@ -20,10 +20,14 @@ along with this program. If not, see . package apps import ( + "fmt" + "reflect" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/types" appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" @@ -34,14 +38,13 @@ import ( "github.com/apecloud/kubeblocks/pkg/controller/graph" "github.com/apecloud/kubeblocks/pkg/controller/model" 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 +54,35 @@ 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() { + init := func(enableLifecycleAction bool, enablePolicyRules bool) { By("Create a component definition") - compDefObj = testapps.NewComponentDefinitionFactory(compDefName). + 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 +91,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(cluster.Name, compName) 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 +114,72 @@ 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("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.BuildComponentRole(synthesizedComp, compDefObj, serviceAccountName) + 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, cmpdRoleBinding)).To(BeTrue()) }) }) }) diff --git a/controllers/apps/transformer_component_utils.go b/controllers/apps/transformer_component_utils.go index 47382baa6d8..aeb18f782a9 100644 --- a/controllers/apps/transformer_component_utils.go +++ b/controllers/apps/transformer_component_utils.go @@ -20,11 +20,7 @@ along with this program. If not, see . package apps 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" @@ -59,28 +55,5 @@ func skipSetCompOwnershipNFinalizer(obj client.Object) bool { } 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 f6ff2897e8e..e742c2e68e2 100644 --- a/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml +++ b/deploy/helm/crds/apps.kubeblocks.io_clusters.yaml @@ -5032,19 +5032,14 @@ 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-{cluster.name}-{component.name}", bound to a role with rules defined in ComponentDefinition's + `policyRules` field. It will also be bound to a default role named "kubeblocks-cluster-pod-role" + which is installed together with KubeBlocks. - 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, nor create RoleBinding accordingly. type: string serviceRefs: description: |- @@ -13734,19 +13729,14 @@ 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-{cluster.name}-{component.name}", bound to a role with rules defined in ComponentDefinition's + `policyRules` field. It will also be bound to a default role named "kubeblocks-cluster-pod-role" + which is installed together with KubeBlocks. - 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, nor create RoleBinding accordingly. 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 54164157d4a..3a7325a9b99 100644 --- a/deploy/helm/crds/apps.kubeblocks.io_componentdefinitions.yaml +++ b/deploy/helm/crds/apps.kubeblocks.io_componentdefinitions.yaml @@ -3949,9 +3949,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 d2bc9d66174..eceb1e27938 100644 --- a/deploy/helm/crds/apps.kubeblocks.io_components.yaml +++ b/deploy/helm/crds/apps.kubeblocks.io_components.yaml @@ -4800,18 +4800,14 @@ 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-{cluster.name}-{component.name}", bound to a role with rules defined in ComponentDefinition's + `policyRules` field. It will also be bound to a default role named "kubeblocks-cluster-pod-role" + which is installed together with KubeBlocks. - 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, nor create RoleBinding accordingly. 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 e68d69385eb..1ad99946012 100644 --- a/deploy/helm/templates/rbac/cluster_pod_required_role.yaml +++ b/deploy/helm/templates/rbac/cluster_pod_required_role.yaml @@ -14,123 +14,15 @@ aggregationRule: apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - name: kubeblocks-lorry-pod-role + name: kubeblocks-kb-agent-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: - events verbs: - create -- 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: kubeblocks-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 ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: kubeblocks-backup-pod-role - labels: - {{- include "kubeblocks.labels" . | nindent 4 }} - app.kubernetes.io/required-by: pod -rules: -- apiGroups: - - "dataprotection.kubeblocks.io" - resources: - - backups/status - verbs: - - get - - update - - patch -- apiGroups: - - "dataprotection.kubeblocks.io" - resources: - - backups - verbs: - - create - - get \ 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 bafdf2a1901..56f051499da 100644 --- a/docs/developer_docs/api-reference/cluster.md +++ b/docs/developer_docs/api-reference/cluster.md @@ -661,14 +661,12 @@ 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-{cluster.name}-{component.name}”, bound to a role with rules defined in ComponentDefinition’s +policyRules field. It will also be bound to a default role named “kubeblocks-cluster-pod-role” +which is installed together with KubeBlocks.

+

If the field is not empty, the specified ServiceAccount will be used. And KubeBlocks will not +create a ServiceAccount, nor create RoleBinding accordingly.

@@ -1432,7 +1430,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.

@@ -2701,15 +2698,12 @@ 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-{cluster.name}-{component.name}”, bound to a role with rules defined in ComponentDefinition’s +policyRules field. It will also be bound to a default role named “kubeblocks-cluster-pod-role” +which is installed together with KubeBlocks.

+

If the field is not empty, the specified ServiceAccount will be used. And KubeBlocks will not +create a ServiceAccount, nor create RoleBinding accordingly.

@@ -4655,7 +4649,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.

@@ -5406,14 +5399,12 @@ 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-{cluster.name}-{component.name}”, bound to a role with rules defined in ComponentDefinition’s +policyRules field. It will also be bound to a default role named “kubeblocks-cluster-pod-role” +which is installed together with KubeBlocks.

+

If the field is not empty, the specified ServiceAccount will be used. And KubeBlocks will not +create a ServiceAccount, nor create RoleBinding accordingly.

diff --git a/pkg/constant/pattern.go b/pkg/constant/pattern.go index 3e2255b2dde..86eb2664157 100644 --- a/pkg/constant/pattern.go +++ b/pkg/constant/pattern.go @@ -75,8 +75,8 @@ func GenerateClusterComponentEnvPattern(clusterName, compName string) string { } // GenerateDefaultServiceAccountName generates default service account name for a cluster. -func GenerateDefaultServiceAccountName(name string) string { - return fmt.Sprintf("%s-%s", KBLowerPrefix, name) +func GenerateDefaultServiceAccountName(clusterName, compName string) string { + return fmt.Sprintf("%s-%s-%s", KBLowerPrefix, clusterName, compName) } // 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 34e7a04c9c7..7c828e321bd 100644 --- a/pkg/controller/component/synthesize_component.go +++ b/pkg/controller/component/synthesize_component.go @@ -438,11 +438,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.ClusterName, synthesizeComp.Name) synthesizeComp.PodSpec.ServiceAccountName = synthesizeComp.ServiceAccountName } diff --git a/pkg/controller/factory/builder.go b/pkg/controller/factory/builder.go index 2703b999f94..f5b7ec09c4d 100644 --- a/pkg/controller/factory/builder.go +++ b/pkg/controller/factory/builder.go @@ -374,16 +374,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(constant.GetCompLabels(synthesizedComp.ClusterName, synthesizedComp.Name)). AddLabelsInMap(synthesizedComp.StaticLabels). 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, @@ -391,3 +387,13 @@ func BuildRoleBinding(synthesizedComp *component.SynthesizedComponent, saName st }). GetObject() } + +func BuildComponentRole(synthesizedComp *component.SynthesizedComponent, cmpd *appsv1.ComponentDefinition, saName string) *rbacv1.Role { + rules := cmpd.Spec.PolicyRules + if len(rules) == 0 { + return nil + } + return builder.NewRoleBuilder(synthesizedComp.Namespace, saName). + AddPolicyRules(rules). + GetObject() +} diff --git a/pkg/controller/factory/builder_test.go b/pkg/controller/factory/builder_test.go index 38ec01bb248..56ce849a516 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" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -256,7 +257,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 f19935e1c77..3d2784d2a85 100644 --- a/pkg/controller/model/graph_client.go +++ b/pkg/controller/model/graph_client.go @@ -174,7 +174,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/operations/custom/action_workload.go b/pkg/operations/custom/action_workload.go index f280a45c3a3..83e12c3a904 100644 --- a/pkg/operations/custom/action_workload.go +++ b/pkg/operations/custom/action_workload.go @@ -194,7 +194,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.Cluster.Name, w.Comp.Name)} if exists, _ := intctrlutil.CheckResourceExists(actionCtx.ReqCtx.Ctx, actionCtx.Client, saKey, &corev1.ServiceAccount{}); exists { podSpec.ServiceAccountName = saKey.Name }