[backport v0.29.x-gp] namespace-scoped rbac (#500)

* 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> * update changelog * remove sidecar api changes that were tangential to rbac api changes * generate with correct go version * make namespace rbac resource naming unique * update operator rbac template resource naming to support multiple helm chart installations (#502) * make Role naming consistent with ClusterRole to support multi installation of a helm chart * fix roleRef * add changelog * update unit test * pr feedback * generate * rm cherry-picked changelog * update changelog folder to be gp patch * add validation for gp-patch * try with 0 * rm 0
solo-io · Sep 21, 2023 · d9c4edb · d9c4edb
1 parent 99cf5b9
commit d9c4edb
Show file tree

Hide file tree

Showing 32 changed files with 997 additions and 122 deletions.
diff --git a/changelog/v0.29.12-gp-patch0/ns-rbac-by-helm-flag.yaml b/changelog/v0.29.12-gp-patch0/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/changelog/validation.yaml b/changelog/validation.yaml
@@ -0,0 +1,3 @@
+relaxSemverValidation: true
+allowedLabels:
+  - gp-patch
diff --git a/codegen/cmd_test.go b/codegen/cmd_test.go
@@ -3,7 +3,6 @@ package codegen_test
 import (
 	"bytes"
 	"encoding/json"
-	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -1101,7 +1100,7 @@ var _ = Describe("Cmd", func() {
 					{
 						Name:             "painter",
 						EnabledDependsOn: []string{"test1", "test2"},
-						Rbac: []rbacv1.PolicyRule{
+						ClusterRbac: []rbacv1.PolicyRule{
 							{
 								Verbs: []string{"GET"},
 							},
@@ -1307,7 +1306,7 @@ var _ = Describe("Cmd", func() {
 			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"))
 		})
@@ -1318,7 +1317,7 @@ var _ = Describe("Cmd", func() {
 			err := cmd.Execute()
 			Expect(err).NotTo(HaveOccurred())
 
-			bytes, err := ioutil.ReadFile(crdFilePath)
+			bytes, err := os.ReadFile(crdFilePath)
 			Expect(err).NotTo(HaveOccurred())
 			generatedCrd := &v12.CustomResourceDefinition{}
 			Expect(yaml.Unmarshal(bytes, generatedCrd)).NotTo(HaveOccurred())
@@ -1340,7 +1339,7 @@ var _ = Describe("Cmd", func() {
 			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:"))
 		})
@@ -1480,13 +1479,160 @@ var _ = Describe("Cmd", func() {
 			map[string]interface{}{"FOO": map[string]interface{}{"valueFrom": map[string]interface{}{"secretKeyRef": map[string]interface{}{"name": "bar", "key": "baz"}}}},
 			[]v1.EnvVar{{Name: "FOO", ValueFrom: &v1.EnvVarSource{SecretKeyRef: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "bar"}, Key: "baz"}}}}),
 	)
+
+	It("can configure cluster-scoped and namespace-scoped RBAC", func() {
+		cmd := &Command{
+			RenderProtos: false,
+			Chart: &Chart{
+				Operators: []Operator{
+					{
+						Name:             "painter",
+						EnabledDependsOn: []string{"$painter.enabled"},
+						ClusterRbac: []rbacv1.PolicyRule{
+							{
+								Verbs: []string{"GET"},
+							},
+						},
+						NamespaceRbac: map[string][]rbacv1.PolicyRule{
+							"secrets": {
+								rbacv1.PolicyRule{
+									Verbs:     []string{"GET", "LIST", "WATCH"},
+									APIGroups: []string{""},
+									Resources: []string{"secrets"},
+								},
+							},
+						},
+					},
+				},
+				Values: nil,
+				Data: Data{
+					ApiVersion:  "v1",
+					Description: "",
+					Name:        "Painting Operator",
+					Version:     "v0.0.1",
+					Home:        "https://docs.solo.io/skv2/latest",
+					Sources: []string{
+						"https://github.com/solo-io/skv2",
+					},
+				},
+			},
+			ManifestRoot: "codegen/test/chart",
+		}
+
+		Expect(cmd.Execute()).NotTo(HaveOccurred(), "failed to execute command")
+
+		absPath, err := filepath.Abs("./codegen/test/chart/templates/rbac.yaml")
+		Expect(err).NotTo(HaveOccurred(), "failed to get abs path")
+
+		rbac, err := os.ReadFile(absPath)
+		Expect(err).NotTo(HaveOccurred(), "failed to read rbac.yaml")
+		clusterRole1Tmpl := `
+kind: ClusterRole
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: painter-{{ .Release.Namespace }}
+  labels:
+    app: painter
+rules:
+- verbs:
+  - GET`
+		clusterRoleBinding1Tmpl := `
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: painter-{{ .Release.Namespace }}
+  labels:
+    app: painter
+subjects:
+- kind: ServiceAccount
+  name: painter
+  namespace: {{ .Release.Namespace }}
+roleRef:
+  kind: ClusterRole
+  name: painter-{{ .Release.Namespace }}
+  apiGroup: rbac.authorization.k8s.io`
+		clusterRole2Tmpl := `
+kind: ClusterRole
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: painter-{{ .Release.Name }}-{{ .Release.Namespace }}-namespaced
+  labels:
+    app: painter
+rules:
+{{- if not (has "secrets" $painterNamespacedResources) }}
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - GET
+  - LIST
+  - WATCH
+{{- end }}`
+		clusterRoleBinding2Tmpl := `
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: painter-{{ .Release.Name }}-{{ .Release.Namespace }}-namespaced
+  labels:
+    app: painter
+subjects:
+- kind: ServiceAccount
+  name: painter
+  namespace: {{ .Release.Namespace }}
+roleRef:
+  kind: ClusterRole
+  name: painter-{{ .Release.Name }}-{{ .Release.Namespace }}-namespaced
+  apiGroup: rbac.authorization.k8s.io`
+		roleTmpl := `
+kind: Role
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: painter-{{ $.Release.Name }}-{{ $.Release.Namespace }}-namespaced
+  namespace: {{ $ns }}
+  labels:
+    app: painter
+rules:
+{{- if (has "secrets" $resources) }}
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - GET
+  - LIST
+  - WATCH
+{{- end }}`
+		roleBindingTmpl := `
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: painter-{{ $.Release.Name }}-{{ $.Release.Namespace }}-namespaced
+  namespace: {{ $ns }}
+  labels:
+    app: painter
+subjects:
+- kind: ServiceAccount
+  name: painter
+  namespace: {{ $.Release.Namespace }}
+roleRef:
+  kind: Role
+  name: painter-{{ $.Release.Name }}-{{ $.Release.Namespace }}-namespaced
+  apiGroup: rbac.authorization.k8s.io`
+		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)
@@ -1506,7 +1652,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
@@ -75,7 +75,11 @@ type Operator struct {
 	Deployment Deployment
 
 	// these populate the generated ClusterRole for the operator
-	Rbac []rbacv1.PolicyRule
+	ClusterRbac []rbacv1.PolicyRule
+
+	// these populate the generated Role for the operator
+	// 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

diff --git a/codegen/model/resource.go b/codegen/model/resource.go
@@ -158,7 +158,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
@@ -129,8 +129,8 @@ func makeTemplateFuncs(customFuncs template.FuncMap) template.FuncMap {
 		},
 
 		"containerConfigs": containerConfigs,
-
-		"opVar": opVar,
+		"toListItem":       toListItem,
+		"opVar":            opVar,
 	}
 
 	for k, v := range skv2Funcs {
@@ -144,6 +144,10 @@ func makeTemplateFuncs(customFuncs template.FuncMap) template.FuncMap {
 	return f
 }
 
+func toListItem(item interface{}) []interface{} {
+	return []interface{}{item}
+}
+
 type containerConfig struct {
 	model.Container
 	Name      string

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) -]]