Default release namespace for namespaced rbac (#579)

* Default to release namespace for namespaced rbac * spacing fix * comments * parenthesis fix * spacing * remove comments * simplify * comments * spacing * dash fix * more than one listed resource needed * changelog * changelog * changelog * codegen * Set resourcesToNamespaces correctly so that --secret-namespaces flag can be set correctly * fix * remove * Add test for namespaced-rbac rendering
solo-io · Sep 4, 2024 · 4f24776 · 4f24776
1 parent 1b81dd0
commit 4f24776
Show file tree

Hide file tree

Showing 14 changed files with 476 additions and 176 deletions.
diff --git a/changelog/v0.40.6/default-release-namespace-for-namespaced-rbac.yaml b/changelog/v0.40.6/default-release-namespace-for-namespaced-rbac.yaml
@@ -0,0 +1,4 @@
+changelog:
+- type: NEW_FEATURE
+  description: "If namespacedRbac.namespaces is an empty list but namespacedRbac.resources is not, we will use a sane default of the Release.Namespace instead of ignoring the namespacedRbac.resources input."
+  issueLink: https://github.com/solo-io/gloo-mesh-enterprise/issues/15739
diff --git a/codegen/cmd_test.go b/codegen/cmd_test.go
@@ -3217,10 +3217,39 @@ roleRef:
 		cmd := &Command{
 			RenderProtos: false,
 			Chart: &Chart{
+				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",
+					},
+				},
 				Operators: []Operator{
 					{
 						Name:                  "painter",
 						CustomEnableCondition: "$painter.enabled",
+						Deployment: Deployment{
+							Container: Container{
+								Image: Image{
+									Tag:        "v0.0.0",
+									Repository: "painter",
+									Registry:   "quay.io/solo-io",
+									PullPolicy: "IfNotPresent",
+								},
+							},
+						},
+						Values: map[string]any{
+							"painter": map[string]any{
+								"enabled": true,
+								"namespacedRbac": map[string]any{
+									"resources":  []string{"secrets"},
+									"namespaces": []string{},
+								},
+							},
+						},
 						ClusterRbac: []rbacv1.PolicyRule{
 							{
 								Verbs: []string{"GET"},
@@ -3235,26 +3264,14 @@ roleRef:
 								},
 							},
 						},
-					},
-				},
-				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",
+			ManifestRoot: "codegen/test/chart/namespaced-rbac",
 		}
 
 		Expect(cmd.Execute()).NotTo(HaveOccurred(), "failed to execute command")
 
-		absPath, err := filepath.Abs("./codegen/test/chart/templates/rbac.yaml")
+		absPath, err := filepath.Abs("./codegen/test/chart/namespaced-rbac/templates/rbac.yaml")
 		Expect(err).NotTo(HaveOccurred(), "failed to get abs path")
 
 		rbac, err := os.ReadFile(absPath)
@@ -3358,6 +3375,54 @@ roleRef:
 		Expect(string(rbac)).To(ContainSubstring(clusterRoleBinding2Tmpl))
 		Expect(string(rbac)).To(ContainSubstring(roleTmpl))
 		Expect(string(rbac)).To(ContainSubstring(roleBindingTmpl))
+
+		helmValues := map[string]interface{}{
+			"painter": map[string]any{
+				"enabled": true,
+				"namespacedRbac": []any{
+					map[string]any{
+						"resources":  []string{"secrets"},
+						"namespaces": []string{},
+					},
+				},
+			},
+		}
+		renderedManifests := helmTemplate("codegen/test/chart/namespaced-rbac", helmValues)
+		renderedRole := `
+kind: Role
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: painter-release-name-default-namespaced
+  namespace: default
+  labels:
+    app: painter
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - GET
+  - LIST
+  - WATCH`
+		renderedRoleBinding := `
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: painter-release-name-default-namespaced
+  namespace: default
+  labels:
+    app: painter
+subjects:
+- kind: ServiceAccount
+  name: painter
+  namespace: default
+roleRef:
+  kind: Role
+  name: painter-release-name-default-namespaced
+  apiGroup: rbac.authorization.k8s.io`
+		Expect(string(renderedManifests)).To(ContainSubstring(renderedRole))
+		Expect(string(renderedManifests)).To(ContainSubstring(renderedRoleBinding))
 	})
 })
 

diff --git a/codegen/templates/chart/operator-rbac.yamltmpl b/codegen/templates/chart/operator-rbac.yamltmpl
@@ -91,6 +91,11 @@ Prefixed with '#' to avoid printing variable values in yaml, per https://github.
 [[- end ]]
 
 {{- range $entry := $[[ $operatorVar ]].namespacedRbac }}
+[[- /*  If resources were chosen but no namespaces were chosen, default to restricting those resources to the Release.Namespace. */]]
+  {{- if and (eq (len $entry.namespaces) 0) (ge (len $entry.resources) 1) }}
+    {{- set $[[ $operatorVar ]]NsToResources $.Release.Namespace ($entry.resources | mustUniq) }}
+  {{- end }}
+[[- /*  Otherwise, iterate through each of the namespaces and resources listed */]]
   {{- range $ns := $entry.namespaces }}
     {{- set $[[ $operatorVar ]]NsToResources $ns (concat $entry.resources (get $[[ $operatorVar ]]NsToResources $ns | default list) | mustUniq) }}
   {{- end }}

diff --git a/codegen/test/chart/namespaced-rbac/Chart.yaml b/codegen/test/chart/namespaced-rbac/Chart.yaml
@@ -0,0 +1,8 @@
+# Code generated by skv2. DO NOT EDIT.
+
+apiVersion: v1
+home: https://docs.solo.io/skv2/latest
+name: Painting Operator
+sources:
+- https://github.com/solo-io/skv2
+version: v0.0.1
diff --git a/codegen/test/chart/namespaced-rbac/templates/_helpers.tpl b/codegen/test/chart/namespaced-rbac/templates/_helpers.tpl
@@ -0,0 +1,64 @@
+# Code generated by skv2. DO NOT EDIT.
+
+
+
+{{/* Below are library functions provided by skv2 */}}
+
+{{- /*
+
+"skv2.utils.merge" takes an array of three values:
+- the top context
+- the yaml block that will be merged in (override)
+- the name of the base template (source)
+
+note: the source must be a named template (helm partial). This is necessary for the merging logic.
+
+The behaviour is as follows, to align with already existing helm behaviour:
+- If no source is found (template is empty), the merged output will be empty
+- If no overrides are specified, the source is rendered as is
+- If overrides are specified and source is not empty, overrides will be merged in to the source.
+
+Overrides can replace / add to deeply nested dictionaries, but will completely replace lists.
+Examples:
+
+┌─────────────────────┬───────────────────────┬────────────────────────┐
+│ Source (template)   │       Overrides       │        Result          │
+├─────────────────────┼───────────────────────┼────────────────────────┤
+│ metadata:           │ metadata:             │ metadata:              │
+│   labels:           │   labels:             │   labels:              │
+│     app: gloo       │    app: gloo1         │     app: gloo1         │
+│     cluster: useast │    author: infra-team │     author: infra-team │
+│                     │                       │     cluster: useast    │
+├─────────────────────┼───────────────────────┼────────────────────────┤
+│ lists:              │ lists:                │ lists:                 │
+│   groceries:        │  groceries:           │   groceries:           │
+│   - apple           │   - grapes            │   - grapes             │
+│   - banana          │                       │                        │
+└─────────────────────┴───────────────────────┴────────────────────────┘
+
+skv2.utils.merge is a fork of a helm library chart function (https://github.com/helm/charts/blob/master/incubator/common/templates/_util.tpl).
+This includes some optimizations to speed up chart rendering time, and merges in a value (overrides) with a named template, unlike the upstream
+version, which merges two named templates.
+
+*/ -}}
+{{- define "skv2.utils.merge" -}}
+{{- $top := first . -}}
+{{- $overrides := (index . 1) -}}
+{{- $tpl := fromYaml (include (index . 2) $top) -}}
+{{- if or (empty $overrides) (empty $tpl) -}}
+{{ include (index . 2) $top }} {{/* render source as is */}}
+{{- else -}}
+{{- $merged := merge $overrides $tpl -}}
+{{- 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 }}
diff --git a/codegen/test/chart/namespaced-rbac/templates/deployment.yaml b/codegen/test/chart/namespaced-rbac/templates/deployment.yaml
@@ -0,0 +1,138 @@
+# Code generated by skv2. DO NOT EDIT.
+
+
+
+{{- $painter := $.Values.painter }}
+---
+
+{{- define "painter.deploymentSpec" }}
+# Deployment manifest for painter
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  labels:
+    app: painter
+  annotations:
+    app.kubernetes.io/name: painter
+  name: painter
+  namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
+spec:
+  selector:
+    matchLabels:
+      app: painter
+  template:
+    metadata:
+      labels:
+        app: painter
+      annotations:
+        app.kubernetes.io/name: painter
+    spec:
+      serviceAccountName: painter
+      {{- /* Override the default podSecurityContext config if it is set. */}}
+{{- if or ($.Values.painter.podSecurityContext) (eq "map[]" (printf "%v" $.Values.painter.podSecurityContext)) }}
+      securityContext:
+{{ toYaml $.Values.painter.podSecurityContext | indent 8 }}
+{{- end }}
+      containers:
+{{- $painter := $.Values.painter }}
+{{- $painterImage := $painter.image }}
+      - name: painter
+        image: {{ $painterImage.registry }}/{{ $painterImage.repository }}:{{ $painterImage.tag }}
+        imagePullPolicy: {{ $painterImage.pullPolicy }}
+  {{- if or $painter.env $painter.extraEnvs }}
+        env:
+  {{- end }}
+{{- if $painter.env }}
+{{- toYaml $painter.env | nindent 10 }}
+{{- end }}
+{{- range $name, $item := $painter.extraEnvs }}
+          - name: {{ $name }}
+  {{- $item | toYaml | nindent 12 }}
+{{- end }}
+        resources:
+{{- if $painter.resources }}
+{{ toYaml $painter.resources | indent 10}}
+{{- else}}
+          requests:
+            cpu: 500m
+            memory: 256Mi
+{{- end }}
+        {{- /*
+          Render securityContext configs if it is set.
+          If securityContext is not set, render the default securityContext.
+          If securityContext is set to 'false', render an empty map.
+        */}}
+        securityContext:
+{{- if or ($painter.securityContext) (eq "map[]" (printf "%v" $painter.securityContext)) }}
+{{ toYaml $painter.securityContext | indent 10}}
+{{/* Because securityContext is nil by default we can only perform following conversion if it is a boolean. Skip conditional otherwise. */}}
+{{- else if eq (ternary $painter.securityContext true (eq "bool" (printf "%T" $painter.securityContext))) false }}
+          {}
+{{- else}}
+          runAsNonRoot: true
+          {{- if not $painter.floatingUserId }}
+          runAsUser: {{ printf "%.0f" (float64 $painter.runAsUser) }}
+          {{- end }}
+          readOnlyRootFilesystem: true
+          allowPrivilegeEscalation: false
+          capabilities:
+            drop:
+            - ALL
+{{- end }}
+{{- $pullSecrets := (list) -}}
+{{- if $painterImage.pullSecret }}
+  {{- $pullSecrets = concat $pullSecrets (list (dict "name" $painterImage.pullSecret)) -}}
+{{- end }}
+{{- if $painter.imagePullSecrets }}
+  {{- $pullSecrets = concat $pullSecrets $painter.imagePullSecrets -}}
+{{- end }}
+{{- if gt (len $pullSecrets) 0 -}}
+  {{- (dict "imagePullSecrets" $pullSecrets) | toYaml | nindent 6 }}
+{{- end }}
+{{- end }} {{/* define "painter.deploymentSpec" */}}
+
+{{/* Render painter deployment template with overrides from values*/}}
+{{ if $painter.enabled }}
+{{- $painterDeploymentOverrides := dict }}
+{{- if $painter.deploymentOverrides }}
+{{- $painterDeploymentOverrides = $painter.deploymentOverrides  }}
+{{- end }}
+---
+{{ include "skv2.utils.merge" (list . $painterDeploymentOverrides "painter.deploymentSpec") }}
+{{- end }}
+---
+{{ if $painter.enabled }}
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  labels:
+    app: painter
+  {{- if $painter.serviceAccount}}
+  {{- if $painter.serviceAccount.extraAnnotations }}
+  annotations:
+    {{- range $key, $value := $painter.serviceAccount.extraAnnotations }}
+    {{ $key }}: {{ $value }}
+    {{- end }}
+  {{- end }}
+  {{- end}}
+  name: painter
+  namespace: {{ default .Release.Namespace $.Values.painter.namespace }}
+{{- end }}
+
+
+{{- define "painter.serviceSpec"}}
+
+{{- end }} {{/* define "painter.serviceSpec" */}}
+{{ if $painter.enabled }}
+{{/* Render painter service template with overrides from values*/}}
+{{- $painterServiceOverrides := dict }}
+{{- if $painter.serviceOverrides }}
+{{- $painterServiceOverrides = $painter.serviceOverrides  }}
+{{- end }}
+
+---
+
+{{ include "skv2.utils.merge" (list . $painterServiceOverrides "painter.serviceSpec") }}
+{{- end }}
+