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.

Sign up for GitHub

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

chore: support define policy rules in helm values #8854

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+44 −2
Open

chore: support define policy rules in helm values #8854

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
92f742f
chore: suuport define policy rules in values
cjc7373 Jan 23, 2025
e3cbf2f
fix
cjc7373 Jan 23, 2025
2c58218
fix
cjc7373 Jan 24, 2025
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
2 changes: 2 additions & 0 deletions apis/apps/v1/componentdefinition_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,8 @@ 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.
//
// To prevent privilege escalation, only permissions already owned by Kubeblocks can be added here.
cjc7373 marked this conversation as resolved.
Show resolved Hide resolved
//
// This field is immutable.
//
// +optional
Expand Down
3 changes: 3 additions & 0 deletions config/crd/bases/apps.kubeblocks.io_componentdefinitions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8331,6 +8331,9 @@ spec:
This ensures that the Pods in the Component has appropriate permissions to function.


To prevent privilege escalation, only permissions already owned by Kubeblocks can be added here.


This field is immutable.
items:
description: |-
Expand Down
3 changes: 3 additions & 0 deletions deploy/helm/crds/apps.kubeblocks.io_componentdefinitions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8331,6 +8331,9 @@ spec:
This ensures that the Pods in the Component has appropriate permissions to function.


To prevent privilege escalation, only permissions already owned by Kubeblocks can be added here.


This field is immutable.
items:
description: |-
Expand Down
21 changes: 21 additions & 0 deletions deploy/helm/templates/rbac/rbac_manager_additaional_role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{{- if .Values.rbac.enabled }}
# Additional role that is required for addons. Can be defined by user.
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: {{ include "kubeblocks.fullname" . }}-rbac-manager-additional-role
labels:
{{- include "kubeblocks.labels" . | nindent 4 }}
rules:
# rabbitmq needs this
- apiGroups:
- ""
resources:
- endpoints
verbs:
- get
# user defined rules
{{- with .Values.rbac.additionalPolicyRules }}
{{ toYaml . }}
{{- end }}
Copy link
Contributor

Choose a reason for hiding this comment

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

why not definied in cmpd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Permissions defined in cmpd can't exceed the permission owned by kubeblocks. So if a user want to define a permission kb does not own by default, he can add it here so that kb owns it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this limitation enforced? Is it a restriction within Kubernetes itself, or does KB have validation logic (I can't find the code)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's enforced by Kubernetes. (doc: https://kubernetes.io/docs/reference/access-authn-authz/rbac/#privilege-escalation-prevention-and-bootstrapping)

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, please update PR description correspondingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should just allow the escalation by adding the escalate verb and the bind verb to the RBAC rules of KB, according to the K8s doc.

What do you think? @leon-inf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would that be too dangerous?

Copy link
Contributor

Choose a reason for hiding this comment

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

The point is:

  • It makes no sense to add additional but useless rules for KB.
  • Rules that need to be added are unpredictable.

{{- end }}
9 changes: 7 additions & 2 deletions deploy/helm/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,15 @@ fullnameOverride: ""
##
## If it is set to false, then you will need to create the service account
## named `cluster.ComponentSpec.ServiceAccountName` and the corresponding (cluster) role binding
## manually or through the cluster's Helm template, as shown in the example:
## helm install mysql apecloud-mysql-cluster
## manually.
##
## @param rbac.additionalPolicyRules
## In your Componentdefinition CR's `policyRules` field, you can only define rules kubeblocks
## already has. If you want to define a rule that kubeblocks does not have, you can add it here.
## This field is `[]rbacv1.PolicyRule`.
rbac:
enabled: true
additionalPolicyRules: []

## Deployment update strategy.
## Ref: https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy
Expand Down
2 changes: 2 additions & 0 deletions docs/developer_docs/api-reference/cluster.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1492,6 +1492,7 @@ Kubernetes resources within the namespace.</p>
<p>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.</p>
<p>To prevent privilege escalation, only permissions already owned by Kubeblocks can be added here.</p>
<p>This field is immutable.</p>
</td>
</tr>
Expand Down Expand Up @@ -5362,6 +5363,7 @@ Kubernetes resources within the namespace.</p>
<p>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.</p>
<p>To prevent privilege escalation, only permissions already owned by Kubeblocks can be added here.</p>
<p>This field is immutable.</p>
</td>
</tr>
Expand Down
Loading