Skip to content

Commit e34786e

Browse files
supreme-gg-ggsupreme-gg-gg
authored
refactor(helm): remove rbac.clusterScoped, derive RBAC scope from rbac.namespaces (#57)
* namespaced rbac update with kagent Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io> * use proper helath check Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io> --------- Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
1 parent 18edd55 commit e34786e

6 files changed

Lines changed: 149 additions & 64 deletions

File tree

‎helm/kagent-tools/templates/_helpers.tpl‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,29 @@ Service account name: default when useDefaultServiceAccount is true, otherwise t
7373
{{- end }}
7474

7575
{{/*
76-
Watch namespaces - transforms list of namespaces cached by the controller into comma-separated string
77-
Removes duplicates
76+
Watch namespaces - comma-separated list for controllers that watch a subset of namespaces.
77+
Precedence: controller.watchNamespaces (explicit override) > rbac.namespaces > empty (watch all).
7878
*/}}
7979
{{- define "kagent-tools.watchNamespaces" -}}
80-
{{- $nsSet := dict }}
81-
{{- .Values.controller.watchNamespaces | default list | uniq | join "," }}
80+
{{- $ctrl := index .Values "controller" | default dict -}}
81+
{{- if index $ctrl "watchNamespaces" -}}
82+
{{- index $ctrl "watchNamespaces" | uniq | join "," -}}
83+
{{- else if and .Values.rbac .Values.rbac.namespaces -}}
84+
{{- .Values.rbac.namespaces | uniq | join "," -}}
85+
{{- end -}}
86+
{{- end }}
87+
88+
{{/*
89+
Guards on the rbac block
90+
*/}}
91+
{{- define "kagent-tools.rbac.validate" -}}
92+
{{- if and .Values.rbac (hasKey .Values.rbac "clusterScoped") -}}
93+
{{- fail "rbac.clusterScoped has been removed. Leave rbac.namespaces empty for cluster-scoped RBAC, or set rbac.namespaces=[<ns>, ...] for namespaced RBAC." -}}
94+
{{- end -}}
95+
{{- if and .Values.rbac .Values.rbac.namespaces -}}
96+
{{- $installNs := include "kagent-tools.namespace" . -}}
97+
{{- if not (has $installNs .Values.rbac.namespaces) -}}
98+
{{- fail (printf "rbac.namespaces is set but does not include the install namespace %q" $installNs) -}}
99+
{{- end -}}
100+
{{- end -}}
82101
{{- end -}}

‎helm/kagent-tools/templates/clusterrole.yaml‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -74,31 +74,17 @@
7474
- apiGroups: ["*"]
7575
resources: ["*"]
7676
verbs: ["*"]
77-
{{- if .Values.rbac.clusterScoped }}
77+
{{- if not .Values.rbac.namespaces }}
7878
- nonResourceURLs: ["*"]
7979
verbs: ["*"]
8080
{{- end }}
8181
{{- end }}
8282
{{- end -}}
8383

8484
{{- if and (not .Values.useDefaultServiceAccount) .Values.rbac.create }}
85-
{{- if .Values.rbac.clusterScoped }}
86-
apiVersion: rbac.authorization.k8s.io/v1
87-
kind: ClusterRole
88-
metadata:
89-
{{- if .Values.rbac.readOnly }}
90-
name: {{ include "kagent-tools.fullname" . }}-read-role
91-
{{- else }}
92-
name: {{ include "kagent-tools.fullname" . }}-cluster-admin-role
93-
{{- end }}
94-
labels:
95-
{{- include "kagent-tools.labels" . | nindent 4 }}
96-
rules:
97-
{{- include "kagent-tools.rules" . | nindent 2 }}
98-
99-
{{- else }}
100-
{{- $namespaces := .Values.rbac.namespaces | default (list (include "kagent-tools.namespace" .)) }}
101-
{{- range $namespace := $namespaces }}
85+
{{- include "kagent-tools.rbac.validate" . -}}
86+
{{- if .Values.rbac.namespaces }}
87+
{{- range $namespace := (.Values.rbac.namespaces | uniq | sortAlpha) }}
10288
---
10389
apiVersion: rbac.authorization.k8s.io/v1
10490
kind: Role
@@ -114,5 +100,19 @@ metadata:
114100
rules:
115101
{{- include "kagent-tools.rules" $ | nindent 2 }}
116102
{{- end }}
103+
104+
{{- else }}
105+
apiVersion: rbac.authorization.k8s.io/v1
106+
kind: ClusterRole
107+
metadata:
108+
{{- if .Values.rbac.readOnly }}
109+
name: {{ include "kagent-tools.fullname" . }}-read-role
110+
{{- else }}
111+
name: {{ include "kagent-tools.fullname" . }}-cluster-admin-role
112+
{{- end }}
113+
labels:
114+
{{- include "kagent-tools.labels" . | nindent 4 }}
115+
rules:
116+
{{- include "kagent-tools.rules" . | nindent 2 }}
117117
{{- end }}
118118
{{- end }}

‎helm/kagent-tools/templates/clusterrolebinding.yaml‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,8 @@
11
{{- if and (not .Values.useDefaultServiceAccount) .Values.rbac.create }}
2+
{{- include "kagent-tools.rbac.validate" . -}}
23

3-
{{- if .Values.rbac.clusterScoped }}
4-
apiVersion: rbac.authorization.k8s.io/v1
5-
kind: ClusterRoleBinding
6-
metadata:
7-
{{- if .Values.rbac.readOnly }}
8-
name: {{ include "kagent-tools.fullname" . }}-read-rolebinding
9-
{{- else }}
10-
name: {{ include "kagent-tools.fullname" . }}-cluster-admin-rolebinding
11-
{{- end }}
12-
labels:
13-
{{- include "kagent-tools.labels" . | nindent 4 }}
14-
roleRef:
15-
apiGroup: rbac.authorization.k8s.io
16-
kind: ClusterRole
17-
{{- if .Values.rbac.readOnly }}
18-
name: {{ include "kagent-tools.fullname" . }}-read-role
19-
{{- else }}
20-
name: {{ include "kagent-tools.fullname" . }}-cluster-admin-role
21-
{{- end }}
22-
subjects:
23-
- kind: ServiceAccount
24-
name: {{ include "kagent-tools.fullname" . }}
25-
namespace: {{ include "kagent-tools.namespace" . }}
26-
27-
{{- else }}
28-
{{- $namespaces := .Values.rbac.namespaces | default (list (include "kagent-tools.namespace" .)) }}
29-
{{- range $namespace := $namespaces }}
4+
{{- if .Values.rbac.namespaces }}
5+
{{- range $namespace := (.Values.rbac.namespaces | uniq | sortAlpha) }}
306
---
317
apiVersion: rbac.authorization.k8s.io/v1
328
kind: RoleBinding
@@ -52,6 +28,30 @@ subjects:
5228
name: {{ include "kagent-tools.fullname" $ }}
5329
namespace: {{ include "kagent-tools.namespace" $ }}
5430
{{- end }}
31+
32+
{{- else }}
33+
apiVersion: rbac.authorization.k8s.io/v1
34+
kind: ClusterRoleBinding
35+
metadata:
36+
{{- if .Values.rbac.readOnly }}
37+
name: {{ include "kagent-tools.fullname" . }}-read-rolebinding
38+
{{- else }}
39+
name: {{ include "kagent-tools.fullname" . }}-cluster-admin-rolebinding
40+
{{- end }}
41+
labels:
42+
{{- include "kagent-tools.labels" . | nindent 4 }}
43+
roleRef:
44+
apiGroup: rbac.authorization.k8s.io
45+
kind: ClusterRole
46+
{{- if .Values.rbac.readOnly }}
47+
name: {{ include "kagent-tools.fullname" . }}-read-role
48+
{{- else }}
49+
name: {{ include "kagent-tools.fullname" . }}-cluster-admin-role
50+
{{- end }}
51+
subjects:
52+
- kind: ServiceAccount
53+
name: {{ include "kagent-tools.fullname" . }}
54+
namespace: {{ include "kagent-tools.namespace" . }}
5555
{{- end }}
5656

5757
{{- end }}

‎helm/kagent-tools/templates/deployment.yaml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ spec:
106106
containerPort: {{ .Values.tools.metrics.port | default .Values.service.ports.tools.targetPort }}
107107
protocol: TCP
108108
readinessProbe:
109-
tcpSocket:
109+
httpGet:
110+
path: /health
110111
port: http-tools
111112
initialDelaySeconds: 15
112113
periodSeconds: 15

‎helm/kagent-tools/tests/rbac_test.yaml‎

Lines changed: 76 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ tests:
1212
of: ClusterRoleBinding
1313
template: clusterrolebinding.yaml
1414

15-
- it: should render Roles when clusterScoped is false
15+
- it: should render Roles when rbac.namespaces is set
1616
set:
17-
rbac.clusterScoped: false
17+
rbac.namespaces:
18+
- NAMESPACE
1819
asserts:
1920
- isKind:
2021
of: Role
@@ -31,36 +32,102 @@ tests:
3132
value: NAMESPACE
3233
template: clusterrolebinding.yaml
3334

34-
- it: should render multiple roles and bindings when namespaces are specified and clusterScoped is false
35+
- it: should render a single role/binding in the listed namespace only (no release-ns fallback)
36+
set:
37+
rbac.namespaces:
38+
- NAMESPACE
39+
asserts:
40+
- hasDocuments:
41+
count: 1
42+
template: clusterrole.yaml
43+
- equal:
44+
path: metadata.namespace
45+
value: NAMESPACE
46+
template: clusterrole.yaml
47+
- hasDocuments:
48+
count: 1
49+
template: clusterrolebinding.yaml
50+
- equal:
51+
path: metadata.namespace
52+
value: NAMESPACE
53+
template: clusterrolebinding.yaml
54+
55+
- it: should render multiple roles and bindings when namespaces are specified
3556
set:
36-
rbac.clusterScoped: false
3757
rbac.namespaces:
3858
- ns1
59+
- NAMESPACE
3960
- ns2
4061
asserts:
4162
- hasDocuments:
42-
count: 2
63+
count: 3
4364
template: clusterrole.yaml
4465
- equal:
4566
path: metadata.namespace
46-
value: ns1
67+
value: NAMESPACE
4768
template: clusterrole.yaml
4869
documentIndex: 0
4970
- equal:
5071
path: metadata.namespace
51-
value: ns2
72+
value: ns1
5273
template: clusterrole.yaml
5374
documentIndex: 1
75+
- equal:
76+
path: metadata.namespace
77+
value: ns2
78+
template: clusterrole.yaml
79+
documentIndex: 2
5480
- hasDocuments:
55-
count: 2
81+
count: 3
5682
template: clusterrolebinding.yaml
5783
- equal:
5884
path: metadata.namespace
59-
value: ns1
85+
value: NAMESPACE
6086
template: clusterrolebinding.yaml
6187
documentIndex: 0
88+
- equal:
89+
path: metadata.namespace
90+
value: ns1
91+
template: clusterrolebinding.yaml
92+
documentIndex: 1
6293
- equal:
6394
path: metadata.namespace
6495
value: ns2
6596
template: clusterrolebinding.yaml
97+
documentIndex: 2
98+
99+
- it: should fail rendering if the removed rbac.clusterScoped field is set
100+
set:
101+
rbac.clusterScoped: false
102+
template: clusterrolebinding.yaml
103+
asserts:
104+
- failedTemplate:
105+
errorMessage: "rbac.clusterScoped has been removed. Leave rbac.namespaces empty for cluster-scoped RBAC, or set rbac.namespaces=[<ns>, ...] for namespaced RBAC."
106+
107+
- it: should fail rendering if rbac.namespaces is set but does not include the install namespace
108+
set:
109+
rbac.namespaces:
110+
- some-other-ns
111+
template: clusterrolebinding.yaml
112+
asserts:
113+
- failedTemplate:
114+
errorMessage: 'rbac.namespaces is set but does not include the install namespace "NAMESPACE"'
115+
116+
- it: should accept a custom install namespace when listed in rbac.namespaces
117+
set:
118+
namespaceOverride: my-ns
119+
rbac.namespaces:
120+
- my-ns
121+
- other-ns
122+
template: clusterrole.yaml
123+
asserts:
124+
- hasDocuments:
125+
count: 2
126+
- equal:
127+
path: metadata.namespace
128+
value: my-ns
129+
documentIndex: 0
130+
- equal:
131+
path: metadata.namespace
132+
value: other-ns
66133
documentIndex: 1

‎helm/kagent-tools/values.yaml‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,9 @@ rbac:
104104
# When false, no ClusterRole or ClusterRoleBinding are created.
105105
# The ServiceAccount is still created allowing you to attach your own roles externally.
106106
create: true
107-
# -- If true, creates ClusterRole and ClusterRoleBinding resources.
108-
# If false, creates Role and RoleBinding resources instead.
109-
clusterScoped: true
110-
# -- When clusterScoped is false, specify additional namespaces to create Roles and RoleBindings in.
111-
# If empty, defaults to the release namespace.
107+
# -- Namespaces in which to create Role and RoleBinding resources.
108+
# If empty (default), the chart creates cluster-scoped ClusterRole and ClusterRoleBinding resources.
109+
# If set, the chart creates a Role + RoleBinding per listed namespace (install namespace must be included).
112110
namespaces: []
113111
# When true, deploys a read-only ClusterRole (get, list, watch) instead of cluster-admin.
114112
# Pairs well with the --read-only CLI flag which disables write operations at the application layer.

0 commit comments

Comments
 (0)