feat: implement cmpd's PolicyRules #8328
Conversation
cjc7373
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8328 +/- ##
==========================================
+ Coverage 60.62% 60.78% +0.15%
==========================================
Files 376 377 +1
Lines 45405 45462 +57
==========================================
+ Hits 27528 27632 +104
+ Misses 15329 15257 -72
- Partials 2548 2573 +25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
cjc7373
force-pushed
the
feature/rbac-for-component
branch
from
8c646de
to
ea9c0a7
Compare
|
relevant addon PR: apecloud/kubeblocks-addons#1197 |
apis/apps/v1/cluster_types.go
Outdated
| // 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. |
There was a problem hiding this comment.
Should the controller still need to bind these rules defined in kubeblocks-cluster-pod-role in case they are not bound by the user?
There was a problem hiding this comment.
The rules in cmpd are the same.
There was a problem hiding this comment.
Since this field only involves the declaration of the service account (name) and does not pertain to the rules bound.
apis/apps/v1/cluster_types.go
Outdated
| // 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 |
There was a problem hiding this comment.
As a further improvement, components with the same cmp can share a default service account.
There was a problem hiding this comment.
So that the rbac provision can be moved to cmpd controller?
There was a problem hiding this comment.
I ignored a problem. Cmpd is not namespaced, but serviceaccount/rolebinding is. So that it is not feasible to move rbac provision to cmpd controller. Cmpd controller can watch a component to trigger reconcile. The reconcilation related to rbac may look like this:
- It lists all components that references the cmpd. Find all the namespaces of them (list 1).
- It lists all managed rbac resources (serviceaccount/rolebinding/role) using label selector. Find all the namespaces of them (list 2).
- Create/update rbac resources in list 1.
- Delete rbac resouces in list 2 but not in list 1.
github-actions
bot
added
size/XXL
|
After a second thought, rbac provision will remain in component controller. These rbac resources will use the component which first creates them as their owner. If the component has been deleted, their owner will be transfered to another component which shares a same cmpd. If no such component can be found, they will be deleted. Another note: equality.Semantic is used to replace reflect.DeepEqual to avoid the false alarm of something like |
Fixes #8310. Things done in this PR:
serviceAccountNamein cluster and component CR. KB now does not create rbac resources if user has specified a service account.kb-<clusterName>-<compName>.Addon changes (like update pg addon's cmpd policyRule since we removed kubeblocks-patroni-pod-role) will be addressed in apecloud/kubeblocks-addons#1197.