From f3f619386b81e1857e1c8d1edf275902345c8ef6 Mon Sep 17 00:00:00 2001
From: "J. Conrad Hanson" <27842017+conradhanson@users.noreply.github.com>
Date: Mon, 11 Sep 2023 16:44:47 -0400
Subject: [PATCH] create Roles & RoleBindings (#481)

* create either Cluster-scoped or ns-scoped rbac, not both

* update tests

* changelog

* dont regress operator api; add cluster-scoping to NamespaceRbac based on 'watchNamespaces' helm api

* update tests

* update changelog

* update rbac template and operator and sidecar api

* move changelog

* update Resource.ClusterScoped explanation

* Adding changelog file to new location

* Deleting changelog file from old location

* update operator rbac template from v0.32.x branch

* update operator.NamespaceRbac and rbac template

* update cmd test

* Adding changelog file to new location

* Deleting changelog file from old location

* update changelog

* add check to rbac template to validate user-specified namespaced resources

* Adding changelog file to new location

* Deleting changelog file from old location

* fix operator deployment template

* pr feedback: add release name and ns to rbac tmpl clusterrole/binding

* add NamespaceRbac to sidecar

* generate

* move resource-to-namespaces map to helper func

* generate

* dont quote

---------

Co-authored-by: changelog-bot <changelog-bot>
---
 changelog/v0.34.3/ns-rbac-by-helm-flag.yaml   |   6 +
 codegen/cmd_test.go                           | 115 ++++++++++++------
 codegen/model/chart.go                        |   6 +-
 codegen/model/resource.go                     |   2 +-
 codegen/render/funcs.go                       |   6 +-
 codegen/templates/chart/_helpers.tpl          |  15 +++
 .../chart/operator-deployment.yamltmpl        |   5 +-
 .../templates/chart/operator-rbac.yamltmpl    | 111 +++++++++++++----
 .../test/chart-envvars/templates/_helpers.tpl |   2 +-
 .../chart-envvars/templates/deployment.yaml   |   1 -
 .../test/chart-envvars/templates/rbac.yaml    |   1 -
 .../test/chart-no-desc/templates/_helpers.tpl |   2 +-
 .../chart-no-desc/templates/deployment.yaml   |   1 -
 .../test/chart-no-desc/templates/rbac.yaml    |   1 -
 .../templates/_helpers.tpl                    |   2 +-
 .../templates/deployment.yaml                 |   1 -
 .../chart-sidecar-svcport/templates/rbac.yaml |   1 -
 .../test/chart-sidecar/templates/_helpers.tpl |   2 +-
 .../chart-sidecar/templates/deployment.yaml   |   1 -
 .../test/chart-sidecar/templates/rbac.yaml    |   1 -
 .../test/chart-svcport/templates/_helpers.tpl |   2 +-
 .../chart-svcport/templates/deployment.yaml   |   1 -
 .../test/chart-svcport/templates/rbac.yaml    |   1 -
 .../templates/_helpers.tpl                    |   2 +-
 .../templates/deployment.yaml                 |   2 -
 .../conditional-sidecar/templates/rbac.yaml   |  14 ++-
 codegen/test/chart/templates/_helpers.tpl     |  10 ++
 codegen/test/chart/templates/deployment.yaml  |   1 -
 codegen/test/chart/templates/rbac.yaml        |  82 +++++++++++--
 .../templates/_helpers.tpl                    |   2 +-
 .../templates/deployment.yaml                 |   1 -
 .../name_override_chart/templates/rbac.yaml   |   1 -
 32 files changed, 298 insertions(+), 103 deletions(-)
 create mode 100644 changelog/v0.34.3/ns-rbac-by-helm-flag.yaml

diff --git a/changelog/v0.34.3/ns-rbac-by-helm-flag.yaml b/changelog/v0.34.3/ns-rbac-by-helm-flag.yaml
new file mode 100644
index 000000000..a6955c253
--- /dev/null
+++ b/changelog/v0.34.3/ns-rbac-by-helm-flag.yaml
@@ -0,0 +1,6 @@
+changelog:
+  - type: NEW_FEATURE
+    issueLink:   https://github.com/solo-io/gloo-mesh-enterprise/issues/10521
+    description: >
+      Add the ability to toggle between generating a ClusterRole/Binding or Role/Binding for namespace-scoped rbac policies.
+    resolvesIssue: false
\ No newline at end of file
diff --git a/codegen/cmd_test.go b/codegen/cmd_test.go
index 0df3bc545..1219a2068 100644
--- a/codegen/cmd_test.go
+++ b/codegen/cmd_test.go
@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -82,7 +81,7 @@ var _ = Describe("Cmd", func() {
 											},
 										},
 									},
-									Rbac: []rbacv1.PolicyRule{{
+									ClusterRbac: []rbacv1.PolicyRule{{
 										Verbs:     []string{"*"},
 										APIGroups: []string{"apiextensions.k8s.io"},
 										Resources: []string{"customresourcedefinitions"},
@@ -1621,7 +1620,7 @@ roleRef:
 			err := cmd.Execute()
 			Expect(err).NotTo(HaveOccurred())
 
-			bytes, err := ioutil.ReadFile(crdFilePath)
+			bytes, err := os.ReadFile(crdFilePath)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(string(bytes)).To(ContainSubstring("description: OpenAPI gen test for recursive fields"))
 		})
@@ -1633,7 +1632,7 @@ roleRef:
 			err := cmd.Execute()
 			Expect(err).NotTo(HaveOccurred())
 
-			bytes, err := ioutil.ReadFile(crdFilePath)
+			bytes, err := os.ReadFile(crdFilePath)
 			Expect(err).NotTo(HaveOccurred())
 			paintCrdYaml := ""
 			for _, crd := range strings.Split(string(bytes), "---") {
@@ -1663,7 +1662,7 @@ roleRef:
 			err := cmd.Execute()
 			Expect(err).NotTo(HaveOccurred())
 
-			bytes, err := ioutil.ReadFile(crdFilePath)
+			bytes, err := os.ReadFile(crdFilePath)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(string(bytes)).NotTo(ContainSubstring("description:"))
 		})
@@ -2001,11 +2000,13 @@ roleRef:
 								Verbs: []string{"GET"},
 							},
 						},
-						NamespaceRbac: []rbacv1.PolicyRule{
-							{
-								Verbs:     []string{"GET", "LIST", "WATCH"},
-								APIGroups: []string{""},
-								Resources: []string{"secrets"},
+						NamespaceRbac: map[string][]rbacv1.PolicyRule{
+							"secrets": {
+								rbacv1.PolicyRule{
+									Verbs:     []string{"GET", "LIST", "WATCH"},
+									APIGroups: []string{""},
+									Resources: []string{"secrets"},
+								},
 							},
 						},
 					},
@@ -2022,7 +2023,6 @@ roleRef:
 					},
 				},
 			},
-
 			ManifestRoot: "codegen/test/chart",
 		}
 
@@ -2033,15 +2033,40 @@ roleRef:
 
 		rbac, err := os.ReadFile(absPath)
 		Expect(err).NotTo(HaveOccurred(), "failed to read rbac.yaml")
-		roleTmpl := `
-kind: Role
+		clusterRole1Tmpl := `
+kind: ClusterRole
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: painter-{{ default .Release.Namespace $painter.namespace }}
+  labels:
+    app: painter
+rules:
+- verbs:
+  - GET`
+		clusterRoleBinding1Tmpl := `
+kind: ClusterRoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
+  name: painter-{{ default .Release.Namespace $painter.namespace }}
+  labels:
+    app: painter
+subjects:
+- kind: ServiceAccount
   name: painter
-  namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
+  namespace: {{ default .Release.Namespace $painter.namespace }}
+roleRef:
+  kind: ClusterRole
+  name: painter-{{ default .Release.Namespace $painter.namespace }}
+  apiGroup: rbac.authorization.k8s.io`
+		clusterRole2Tmpl := `
+kind: ClusterRole
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: painter-{{ .Release.Name }}-{{ .Release.Namespace }}
   labels:
     app: painter
 rules:
+{{- if not (has "secrets" $painterNamespacedResources) }}
 - apiGroups:
   - ""
   resources:
@@ -2049,52 +2074,64 @@ rules:
   verbs:
   - GET
   - LIST
-  - WATCH`
-		roleBindingTmpl := `
-kind: RoleBinding
+  - WATCH
+{{- end }}`
+		clusterRoleBinding2Tmpl := `
+kind: ClusterRoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
-  name: painter
-  namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
+  name: painter-{{ .Release.Name }}-{{ .Release.Namespace }}
   labels:
     app: painter
 subjects:
 - kind: ServiceAccount
   name: painter
-  namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
+  namespace: {{ default .Release.Namespace $painter.namespace }}
 roleRef:
-  kind: Role
-  name: painter
+  kind: ClusterRole
+  name: painter-{{ .Release.Name }}-{{ .Release.Namespace }}
   apiGroup: rbac.authorization.k8s.io`
-		clusterRoleTmpl := `
-kind: ClusterRole
+		roleTmpl := `
+kind: Role
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
-  name: painter-{{ default .Release.Namespace $.Values.painter.namespace }}
+  name: painter
+  namespace: {{ $ns }}
   labels:
     app: painter
 rules:
-- verbs:
-  - GET`
-		clusterRoleBindingTmpl := `
-kind: ClusterRoleBinding
+{{- if (has "secrets" $resources) }}
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - GET
+  - LIST
+  - WATCH
+{{- end }}`
+		roleBindingTmpl := `
+kind: RoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
-  name: painter-{{ default .Release.Namespace $.Values.painter.namespace }}
+  name: painter
+  namespace: {{ $ns }}
   labels:
     app: painter
 subjects:
 - kind: ServiceAccount
   name: painter
-  namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
+  namespace: {{ default $.Release.Namespace $painter.namespace }}
 roleRef:
-  kind: ClusterRole
-  name: painter-{{ default .Release.Namespace $.Values.painter.namespace }}
+  kind: Role
+  name: painter
   apiGroup: rbac.authorization.k8s.io`
-		Expect(rbac).To(ContainSubstring(roleTmpl))
-		Expect(rbac).To(ContainSubstring(roleBindingTmpl))
-		Expect(rbac).To(ContainSubstring(clusterRoleTmpl))
-		Expect(rbac).To(ContainSubstring(clusterRoleBindingTmpl))
+		Expect(string(rbac)).To(ContainSubstring(clusterRole1Tmpl))
+		Expect(string(rbac)).To(ContainSubstring(clusterRoleBinding1Tmpl))
+		Expect(string(rbac)).To(ContainSubstring(clusterRole2Tmpl))
+		Expect(string(rbac)).To(ContainSubstring(clusterRoleBinding2Tmpl))
+		Expect(string(rbac)).To(ContainSubstring(roleTmpl))
+		Expect(string(rbac)).To(ContainSubstring(roleBindingTmpl))
 	})
 })
 
@@ -2102,7 +2139,7 @@ func helmTemplate(path string, values interface{}) []byte {
 	raw, err := yaml.Marshal(values)
 	ExpectWithOffset(1, err).NotTo(HaveOccurred())
 
-	helmValuesFile, err := ioutil.TempFile("", "-helm-values-skv2-test")
+	helmValuesFile, err := os.CreateTemp("", "-helm-values-skv2-test")
 	ExpectWithOffset(1, err).NotTo(HaveOccurred())
 
 	_, err = helmValuesFile.Write(raw)
@@ -2123,7 +2160,7 @@ func helmTemplate(path string, values interface{}) []byte {
 }
 
 func helmValuesFromFile(path string) map[string]interface{} {
-	data, err := ioutil.ReadFile(path)
+	data, err := os.ReadFile(path)
 	Expect(err).NotTo(HaveOccurred())
 
 	out := make(map[string]interface{})
diff --git a/codegen/model/chart.go b/codegen/model/chart.go
index 4a482d889..71ef4e76b 100644
--- a/codegen/model/chart.go
+++ b/codegen/model/chart.go
@@ -79,7 +79,8 @@ type Operator struct {
 	ClusterRbac []rbacv1.PolicyRule
 
 	// these populate the generated Role for the operator
-	NamespaceRbac []rbacv1.PolicyRule
+	// key should be the k8s resource name (lower-case, plural version)
+	NamespaceRbac map[string][]rbacv1.PolicyRule
 
 	// if at least one port is defined, create a Service for it
 	Service Service
@@ -151,7 +152,8 @@ type ReadinessProbe struct {
 type Sidecar struct {
 	Container
 	Service
-	Rbac            []rbacv1.PolicyRule
+	ClusterRbac     []rbacv1.PolicyRule
+	NamespaceRbac   map[string][]rbacv1.PolicyRule
 	Volumes         []corev1.Volume
 	Name            string
 	EnableStatement string `json:"enableStatement,omitempty" yaml:"enableStatement,omitempty"` // Optional: if specified, the operator resources will be abled based on the condition specified in the enable statement.
diff --git a/codegen/model/resource.go b/codegen/model/resource.go
index e508a6008..1a24831cd 100644
--- a/codegen/model/resource.go
+++ b/codegen/model/resource.go
@@ -169,7 +169,7 @@ type Resource struct {
 	Status *Field
 
 	// Whether or not the resource is cluster-scoped.
-	// This is important when rendering the CustomResourceDefinition manifest.
+	// This is important when rendering the CustomResourceDefinition manifest and RBAC policies.
 	ClusterScoped bool
 
 	// Set the short name of the resource
diff --git a/codegen/render/funcs.go b/codegen/render/funcs.go
index b3d14974e..2c239ab2c 100644
--- a/codegen/render/funcs.go
+++ b/codegen/render/funcs.go
@@ -161,7 +161,8 @@ func toListItem(item interface{}) []interface{} {
 type containerConfig struct {
 	model.Container
 	model.Service
-	Rbac            []rbacv1.PolicyRule
+	ClusterRbac     []rbacv1.PolicyRule
+	NamespaceRbac   map[string][]rbacv1.PolicyRule
 	Volumes         []corev1.Volume
 	Name            string
 	ValuesVar       string
@@ -179,7 +180,8 @@ func containerConfigs(op model.Operator) []containerConfig {
 	for _, sidecar := range op.Deployment.Sidecars {
 		config := containerConfig{
 			EnableStatement: sidecar.EnableStatement, // Change this to base name of operator e.g: $.Values.glooAgent.X
-			Rbac:            sidecar.Rbac,
+			ClusterRbac:     sidecar.ClusterRbac,
+			NamespaceRbac:   sidecar.NamespaceRbac,
 			Volumes:         sidecar.Volumes,
 			Service:         sidecar.Service,
 			Container:       sidecar.Container,
diff --git a/codegen/templates/chart/_helpers.tpl b/codegen/templates/chart/_helpers.tpl
index 792c08a61..5b424ab4b 100644
--- a/codegen/templates/chart/_helpers.tpl
+++ b/codegen/templates/chart/_helpers.tpl
@@ -53,3 +53,18 @@ version, which merges two named templates.
 {{- toYaml $merged -}} {{/* render source with overrides as YAML */}}
 {{- end -}}
 {{- end -}}
+
+[[- range $operator := $.Operators ]]
+    [[- if $operator.NamespaceRbac ]]
+
+{{- define "[[ (lower_camel $operator.Name) ]].namespacesForResource" }}
+{{- $resourcesToNamespaces := dict }}
+{{- range $entry := [[ (opVar $operator) ]].namespacedRbac }}
+  {{- range $resource := $entry.resources }}
+    {{- $_ := set $resourcesToNamespaces $resource (concat $entry.namespaces (get $resourcesToNamespaces $resource | default list) | mustUniq) }}
+  {{- end }}
+{{- end }}
+{{- get $resourcesToNamespaces  .Resource | join "," }}
+{{- end }}
+    [[- end ]]
+[[- end ]]
\ No newline at end of file
diff --git a/codegen/templates/chart/operator-deployment.yamltmpl b/codegen/templates/chart/operator-deployment.yamltmpl
index 36c3d699b..f7239de19 100644
--- a/codegen/templates/chart/operator-deployment.yamltmpl
+++ b/codegen/templates/chart/operator-deployment.yamltmpl
@@ -1,8 +1,8 @@
-[[/*
+[[- /*
 This template contains the core components for the Operator deployment.
 Expressions evaluating Helm Values use "{{" and "}}"
 Expressions evaluating SKv2 Config use "[[" and "]]"
-*/]]
+*/ -]]
 
 [[- range $operator := $.Operators -]]
 [[- $operatorVar := (lower_camel $operator.Name) -]]
@@ -29,7 +29,6 @@ Expressions evaluating SKv2 Config use "[[" and "]]"
 [[- if $operator.Deployment.UseDaemonSet ]]
 [[- $workloadKind = "DaemonSet" ]]
 [[- end ]]
-
 # [[ $workloadKind ]] manifest for [[ $operator.Name ]]
 
 apiVersion: apps/v1
diff --git a/codegen/templates/chart/operator-rbac.yamltmpl b/codegen/templates/chart/operator-rbac.yamltmpl
index 4ce458357..1d08bc7ae 100644
--- a/codegen/templates/chart/operator-rbac.yamltmpl
+++ b/codegen/templates/chart/operator-rbac.yamltmpl
@@ -1,15 +1,15 @@
-[[/*
+[[- /*
 This template contains the RBAC config required by the Operator deployment.
 Expressions evaluating Helm Values use "{{" and "}}"
 Expressions evaluating SKv2 Config use [[ "[[" ]] and [[ "]]" ]]
-*/]]
+*/ -]]
 
 [[- range $operator := $.Operators -]]
 [[- $operatorVar := (lower_camel $operator.Name) -]]
 [[- if or $operator.ClusterRbac $operator.NamespaceRbac ]]
 # Rbac manifests for [[ $operator.Name ]]
 
-{{- $[[ $operatorVar ]] := [[ (opVar $operator)]] }}
+{{- $[[ $operatorVar ]] := [[ (opVar $operator) ]] }}
 
 [[- $operatorEnabledCondition := printf "\n{{ if $%s.enabled }}\n" $operatorVar -]]
 [[- if (gt (len $operator.CustomEnableCondition) 0) -]]
@@ -17,6 +17,7 @@ Expressions evaluating SKv2 Config use [[ "[[" ]] and [[ "]]" ]]
 [[- end -]]
 [[- $operatorEnabledCondition -]]
 [[- if $operator.ClusterRbac ]]
+
 ---
 
 kind: ClusterRole
@@ -25,23 +26,24 @@ metadata:
 [[- if $operator.NamespaceFromValuePath ]]
   name: [[ $operator.Name ]]-[[ printf "{{ %s | default $.Release.Namespace }}" $operator.NamespaceFromValuePath ]]
 [[- else ]]
-  name: [[ $operator.Name ]]-{{ default .Release.Namespace [[ (opVar $operator) ]].namespace }}
+  name: [[ $operator.Name ]]-{{ default .Release.Namespace $[[ $operatorVar ]].namespace }}
 [[- end ]]
   labels:
     app: [[ $operator.Name ]]
 rules:
 [[ toYaml $operator.ClusterRbac ]]
 [[- range $container := containerConfigs $operator -]]
-[[- if and ($container.Rbac) (gt (len $container.EnableStatement) 0) ]]
+[[- if and ($container.ClusterRbac) (gt (len $container.EnableStatement) 0) ]]
 [[ printf "{{ if %s }}" $container.EnableStatement ]]
 [[- end ]]
-[[- range $rule := $container.Rbac ]]
+[[- range $rule := $container.ClusterRbac ]]
 [[ toYaml (toListItem $rule) ]]
 [[- end ]]
-[[- if and ($container.Rbac) (gt (len $container.EnableStatement) 0) ]]
+[[- if and ($container.ClusterRbac) (gt (len $container.EnableStatement) 0) ]]
 {{- end }}
 [[- end ]]
 [[- end ]]
+
 ---
 
 kind: ClusterRoleBinding
@@ -50,7 +52,7 @@ metadata:
 [[- if $operator.NamespaceFromValuePath ]]
   name: [[ $operator.Name ]]-[[ printf "{{ %s | default $.Release.Namespace }}" $operator.NamespaceFromValuePath ]]
 [[- else ]]
-  name: [[ $operator.Name ]]-{{ default .Release.Namespace [[ (opVar $operator) ]].namespace }}
+  name: [[ $operator.Name ]]-{{ default .Release.Namespace $[[ $operatorVar ]].namespace }}
 [[- end ]]
   labels:
     app: [[ $operator.Name ]]
@@ -60,35 +62,105 @@ subjects:
 [[- if $operator.NamespaceFromValuePath ]]
   namespace: [[ printf "{{ %s | default $.Release.Namespace }}" $operator.NamespaceFromValuePath ]]
 [[- else ]]
-  namespace: {{ default .Release.Namespace [[ (opVar $operator) ]].namespace }}
+  namespace: {{ default .Release.Namespace $[[ $operatorVar ]].namespace }}
 [[- end ]]
 roleRef:
   kind: ClusterRole
 [[- if $operator.NamespaceFromValuePath ]]
   name: [[ $operator.Name ]]-[[ printf "{{ %s | default $.Release.Namespace }}" $operator.NamespaceFromValuePath ]]
 [[- else ]]
-  name: [[ $operator.Name ]]-{{ default .Release.Namespace [[ (opVar $operator) ]].namespace }}
+  name: [[ $operator.Name ]]-{{ default .Release.Namespace $[[ $operatorVar ]].namespace }}
 [[- end ]]
   apiGroup: rbac.authorization.k8s.io
 
 [[- end ]][[/* if $operator.ClusterRbac */]]
 
-[[- if $operator.NamespaceRbac ]]
+[[ if $operator.NamespaceRbac ]]
+[[- /* 
+We need the following variables:
+  1. map of namespace -> list of resources
+  2. list of resources to be namespace restricted
+  3. list of namespaced resources that the operator supports
+*/ -]]
+{{- $[[ $operatorVar ]]NsToResources := dict }}
+{{- $[[ $operatorVar ]]NamespacedResources := list }}
+{{- $[[ $operatorVar ]]SupportedResources := list }}
+[[- range $resource, $_ := $operator.NamespaceRbac ]]
+{{- $[[ $operatorVar ]]SupportedResources = append $[[ $operatorVar ]]SupportedResources [[ quote $resource ]] }}
+[[- end ]]
+
+{{- range $entry := $[[ $operatorVar ]].namespacedRbac }}
+  {{- range $ns := $entry.namespaces }}
+    {{- set $[[ $operatorVar ]]NsToResources $ns (concat $entry.resources (get $[[ $operatorVar ]]NsToResources $ns | default list) | mustUniq) }}
+  {{- end }}
+  {{- range $resource := $entry.resources }}
+    {{- if (has $resource $[[ $operatorVar ]]SupportedResources) }}
+      {{- $[[ $operatorVar ]]NamespacedResources = (append $[[ $operatorVar ]]NamespacedResources $resource | mustUniq) }}
+    {{- else }}
+      {{- fail (cat "invalid resource name" (quote $resource) "provided to namespacedRbac field. must be one of the following:" ($[[ $operatorVar ]]SupportedResources | join ", ")) }}
+    {{- end }}
+  {{- end }}
+{{- end }}
+
+{{- if not (eq (len $[[ $operatorVar ]]NamespacedResources) [[ len $operator.NamespaceRbac ]]) }}
 ---
 
-kind: Role
+kind: ClusterRole
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: [[ $operator.Name ]]-{{ .Release.Name }}-{{ .Release.Namespace }}
+  labels:
+    app: [[ $operator.Name ]]
+rules:
+[[- range $resource, $policies := $operator.NamespaceRbac ]]
+{{- if not (has [[ quote $resource ]] $[[ $operatorVar ]]NamespacedResources) }}
+[[- range $policy := $policies ]]
+[[ toYaml (toListItem $policy) ]]
+[[- end ]]
+{{- end }}
+[[- end ]]
+
+---
+
+kind: ClusterRoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
+  name: [[ $operator.Name ]]-{{ .Release.Name }}-{{ .Release.Namespace }}
+  labels:
+    app: [[ $operator.Name ]]
+subjects:
+- kind: ServiceAccount
   name: [[ $operator.Name ]]
 [[- if $operator.NamespaceFromValuePath ]]
   namespace: [[ printf "{{ %s | default $.Release.Namespace }}" $operator.NamespaceFromValuePath ]]
 [[- else ]]
-  namespace: {{ default .Release.Namespace [[ (opVar $operator) ]].namespace }}
+  namespace: {{ default .Release.Namespace $[[ $operatorVar ]].namespace }}
 [[- end ]]
+roleRef:
+  kind: ClusterRole
+  name: [[ $operator.Name ]]-{{ .Release.Name }}-{{ .Release.Namespace }}
+  apiGroup: rbac.authorization.k8s.io
+{{- end }}
+
+{{- range $ns, $resources := $[[ $operatorVar ]]NsToResources }}
+
+---
+
+kind: Role
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: [[ $operator.Name ]]
+  namespace: {{ $ns }}
   labels:
     app: [[ $operator.Name ]]
 rules:
-[[ toYaml $operator.NamespaceRbac ]]
+[[- range $resource, $policies := $operator.NamespaceRbac ]]
+{{- if (has [[ quote $resource ]] $resources) }}
+[[- range $policy := $policies ]]
+[[ toYaml (toListItem $policy) ]]
+[[- end ]]
+{{- end }}
+[[- end ]]
 
 ---
 
@@ -96,11 +168,7 @@ kind: RoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
   name: [[ $operator.Name ]]
-[[- if $operator.NamespaceFromValuePath ]]
-  namespace: [[ printf "{{ %s | default $.Release.Namespace }}" $operator.NamespaceFromValuePath ]]
-[[- else ]]
-  namespace: {{ default .Release.Namespace [[ (opVar $operator) ]].namespace }}
-[[- end ]]
+  namespace: {{ $ns }}
   labels:
     app: [[ $operator.Name ]]
 subjects:
@@ -109,14 +177,15 @@ subjects:
 [[- if $operator.NamespaceFromValuePath ]]
   namespace: [[ printf "{{ %s | default $.Release.Namespace }}" $operator.NamespaceFromValuePath ]]
 [[- else ]]
-  namespace: {{ default .Release.Namespace [[ (opVar $operator) ]].namespace }}
+  namespace: {{ default $.Release.Namespace $[[ $operatorVar ]].namespace }}
 [[- end ]]
 roleRef:
   kind: Role
   name: [[ $operator.Name ]]
   apiGroup: rbac.authorization.k8s.io
 
+  {{- end }}[[/* range $ns, $resources := $[[ $operatorVar ]]NsToResources */]]
 [[- end ]][[/* if $operator.NamespaceRbac */]]
 {{- end }}[[/* $operatorEnabledCondition */]]
 [[- end ]][[/* if or $operator.ClusterRbac $operator.NamespaceRbac */]]
-[[- end ]][[/* range $operator := $.Operators */]]
+[[- end ]][[/* range $operator := $.Operators */]]
\ No newline at end of file
diff --git a/codegen/test/chart-envvars/templates/_helpers.tpl b/codegen/test/chart-envvars/templates/_helpers.tpl
index 4d4e2732b..0c155a127 100644
--- a/codegen/test/chart-envvars/templates/_helpers.tpl
+++ b/codegen/test/chart-envvars/templates/_helpers.tpl
@@ -51,4 +51,4 @@ version, which merges two named templates.
 {{- $merged := merge $overrides $tpl -}}
 {{- toYaml $merged -}} {{/* render source with overrides as YAML */}}
 {{- end -}}
-{{- end -}}
+{{- end -}}
\ No newline at end of file
diff --git a/codegen/test/chart-envvars/templates/deployment.yaml b/codegen/test/chart-envvars/templates/deployment.yaml
index eb23a7836..21477911a 100644
--- a/codegen/test/chart-envvars/templates/deployment.yaml
+++ b/codegen/test/chart-envvars/templates/deployment.yaml
@@ -6,7 +6,6 @@
 ---
 
 {{- define "painter.deploymentSpec" }}
-
 # Deployment manifest for painter
 
 apiVersion: apps/v1
diff --git a/codegen/test/chart-envvars/templates/rbac.yaml b/codegen/test/chart-envvars/templates/rbac.yaml
index 2cb9151ee..feb93b669 100644
--- a/codegen/test/chart-envvars/templates/rbac.yaml
+++ b/codegen/test/chart-envvars/templates/rbac.yaml
@@ -1,3 +1,2 @@
 # Code generated by skv2. DO NOT EDIT.
 
-
diff --git a/codegen/test/chart-no-desc/templates/_helpers.tpl b/codegen/test/chart-no-desc/templates/_helpers.tpl
index 4d4e2732b..0c155a127 100644
--- a/codegen/test/chart-no-desc/templates/_helpers.tpl
+++ b/codegen/test/chart-no-desc/templates/_helpers.tpl
@@ -51,4 +51,4 @@ version, which merges two named templates.
 {{- $merged := merge $overrides $tpl -}}
 {{- toYaml $merged -}} {{/* render source with overrides as YAML */}}
 {{- end -}}
-{{- end -}}
+{{- end -}}
\ No newline at end of file
diff --git a/codegen/test/chart-no-desc/templates/deployment.yaml b/codegen/test/chart-no-desc/templates/deployment.yaml
index bc3e441fd..27a813a82 100644
--- a/codegen/test/chart-no-desc/templates/deployment.yaml
+++ b/codegen/test/chart-no-desc/templates/deployment.yaml
@@ -6,7 +6,6 @@
 ---
 
 {{- define "painter.deploymentSpec" }}
-
 # Deployment manifest for painter
 
 apiVersion: apps/v1
diff --git a/codegen/test/chart-no-desc/templates/rbac.yaml b/codegen/test/chart-no-desc/templates/rbac.yaml
index 2cb9151ee..feb93b669 100644
--- a/codegen/test/chart-no-desc/templates/rbac.yaml
+++ b/codegen/test/chart-no-desc/templates/rbac.yaml
@@ -1,3 +1,2 @@
 # Code generated by skv2. DO NOT EDIT.
 
-
diff --git a/codegen/test/chart-sidecar-svcport/templates/_helpers.tpl b/codegen/test/chart-sidecar-svcport/templates/_helpers.tpl
index 4d4e2732b..0c155a127 100644
--- a/codegen/test/chart-sidecar-svcport/templates/_helpers.tpl
+++ b/codegen/test/chart-sidecar-svcport/templates/_helpers.tpl
@@ -51,4 +51,4 @@ version, which merges two named templates.
 {{- $merged := merge $overrides $tpl -}}
 {{- toYaml $merged -}} {{/* render source with overrides as YAML */}}
 {{- end -}}
-{{- end -}}
+{{- end -}}
\ No newline at end of file
diff --git a/codegen/test/chart-sidecar-svcport/templates/deployment.yaml b/codegen/test/chart-sidecar-svcport/templates/deployment.yaml
index 46a51ffd4..8def14d6d 100644
--- a/codegen/test/chart-sidecar-svcport/templates/deployment.yaml
+++ b/codegen/test/chart-sidecar-svcport/templates/deployment.yaml
@@ -6,7 +6,6 @@
 ---
 
 {{- define "painter.deploymentSpec" }}
-
 # Deployment manifest for painter
 
 apiVersion: apps/v1
diff --git a/codegen/test/chart-sidecar-svcport/templates/rbac.yaml b/codegen/test/chart-sidecar-svcport/templates/rbac.yaml
index 2cb9151ee..feb93b669 100644
--- a/codegen/test/chart-sidecar-svcport/templates/rbac.yaml
+++ b/codegen/test/chart-sidecar-svcport/templates/rbac.yaml
@@ -1,3 +1,2 @@
 # Code generated by skv2. DO NOT EDIT.
 
-
diff --git a/codegen/test/chart-sidecar/templates/_helpers.tpl b/codegen/test/chart-sidecar/templates/_helpers.tpl
index 4d4e2732b..0c155a127 100644
--- a/codegen/test/chart-sidecar/templates/_helpers.tpl
+++ b/codegen/test/chart-sidecar/templates/_helpers.tpl
@@ -51,4 +51,4 @@ version, which merges two named templates.
 {{- $merged := merge $overrides $tpl -}}
 {{- toYaml $merged -}} {{/* render source with overrides as YAML */}}
 {{- end -}}
-{{- end -}}
+{{- end -}}
\ No newline at end of file
diff --git a/codegen/test/chart-sidecar/templates/deployment.yaml b/codegen/test/chart-sidecar/templates/deployment.yaml
index 00be645f3..b085c13ad 100644
--- a/codegen/test/chart-sidecar/templates/deployment.yaml
+++ b/codegen/test/chart-sidecar/templates/deployment.yaml
@@ -6,7 +6,6 @@
 ---
 
 {{- define "painter.deploymentSpec" }}
-
 # Deployment manifest for painter
 
 apiVersion: apps/v1
diff --git a/codegen/test/chart-sidecar/templates/rbac.yaml b/codegen/test/chart-sidecar/templates/rbac.yaml
index 2cb9151ee..feb93b669 100644
--- a/codegen/test/chart-sidecar/templates/rbac.yaml
+++ b/codegen/test/chart-sidecar/templates/rbac.yaml
@@ -1,3 +1,2 @@
 # Code generated by skv2. DO NOT EDIT.
 
-
diff --git a/codegen/test/chart-svcport/templates/_helpers.tpl b/codegen/test/chart-svcport/templates/_helpers.tpl
index 4d4e2732b..0c155a127 100644
--- a/codegen/test/chart-svcport/templates/_helpers.tpl
+++ b/codegen/test/chart-svcport/templates/_helpers.tpl
@@ -51,4 +51,4 @@ version, which merges two named templates.
 {{- $merged := merge $overrides $tpl -}}
 {{- toYaml $merged -}} {{/* render source with overrides as YAML */}}
 {{- end -}}
-{{- end -}}
+{{- end -}}
\ No newline at end of file
diff --git a/codegen/test/chart-svcport/templates/deployment.yaml b/codegen/test/chart-svcport/templates/deployment.yaml
index 9286eeef6..96ce9e8c0 100644
--- a/codegen/test/chart-svcport/templates/deployment.yaml
+++ b/codegen/test/chart-svcport/templates/deployment.yaml
@@ -6,7 +6,6 @@
 ---
 
 {{- define "painter.deploymentSpec" }}
-
 # Deployment manifest for painter
 
 apiVersion: apps/v1
diff --git a/codegen/test/chart-svcport/templates/rbac.yaml b/codegen/test/chart-svcport/templates/rbac.yaml
index 2cb9151ee..feb93b669 100644
--- a/codegen/test/chart-svcport/templates/rbac.yaml
+++ b/codegen/test/chart-svcport/templates/rbac.yaml
@@ -1,3 +1,2 @@
 # Code generated by skv2. DO NOT EDIT.
 
-
diff --git a/codegen/test/chart/conditional-sidecar/templates/_helpers.tpl b/codegen/test/chart/conditional-sidecar/templates/_helpers.tpl
index 4d4e2732b..0c155a127 100644
--- a/codegen/test/chart/conditional-sidecar/templates/_helpers.tpl
+++ b/codegen/test/chart/conditional-sidecar/templates/_helpers.tpl
@@ -51,4 +51,4 @@ version, which merges two named templates.
 {{- $merged := merge $overrides $tpl -}}
 {{- toYaml $merged -}} {{/* render source with overrides as YAML */}}
 {{- end -}}
-{{- end -}}
+{{- end -}}
\ No newline at end of file
diff --git a/codegen/test/chart/conditional-sidecar/templates/deployment.yaml b/codegen/test/chart/conditional-sidecar/templates/deployment.yaml
index 80f468fd3..337bbd1ce 100644
--- a/codegen/test/chart/conditional-sidecar/templates/deployment.yaml
+++ b/codegen/test/chart/conditional-sidecar/templates/deployment.yaml
@@ -6,7 +6,6 @@
 ---
 
 {{- define "gloo-mgmt-server.deploymentSpec" }}
-
 # Deployment manifest for gloo-mgmt-server
 
 apiVersion: apps/v1
@@ -239,7 +238,6 @@ spec:
 ---
 
 {{- define "gloo-agent.deploymentSpec" }}
-
 # Deployment manifest for gloo-agent
 
 apiVersion: apps/v1
diff --git a/codegen/test/chart/conditional-sidecar/templates/rbac.yaml b/codegen/test/chart/conditional-sidecar/templates/rbac.yaml
index 87d8107f2..04dd98687 100644
--- a/codegen/test/chart/conditional-sidecar/templates/rbac.yaml
+++ b/codegen/test/chart/conditional-sidecar/templates/rbac.yaml
@@ -6,12 +6,13 @@
 {{- $glooMgmtServer := $.Values.glooMgmtServer }}
 {{ if $glooMgmtServer.enabled }}
 
+
 ---
 
 kind: ClusterRole
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
-  name: gloo-mgmt-server-{{ default .Release.Namespace $.Values.glooMgmtServer.namespace }}
+  name: gloo-mgmt-server-{{ default .Release.Namespace $glooMgmtServer.namespace }}
   labels:
     app: gloo-mgmt-server
 rules:
@@ -29,20 +30,23 @@ rules:
   verbs:
   - '*'
 {{- end }}
+
 ---
 
 kind: ClusterRoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
-  name: gloo-mgmt-server-{{ default .Release.Namespace $.Values.glooMgmtServer.namespace }}
+  name: gloo-mgmt-server-{{ default .Release.Namespace $glooMgmtServer.namespace }}
   labels:
     app: gloo-mgmt-server
 subjects:
 - kind: ServiceAccount
   name: gloo-mgmt-server
-  namespace: {{ default .Release.Namespace $.Values.glooMgmtServer.namespace }}
+  namespace: {{ default .Release.Namespace $glooMgmtServer.namespace }}
 roleRef:
   kind: ClusterRole
-  name: gloo-mgmt-server-{{ default .Release.Namespace $.Values.glooMgmtServer.namespace }}
+  name: gloo-mgmt-server-{{ default .Release.Namespace $glooMgmtServer.namespace }}
   apiGroup: rbac.authorization.k8s.io
-{{- end }}
+
+
+{{- end }}
\ No newline at end of file
diff --git a/codegen/test/chart/templates/_helpers.tpl b/codegen/test/chart/templates/_helpers.tpl
index 4d4e2732b..1c0181361 100644
--- a/codegen/test/chart/templates/_helpers.tpl
+++ b/codegen/test/chart/templates/_helpers.tpl
@@ -52,3 +52,13 @@ version, which merges two named templates.
 {{- toYaml $merged -}} {{/* render source with overrides as YAML */}}
 {{- end -}}
 {{- end -}}
+
+{{- define "painter.namespacesForResource" }}
+{{- $resourcesToNamespaces := dict }}
+{{- range $entry := $.Values.painter.namespacedRbac }}
+  {{- range $resource := $entry.resources }}
+    {{- $_ := set $resourcesToNamespaces $resource (concat $entry.namespaces (get $resourcesToNamespaces $resource | default list) | mustUniq) }}
+  {{- end }}
+{{- end }}
+{{- get $resourcesToNamespaces  .Resource | join "," }}
+{{- end }}
\ No newline at end of file
diff --git a/codegen/test/chart/templates/deployment.yaml b/codegen/test/chart/templates/deployment.yaml
index eb23a7836..21477911a 100644
--- a/codegen/test/chart/templates/deployment.yaml
+++ b/codegen/test/chart/templates/deployment.yaml
@@ -6,7 +6,6 @@
 ---
 
 {{- define "painter.deploymentSpec" }}
-
 # Deployment manifest for painter
 
 apiVersion: apps/v1
diff --git a/codegen/test/chart/templates/rbac.yaml b/codegen/test/chart/templates/rbac.yaml
index 13f62217e..e5318ec7e 100644
--- a/codegen/test/chart/templates/rbac.yaml
+++ b/codegen/test/chart/templates/rbac.yaml
@@ -6,43 +6,106 @@
 {{- $painter := $.Values.painter }}
 {{ if $painter.enabled }}
 
+
 ---
 
 kind: ClusterRole
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
-  name: painter-{{ default .Release.Namespace $.Values.painter.namespace }}
+  name: painter-{{ default .Release.Namespace $painter.namespace }}
   labels:
     app: painter
 rules:
 - verbs:
   - GET
+
 ---
 
 kind: ClusterRoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
-  name: painter-{{ default .Release.Namespace $.Values.painter.namespace }}
+  name: painter-{{ default .Release.Namespace $painter.namespace }}
   labels:
     app: painter
 subjects:
 - kind: ServiceAccount
   name: painter
-  namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
+  namespace: {{ default .Release.Namespace $painter.namespace }}
 roleRef:
   kind: ClusterRole
-  name: painter-{{ default .Release.Namespace $.Values.painter.namespace }}
+  name: painter-{{ default .Release.Namespace $painter.namespace }}
   apiGroup: rbac.authorization.k8s.io
+
+{{- $painterNsToResources := dict }}
+{{- $painterNamespacedResources := list }}
+{{- $painterSupportedResources := list }}
+{{- $painterSupportedResources = append $painterSupportedResources "secrets" }}
+
+{{- range $entry := $painter.namespacedRbac }}
+  {{- range $ns := $entry.namespaces }}
+    {{- set $painterNsToResources $ns (concat $entry.resources (get $painterNsToResources $ns | default list) | mustUniq) }}
+  {{- end }}
+  {{- range $resource := $entry.resources }}
+    {{- if (has $resource $painterSupportedResources) }}
+      {{- $painterNamespacedResources = (append $painterNamespacedResources $resource | mustUniq) }}
+    {{- else }}
+      {{- fail (cat "invalid resource name" (quote $resource) "provided to namespacedRbac field. must be one of the following:" ($painterSupportedResources | join ", ")) }}
+    {{- end }}
+  {{- end }}
+{{- end }}
+
+{{- if not (eq (len $painterNamespacedResources) 1) }}
+---
+
+kind: ClusterRole
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: painter-{{ .Release.Name }}-{{ .Release.Namespace }}
+  labels:
+    app: painter
+rules:
+{{- if not (has "secrets" $painterNamespacedResources) }}
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - GET
+  - LIST
+  - WATCH
+{{- end }}
+
+---
+
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: painter-{{ .Release.Name }}-{{ .Release.Namespace }}
+  labels:
+    app: painter
+subjects:
+- kind: ServiceAccount
+  name: painter
+  namespace: {{ default .Release.Namespace $painter.namespace }}
+roleRef:
+  kind: ClusterRole
+  name: painter-{{ .Release.Name }}-{{ .Release.Namespace }}
+  apiGroup: rbac.authorization.k8s.io
+{{- end }}
+
+{{- range $ns, $resources := $painterNsToResources }}
+
 ---
 
 kind: Role
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
   name: painter
-  namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
+  namespace: {{ $ns }}
   labels:
     app: painter
 rules:
+{{- if (has "secrets" $resources) }}
 - apiGroups:
   - ""
   resources:
@@ -51,6 +114,7 @@ rules:
   - GET
   - LIST
   - WATCH
+{{- end }}
 
 ---
 
@@ -58,15 +122,17 @@ kind: RoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
   name: painter
-  namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
+  namespace: {{ $ns }}
   labels:
     app: painter
 subjects:
 - kind: ServiceAccount
   name: painter
-  namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
+  namespace: {{ default $.Release.Namespace $painter.namespace }}
 roleRef:
   kind: Role
   name: painter
   apiGroup: rbac.authorization.k8s.io
-{{- end }}
+
+  {{- end }}
+{{- end }}
\ No newline at end of file
diff --git a/codegen/test/name_override_chart/templates/_helpers.tpl b/codegen/test/name_override_chart/templates/_helpers.tpl
index 4d4e2732b..0c155a127 100644
--- a/codegen/test/name_override_chart/templates/_helpers.tpl
+++ b/codegen/test/name_override_chart/templates/_helpers.tpl
@@ -51,4 +51,4 @@ version, which merges two named templates.
 {{- $merged := merge $overrides $tpl -}}
 {{- toYaml $merged -}} {{/* render source with overrides as YAML */}}
 {{- end -}}
-{{- end -}}
+{{- end -}}
\ No newline at end of file
diff --git a/codegen/test/name_override_chart/templates/deployment.yaml b/codegen/test/name_override_chart/templates/deployment.yaml
index 35ddab857..d9b455cb6 100644
--- a/codegen/test/name_override_chart/templates/deployment.yaml
+++ b/codegen/test/name_override_chart/templates/deployment.yaml
@@ -6,7 +6,6 @@
 ---
 
 {{- define "painter-original-name.deploymentSpec" }}
-
 # Deployment manifest for painter-original-name
 
 apiVersion: apps/v1
diff --git a/codegen/test/name_override_chart/templates/rbac.yaml b/codegen/test/name_override_chart/templates/rbac.yaml
index 2cb9151ee..feb93b669 100644
--- a/codegen/test/name_override_chart/templates/rbac.yaml
+++ b/codegen/test/name_override_chart/templates/rbac.yaml
@@ -1,3 +1,2 @@
 # Code generated by skv2. DO NOT EDIT.
 
-