Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement cmpd's PolicyRules #8328

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions apis/apps/v1/cluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
15 changes: 6 additions & 9 deletions apis/apps/v1/component_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
2 changes: 0 additions & 2 deletions apis/apps/v1/componentdefinition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 12 additions & 22 deletions config/crd/bases/apps.kubeblocks.io_clusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |-
Expand Down Expand Up @@ -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: |-
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: |-
Expand Down
16 changes: 6 additions & 10 deletions config/crd/bases/apps.kubeblocks.io_components.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |-
Expand Down
3 changes: 0 additions & 3 deletions controllers/apps/component_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
74 changes: 36 additions & 38 deletions controllers/apps/component_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand All @@ -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) {
Expand Down Expand Up @@ -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() {
Expand Down
30 changes: 2 additions & 28 deletions controllers/apps/transformer_cluster_deletion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove shouldSkipObjOwnedByComp and use isOwnedByComp directly

// 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) {
Expand Down
Loading
Loading