Skip to content

Commit

Permalink
Enhance controller RBAC and helm charts generation
Browse files Browse the repository at this point in the history
This patch enhances the permissions for controllers with the addiotion
of multi-namespace watch mode. The controlelrs now have more
fine-grained permissions, enabling them to watch specific namespaces
exclusively.

Key changes:
- Refactored helm chart generation tooling, especially Roles and
  ClusterRoles
- Addressed a bug where the RoleBinding was incorrectly assigning the
  roles into a different namespace when the controller was installed ina
  namespace that is different fro the one it monitored.

Notable features:
- We start generating one Role/RoleBinding for each monitored namespace.
- We introduced seperate Role and ClusterRoles for the CARM feature
  (prefixed with ack-*-cache)

Less importantly we are replacing some bash-fu that used to inject RBAC
rules into Roles and ClusterRoles, with a different bash-fu that allows
reusing rules for different Roles.

Signed-off-by: Amine Hilaly <[email protected]>
  • Loading branch information
a-hilaly committed Jan 29, 2024
1 parent 3667d16 commit 57b6636
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 37 deletions.
4 changes: 3 additions & 1 deletion pkg/generate/ack/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ var (
releaseTemplatePaths = []string{
"config/controller/kustomization.yaml.tpl",
"helm/templates/cluster-role-binding.yaml.tpl",
"helm/templates/cluster-role-controller.yaml.tpl",
"helm/Chart.yaml.tpl",
"helm/values.yaml.tpl",
"helm/values.schema.json",
"helm/templates/NOTES.txt.tpl",
"helm/templates/role-reader.yaml.tpl",
"helm/templates/role-writer.yaml.tpl",
"helm/templates/_controller-role-kind-patch.yaml.tpl",
"helm/templates/caches-role.yaml.tpl",
"helm/templates/caches-role-binding.yaml.tpl",
"helm/templates/leader-election-role.yaml.tpl",
"helm/templates/leader-election-role-binding.yaml.tpl",
}
Expand Down
29 changes: 21 additions & 8 deletions scripts/build-controller-release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -245,15 +245,28 @@ controller-gen rbac:roleName="$K8S_RBAC_ROLE_NAME" paths=./... output:rbac:artif
# controller-gen rbac outputs a ClusterRole definition in a
# $config_output_dir/rbac/role.yaml file. We additionally add the ability by
# for the user to specify if they want the role to be ClusterRole or Role by specifying installation scope
# in the helm values.yaml. We do this by having a custom helm template named _controller-role-kind-patch.yaml
# which utilizes the template language and adding the auto generated rules to that template.
tail -n +7 "$helm_output_dir/templates/role.yaml" >> "$helm_output_dir/templates/_controller-role-kind-patch.yaml"

# We have some other standard Role files for a reader and writer role, so here we rename
# the `_controller-role-kind-patch.yaml ` file to `cluster-role-controller.yaml`
# to better reflect what is in that file.
mv "$helm_output_dir/templates/_controller-role-kind-patch.yaml" "$helm_output_dir/templates/cluster-role-controller.yaml"
# in the helm values.yaml.

# NOTE(a-hilaly): This is some very bad bash-fu, i'm having thoughts about rewriting this hacky code
# in Go or something else. Maybe we need to rework all our generation scripts to be more modular and
# easier to maintain.

# First we trim the first 6 lines of the role.yaml file (which is the apiVersion, kind, metadata ...)
# this will leave us the rules section of the role.yaml file. We then append the rules section to the
# _helpers-patch.yaml file which is a file that will be included in the _helpers.tpl file. This will
# allow us to use the rules section in the _helpers.tpl file to generate the correct role/clusterrole.
tail -n +7 "$helm_output_dir/templates/role.yaml" > "$helm_output_dir/templates/_helpers-patch.yaml"
helpers_patch_path="$helm_output_dir/templates/_helpers-patch.yaml"

# Some sed-fu to fill the "controller-role-rules" section. Urgh.
sed '/SEDREPLACERULES/{
r '$helpers_patch_path'
d
}' $helm_output_dir/templates/_helpers.tpl > $helm_output_dir/templates/_helpers-new.tpl
mv $helm_output_dir/templates/_helpers-new.tpl $helm_output_dir/templates/_helpers.tpl

rm "$helm_output_dir/templates/role.yaml"
rm "$helpers_patch_path"

popd 1>/dev/null

Expand Down
21 changes: 0 additions & 21 deletions templates/helm/templates/_controller-role-kind-patch.yaml.tpl

This file was deleted.

5 changes: 5 additions & 0 deletions templates/helm/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,8 @@ If release name contains chart name it will be used as a full name.
{{- define "aws.credentials.path" -}}
{{- printf "%s/%s" (include "aws.credentials.secret_mount_path" .) .Values.aws.credentials.secretKey -}}
{{- end -}}

{{/* The rules a of ClusterRole or Role */}}
{{- define "controller-role-rules" }}
SEDREPLACERULES
{{- end }}
26 changes: 26 additions & 0 deletions templates/helm/templates/caches-role-binding.yaml.tpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: ack-namespaces-cache-{{ .ServicePackageName }}-controller
roleRef:
kind: ClusterRole
apiGroup: rbac.authorization.k8s.io
name: ack-namespaces-cache-{{ .ServicePackageName }}-controller
subjects:
- kind: ServiceAccount
name: ack-{{ .ServicePackageName }}-controller
namespace: {{ "{{ .Release.Namespace }}" }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: ack-configmaps-cache-{{ .ServicePackageName }}-controller
namespace: {{ "{{ .Release.Namespace }}" }}
roleRef:
kind: Role
apiGroup: rbac.authorization.k8s.io
name: ack-configmaps-cache-{{ .ServicePackageName }}-controller
subjects:
- kind: ServiceAccount
name: ack-{{ .ServicePackageName }}-controller
namespace: {{ "{{ .Release.Namespace }}" }}
28 changes: 28 additions & 0 deletions templates/helm/templates/caches-role.yaml.tpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: ack-namespaces-cache-{{ .ServicePackageName }}-controller
rules:
- apiGroups:
- ""
resources:
- namespaces
verbs:
- get
- list
- watch
---
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: ack-configmaps-cache-{{ .ServicePackageName }}-controller
namespace: {{ "{{ .Release.Namespace }}" }}
rules:
- apiGroups:
- ""
resources:
- configmaps
verbs:
- get
- list
- watch
28 changes: 21 additions & 7 deletions templates/helm/templates/cluster-role-binding.yaml.tpl
Original file line number Diff line number Diff line change
@@ -1,21 +1,35 @@
apiVersion: rbac.authorization.k8s.io/v1
{{ "{{ if eq .Values.installScope \"cluster\" }}" }}
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: {{ "{{ include \"app.fullname\" . }}" }}
roleRef:
kind: ClusterRole
{{ "{{ else }}" }}
apiGroup: rbac.authorization.k8s.io
name: ack-{{ .ServicePackageName }}-controller
subjects:
- kind: ServiceAccount
name: {{ "{{ include \"service-account.name\" . }}" }}
namespace: {{ "{{ .Release.Namespace }}" }}
{{ "{{ else if .Values.watchNamespace }}" }}
{{ "{{ $namespaces := split \",\" .Values.watchNamespace }}" }}
{{ "{{ $fullname := include \"app.fullname\" . }}" }}
{{ "{{ $releaseNamespace := .Release.Namespace }}" }}
{{ "{{ $serviceAccountName := include \"service-account.name\" . }}" }}
{{ "{{ range $namespaces }}" }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: {{ "{{ include \"app.fullname\" . }}" }}
namespace: {{ "{{ .Release.Namespace }}" }}
name: {{ "{{ $fullname }}" }}
namespace: {{ "{{ . }}" }}
roleRef:
kind: Role
{{ "{{ end }}" }}
apiGroup: rbac.authorization.k8s.io
name: ack-{{ .ServicePackageName }}-controller
subjects:
- kind: ServiceAccount
name: {{ "{{ include \"service-account.name\" . }}" }}
namespace: {{ "{{ .Release.Namespace }}" }}
name: {{ "{{ $serviceAccountName }}" }}
namespace: {{ "{{ $releaseNamespace }}" }}
{{ "{{ end }}" }}
{{ "{{ end }}" }}
28 changes: 28 additions & 0 deletions templates/helm/templates/cluster-role-controller.yaml.tpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{{ "{{ $labels := .Values.role.labels }}" }}
{{ "{{ $rules := include \"controller-role-rules\" . }}" }}
{{ "{{ if eq .Values.installScope \"cluster\" }}" }}
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: ack-{{ .ServicePackageName }}-controller
labels:
{{ "{{- range $key, $value := $labels }}" }}
{{ "{{ $key }}: {{ $value | quote }}" }}
{{ "{{- end }}" }}
{{ "{{- $rules }}" }}
{{ "{{ else if .Values.watchNamespace }}" }}
{{ "{{ $namespaces := split \",\" .Values.watchNamespace }}" }}
{{ "{{ range $namespaces }}" }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: ack-{{ .ServicePackageName }}-controller
namespace: {{ "{{ . }}" }}
labels:
{{ "{{- range $key, $value := $labels }}" }}
{{ "{{ $key }}: {{ $value | quote }}" }}
{{ "{{- end }}" }}
{{ "{{- $rules }}" }}
{{ "{{ end }}" }}
{{ "{{ end }}" }}

0 comments on commit 57b6636

Please sign in to comment.