From 57b6636e0e950f44129e3d9b3d074c639849585f Mon Sep 17 00:00:00 2001 From: Amine Hilaly Date: Sun, 28 Jan 2024 19:26:57 -0600 Subject: [PATCH] Enhance controller RBAC and helm charts generation 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 --- pkg/generate/ack/release.go | 4 ++- scripts/build-controller-release.sh | 29 ++++++++++++++----- .../_controller-role-kind-patch.yaml.tpl | 21 -------------- templates/helm/templates/_helpers.tpl | 5 ++++ .../templates/caches-role-binding.yaml.tpl | 26 +++++++++++++++++ templates/helm/templates/caches-role.yaml.tpl | 28 ++++++++++++++++++ .../templates/cluster-role-binding.yaml.tpl | 28 +++++++++++++----- .../cluster-role-controller.yaml.tpl | 28 ++++++++++++++++++ 8 files changed, 132 insertions(+), 37 deletions(-) delete mode 100644 templates/helm/templates/_controller-role-kind-patch.yaml.tpl create mode 100644 templates/helm/templates/caches-role-binding.yaml.tpl create mode 100644 templates/helm/templates/caches-role.yaml.tpl create mode 100644 templates/helm/templates/cluster-role-controller.yaml.tpl diff --git a/pkg/generate/ack/release.go b/pkg/generate/ack/release.go index f3f6ed76..8c341d02 100644 --- a/pkg/generate/ack/release.go +++ b/pkg/generate/ack/release.go @@ -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", } diff --git a/scripts/build-controller-release.sh b/scripts/build-controller-release.sh index e3cc25d5..88ece078 100755 --- a/scripts/build-controller-release.sh +++ b/scripts/build-controller-release.sh @@ -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 diff --git a/templates/helm/templates/_controller-role-kind-patch.yaml.tpl b/templates/helm/templates/_controller-role-kind-patch.yaml.tpl deleted file mode 100644 index 39e5bf02..00000000 --- a/templates/helm/templates/_controller-role-kind-patch.yaml.tpl +++ /dev/null @@ -1,21 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -{{ "{{ if eq .Values.installScope \"cluster\" }}" }} -kind: ClusterRole -metadata: - creationTimestamp: null - name: ack-{{ .ServicePackageName }}-controller - labels: - {{ "{{- range $key, $value := .Values.role.labels }}" }} - {{ "{{ $key }}: {{ $value | quote }}" }} - {{ "{{- end }}" }} -{{ "{{ else }}" }} -kind: Role -metadata: - creationTimestamp: null - name: ack-{{ .ServicePackageName }}-controller - labels: - {{ "{{- range $key, $value := .Values.role.labels }}" }} - {{ "{{ $key }}: {{ $value | quote }}" }} - {{ "{{- end }}" }} - namespace: {{ "{{ .Release.Namespace }}" }} -{{ "{{ end }}" }} diff --git a/templates/helm/templates/_helpers.tpl b/templates/helm/templates/_helpers.tpl index 391d5de3..b43ccc4b 100644 --- a/templates/helm/templates/_helpers.tpl +++ b/templates/helm/templates/_helpers.tpl @@ -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 }} \ No newline at end of file diff --git a/templates/helm/templates/caches-role-binding.yaml.tpl b/templates/helm/templates/caches-role-binding.yaml.tpl new file mode 100644 index 00000000..34b5f362 --- /dev/null +++ b/templates/helm/templates/caches-role-binding.yaml.tpl @@ -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 }}" }} \ No newline at end of file diff --git a/templates/helm/templates/caches-role.yaml.tpl b/templates/helm/templates/caches-role.yaml.tpl new file mode 100644 index 00000000..6e58d1bf --- /dev/null +++ b/templates/helm/templates/caches-role.yaml.tpl @@ -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 \ No newline at end of file diff --git a/templates/helm/templates/cluster-role-binding.yaml.tpl b/templates/helm/templates/cluster-role-binding.yaml.tpl index 36961fec..53e3f480 100644 --- a/templates/helm/templates/cluster-role-binding.yaml.tpl +++ b/templates/helm/templates/cluster-role-binding.yaml.tpl @@ -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 }}" }} \ No newline at end of file diff --git a/templates/helm/templates/cluster-role-controller.yaml.tpl b/templates/helm/templates/cluster-role-controller.yaml.tpl new file mode 100644 index 00000000..b1cad2ae --- /dev/null +++ b/templates/helm/templates/cluster-role-controller.yaml.tpl @@ -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 }}" }} \ No newline at end of file