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>
solo-io · Sep 11, 2023 · f3f6193 · f3f6193
1 parent a021afb
commit f3f6193
Show file tree

Hide file tree

Showing 32 changed files with 298 additions and 103 deletions.
diff --git a/changelog/v0.34.3/ns-rbac-by-helm-flag.yaml 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
diff --git 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,76 +2033,113 @@ 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:
   - secrets
   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))
 	})
 })
 
 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
@@ -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
@@ -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
@@ -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
@@ -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 ]]
diff --git 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