remove wildcard permissions (kyverno#10785)

* remove wildcard permissions Signed-off-by: Jim Bugwadia <[email protected]> * update codegen Signed-off-by: Jim Bugwadia <[email protected]> * codegen Signed-off-by: Jim Bugwadia <[email protected]> * fix tests Signed-off-by: Jim Bugwadia <[email protected]> * fix background controller perms Signed-off-by: Jim Bugwadia <[email protected]> * remove secrets perm Signed-off-by: Jim Bugwadia <[email protected]> * update tests Signed-off-by: Jim Bugwadia <[email protected]> * update tests Signed-off-by: Jim Bugwadia <[email protected]> * fix reports-controller role Signed-off-by: Jim Bugwadia <[email protected]> * add wildcard check and limit generate policy checks based on `synchronize` Signed-off-by: Jim Bugwadia <[email protected]> * update manifest Signed-off-by: Jim Bugwadia <[email protected]> * fix permissions Signed-off-by: Jim Bugwadia <[email protected]> * fix wildcard check Signed-off-by: Jim Bugwadia <[email protected]> * update default QPS and burst for better performance and to prevent test failure Signed-off-by: Jim Bugwadia <[email protected]> * fix test permissions Signed-off-by: Jim Bugwadia <[email protected]> * fix test permissions Signed-off-by: Jim Bugwadia <[email protected]> * fix test permissions Signed-off-by: Jim Bugwadia <[email protected]> * fix test permissions Signed-off-by: Jim Bugwadia <[email protected]> * fix test permissions Signed-off-by: Jim Bugwadia <[email protected]> * fix test permissions Signed-off-by: Jim Bugwadia <[email protected]> * fix test permissions Signed-off-by: Jim Bugwadia <[email protected]> * fix perms Signed-off-by: Jim Bugwadia <[email protected]> * fix perms Signed-off-by: Jim Bugwadia <[email protected]> * fix test permissions Signed-off-by: Jim Bugwadia <[email protected]> * fix test permissions Signed-off-by: Jim Bugwadia <[email protected]> * fix merge issues Signed-off-by: Jim Bugwadia <[email protected]> * fix merge issues Signed-off-by: Jim Bugwadia <[email protected]> --------- Signed-off-by: Jim Bugwadia <[email protected]> Co-authored-by: Mariam Fahmy <[email protected]>
MariamFahmy98 · Aug 20, 2024 · f063992 · f063992
1 parent 0c2a886
commit f063992
Show file tree

Hide file tree

Showing 189 changed files with 2,230 additions and 309 deletions.
diff --git a/charts/kyverno/README.md b/charts/kyverno/README.md
@@ -358,6 +358,8 @@ The chart values are organised per component.
 | admissionController.featuresOverride | object | `{"admissionReports":{"backPressureThreshold":1000}}` | Overrides features defined at the root level |
 | admissionController.featuresOverride.admissionReports.backPressureThreshold | int | `1000` | Max number of admission reports allowed in flight until the admission controller stops creating new ones |
 | admissionController.rbac.create | bool | `true` | Create RBAC resources |
+| admissionController.rbac.createViewRoleBinding | bool | `true` | Create rolebinding to view role |
+| admissionController.rbac.viewRoleName | string | `"view"` | The view role to use in the rolebinding |
 | admissionController.rbac.serviceAccount.name | string | `nil` | The ServiceAccount name |
 | admissionController.rbac.serviceAccount.annotations | object | `{}` | Annotations for the ServiceAccount |
 | admissionController.rbac.coreClusterRole.extraResources | list | See [values.yaml](values.yaml) | Extra resource permissions to add in the core cluster role. This was introduced to avoid breaking change in the chart but should ideally be moved in `clusterRole.extraResources`. |
@@ -454,6 +456,8 @@ The chart values are organised per component.
 | backgroundController.featuresOverride | object | `{}` | Overrides features defined at the root level |
 | backgroundController.enabled | bool | `true` | Enable background controller. |
 | backgroundController.rbac.create | bool | `true` | Create RBAC resources |
+| backgroundController.rbac.createViewRoleBinding | bool | `true` | Create rolebinding to view role |
+| backgroundController.rbac.viewRoleName | string | `"view"` | The view role to use in the rolebinding |
 | backgroundController.rbac.serviceAccount.name | string | `nil` | Service account name |
 | backgroundController.rbac.serviceAccount.annotations | object | `{}` | Annotations for the ServiceAccount |
 | backgroundController.rbac.coreClusterRole.extraResources | list | See [values.yaml](values.yaml) | Extra resource permissions to add in the core cluster role. This was introduced to avoid breaking change in the chart but should ideally be moved in `clusterRole.extraResources`. |
@@ -606,6 +610,8 @@ The chart values are organised per component.
 | reportsController.featuresOverride | object | `{}` | Overrides features defined at the root level |
 | reportsController.enabled | bool | `true` | Enable reports controller. |
 | reportsController.rbac.create | bool | `true` | Create RBAC resources |
+| reportsController.rbac.createViewRoleBinding | bool | `true` | Create rolebinding to view role |
+| reportsController.rbac.viewRoleName | string | `"view"` | The view role to use in the rolebinding |
 | reportsController.rbac.serviceAccount.name | string | `nil` | Service account name |
 | reportsController.rbac.serviceAccount.annotations | object | `{}` | Annotations for the ServiceAccount |
 | reportsController.rbac.coreClusterRole.extraResources | list | See [values.yaml](values.yaml) | Extra resource permissions to add in the core cluster role. This was introduced to avoid breaking change in the chart but should ideally be moved in `clusterRole.extraResources`. |

diff --git a/charts/kyverno/templates/admission-controller/clusterrole.yaml b/charts/kyverno/templates/admission-controller/clusterrole.yaml
@@ -7,6 +7,8 @@ metadata:
     {{- include "kyverno.admission-controller.labels" . | nindent 4 }}
 aggregationRule:
   clusterRoleSelectors:
+    - matchLabels:
+        rbac.kyverno.io/aggregate-to-admission-controller: "true"
     - matchLabels:
         {{- include "kyverno.admission-controller.matchLabels" . | nindent 8 }}
 ---

diff --git a/charts/kyverno/templates/admission-controller/clusterrolebinding.yaml b/charts/kyverno/templates/admission-controller/clusterrolebinding.yaml
@@ -13,4 +13,21 @@ subjects:
   - kind: ServiceAccount
     name: {{ template "kyverno.admission-controller.serviceAccountName" . }}
     namespace: {{ template "kyverno.namespace" . }}
+{{- if .Values.admissionController.rbac.createViewRoleBinding }}
+---
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: {{ template "kyverno.admission-controller.roleName" . }}:view
+  labels:
+    {{- include "kyverno.admission-controller.labels" . | nindent 4 }}
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: {{ .Values.admissionController.rbac.viewRoleName }}
+subjects:
+  - kind: ServiceAccount
+    name: {{ template "kyverno.admission-controller.serviceAccountName" . }}
+    namespace: {{ template "kyverno.namespace" . }}
+{{- end -}}
 {{- end -}}
diff --git a/charts/kyverno/templates/admission-controller/deployment.yaml b/charts/kyverno/templates/admission-controller/deployment.yaml
@@ -131,6 +131,7 @@ spec:
             - --caSecretName={{ template "kyverno.admission-controller.serviceName" . }}.{{ template "kyverno.namespace" . }}.svc.kyverno-tls-ca
             - --tlsSecretName={{ template "kyverno.admission-controller.serviceName" . }}.{{ template "kyverno.namespace" . }}.svc.kyverno-tls-pair
             - --backgroundServiceAccountName=system:serviceaccount:{{ include "kyverno.namespace" . }}:{{ include "kyverno.background-controller.serviceAccountName" . }}
+            - --reportsServiceAccountName=system:serviceaccount:{{ include "kyverno.namespace" . }}:{{ include "kyverno.reports-controller.serviceAccountName" . }}
             - --servicePort={{ .Values.admissionController.service.port }}
             - --webhookServerPort={{ .Values.admissionController.webhookServer.port }}
             {{- if .Values.admissionController.tracing.enabled }}

diff --git a/charts/kyverno/templates/background-controller/clusterrole.yaml b/charts/kyverno/templates/background-controller/clusterrole.yaml
@@ -8,6 +8,8 @@ metadata:
     {{- include "kyverno.background-controller.labels" . | nindent 4 }}
 aggregationRule:
   clusterRoleSelectors:
+    - matchLabels:
+        rbac.kyverno.io/aggregate-to-background-controller: "true"
     - matchLabels:
         {{- include "kyverno.background-controller.matchLabels" . | nindent 8 }}
 ---
@@ -28,7 +30,9 @@ rules:
       - kyverno.io
     resources:
       - policies
+      - policies/status
       - clusterpolicies
+      - clusterpolicies/status
       - policyexceptions
       - updaterequests
       - updaterequests/status

diff --git a/charts/kyverno/templates/background-controller/clusterrolebinding.yaml b/charts/kyverno/templates/background-controller/clusterrolebinding.yaml
@@ -14,5 +14,22 @@ subjects:
 - kind: ServiceAccount
   name: {{ template "kyverno.background-controller.serviceAccountName" . }}
   namespace: {{ template "kyverno.namespace" . }}
+{{- if .Values.backgroundController.rbac.createViewRoleBinding }}
+---
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: {{ template "kyverno.background-controller.roleName" . }}:view
+  labels:
+    {{- include "kyverno.background-controller.labels" . | nindent 4 }}
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: {{ .Values.backgroundController.rbac.viewRoleName }}
+subjects:
+- kind: ServiceAccount
+  name: {{ template "kyverno.background-controller.serviceAccountName" . }}
+  namespace: {{ template "kyverno.namespace" . }}
+{{- end -}}
 {{- end -}}
 {{- end -}}
diff --git a/charts/kyverno/templates/cleanup-controller/clusterrole.yaml b/charts/kyverno/templates/cleanup-controller/clusterrole.yaml
@@ -8,6 +8,8 @@ metadata:
     {{- include "kyverno.cleanup-controller.labels" . | nindent 4 }}
 aggregationRule:
   clusterRoleSelectors:
+    - matchLabels:
+        rbac.kyverno.io/aggregate-to-cleanup-controller: "true"
     - matchLabels:
         {{- include "kyverno.cleanup-controller.matchLabels" . | nindent 8 }}
 ---

diff --git a/charts/kyverno/templates/reports-controller/clusterrole.yaml b/charts/kyverno/templates/reports-controller/clusterrole.yaml
@@ -8,6 +8,8 @@ metadata:
     {{- include "kyverno.reports-controller.labels" . | nindent 4 }}
 aggregationRule:
   clusterRoleSelectors:
+    - matchLabels:
+        rbac.kyverno.io/aggregate-to-reports-controller: "true"
     - matchLabels:
         {{- include "kyverno.reports-controller.matchLabels" . | nindent 8 }}
 ---
@@ -27,7 +29,6 @@ rules:
   - apiGroups:
       - ''
     resources:
-      - secrets
       - configmaps
       - namespaces
     verbs:

diff --git a/charts/kyverno/templates/reports-controller/clusterrolebinding.yaml b/charts/kyverno/templates/reports-controller/clusterrolebinding.yaml
@@ -14,5 +14,22 @@ subjects:
 - kind: ServiceAccount
   name: {{ template "kyverno.reports-controller.serviceAccountName" . }}
   namespace: {{ template "kyverno.namespace" . }}
+{{- if .Values.reportsController.rbac.createViewRoleBinding }}
+---
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: {{ template "kyverno.reports-controller.roleName" . }}:view
+  labels:
+    {{- include "kyverno.reports-controller.labels" . | nindent 4 }}
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: {{ .Values.reportsController.rbac.viewRoleName }}
+subjects:
+- kind: ServiceAccount
+  name: {{ template "kyverno.reports-controller.serviceAccountName" . }}
+  namespace: {{ template "kyverno.namespace" . }}
+{{- end -}}
 {{- end -}}
 {{- end -}}
diff --git a/charts/kyverno/templates/reports-controller/role.yaml b/charts/kyverno/templates/reports-controller/role.yaml
@@ -19,6 +19,14 @@ rules:
     resourceNames:
       - {{ include "kyverno.config.configMapName" . }}
       - {{ include "kyverno.config.metricsConfigMapName" . }}
+  - apiGroups:
+      - ''
+    resources:
+      - secrets
+    verbs:
+      - get
+      - list
+      - watch
   - apiGroups:
       - coordination.k8s.io
     resources:

diff --git a/charts/kyverno/values.yaml b/charts/kyverno/values.yaml
@@ -697,6 +697,12 @@ admissionController:
     # -- Create RBAC resources
     create: true
 
+    # -- Create rolebinding to view role
+    createViewRoleBinding: true
+
+    # -- The view role to use in the rolebinding
+    viewRoleName: view
+
     serviceAccount:
       # -- The ServiceAccount name
       name:
@@ -709,15 +715,7 @@ admissionController:
       # -- Extra resource permissions to add in the core cluster role.
       # This was introduced to avoid breaking change in the chart but should ideally be moved in `clusterRole.extraResources`.
       # @default -- See [values.yaml](values.yaml)
-      extraResources:
-        - apiGroups:
-            - '*'
-          resources:
-            - '*'
-          verbs:
-            - get
-            - list
-            - watch
+      extraResources: []
 
     clusterRole:
       # -- Extra resource permissions to add in the cluster role
@@ -1108,6 +1106,12 @@ backgroundController:
     # -- Create RBAC resources
     create: true
 
+    # -- Create rolebinding to view role
+    createViewRoleBinding: true
+
+    # -- The view role to use in the rolebinding
+    viewRoleName: view
+
     serviceAccount:
       # -- Service account name
       name:
@@ -1121,14 +1125,6 @@ backgroundController:
       # This was introduced to avoid breaking change in the chart but should ideally be moved in `clusterRole.extraResources`.
       # @default -- See [values.yaml](values.yaml)
       extraResources:
-        - apiGroups:
-            - '*'
-          resources:
-            - '*'
-          verbs:
-            - get
-            - list
-            - watch
         - apiGroups:
             - networking.k8s.io
           resources:
@@ -1154,7 +1150,6 @@ backgroundController:
             - ''
           resources:
             - configmaps
-            - secrets
             - resourcequotas
             - limitranges
           verbs:
@@ -1718,6 +1713,12 @@ reportsController:
     # -- Create RBAC resources
     create: true
 
+    # -- Create rolebinding to view role
+    createViewRoleBinding: true
+
+    # -- The view role to use in the rolebinding
+    viewRoleName: view
+
     serviceAccount:
       # -- Service account name
       name:
@@ -1730,15 +1731,7 @@ reportsController:
       # -- Extra resource permissions to add in the core cluster role.
       # This was introduced to avoid breaking change in the chart but should ideally be moved in `clusterRole.extraResources`.
       # @default -- See [values.yaml](values.yaml)
-      extraResources:
-        - apiGroups:
-            - '*'
-          resources:
-            - '*'
-          verbs:
-            - get
-            - list
-            - watch
+      extraResources: []
 
     clusterRole:
       # -- Extra resource permissions to add in the cluster role

diff --git a/cmd/cli/kubectl-kyverno/commands/apply/command.go b/cmd/cli/kubectl-kyverno/commands/apply/command.go
@@ -273,7 +273,8 @@ func (c *ApplyCommandConfig) applyPolicytoResource(
 	validPolicies := make([]kyvernov1.PolicyInterface, 0, len(policies))
 	for _, pol := range policies {
 		// TODO we should return this info to the caller
-		_, err := policyvalidation.Validate(pol, nil, nil, nil, true, config.KyvernoUserName(config.KyvernoServiceAccountName()))
+		sa := config.KyvernoUserName(config.KyvernoServiceAccountName())
+		_, err := policyvalidation.Validate(pol, nil, nil, nil, true, sa, sa)
 		if err != nil {
 			log.Log.Error(err, "policy validation error")
 			rc.IncrementError(1)

diff --git a/cmd/cli/kubectl-kyverno/commands/oci/push/options.go b/cmd/cli/kubectl-kyverno/commands/oci/push/options.go
@@ -40,7 +40,8 @@ func (o options) execute(ctx context.Context, dir string, keychain authn.Keychai
 		return fmt.Errorf("unable to read policy file or directory %s (%w)", dir, err)
 	}
 	for _, policy := range results.Policies {
-		if _, err := policyvalidation.Validate(policy, nil, nil, nil, true, config.KyvernoUserName(config.KyvernoServiceAccountName())); err != nil {
+		sa := config.KyvernoUserName(config.KyvernoServiceAccountName())
+		if _, err := policyvalidation.Validate(policy, nil, nil, nil, true, sa, sa); err != nil {
 			return fmt.Errorf("validating policy %s: %v", policy.GetName(), err)
 		}
 	}

diff --git a/cmd/cli/kubectl-kyverno/commands/test/test.go b/cmd/cli/kubectl-kyverno/commands/test/test.go
@@ -153,7 +153,8 @@ func runTest(out io.Writer, testCase test.TestCase, registryAccess bool) ([]engi
 	validPolicies := make([]kyvernov1.PolicyInterface, 0, len(results.Policies))
 	for _, pol := range results.Policies {
 		// TODO we should return this info to the caller
-		_, err := policyvalidation.Validate(pol, nil, nil, nil, true, config.KyvernoUserName(config.KyvernoServiceAccountName()))
+		sa := config.KyvernoUserName(config.KyvernoServiceAccountName())
+		_, err := policyvalidation.Validate(pol, nil, nil, nil, true, sa, sa)
 		if err != nil {
 			log.Log.Error(err, "skipping invalid policy", "name", pol.GetName())
 			continue

diff --git a/cmd/internal/flag.go b/cmd/internal/flag.go
@@ -144,8 +144,8 @@ type options struct {
 
 func newOptions() options {
 	return options{
-		clientRateLimitQPS:   20,
-		clientRateLimitBurst: 50,
+		clientRateLimitQPS:   100,
+		clientRateLimitBurst: 200,
 		eventsRateLimitQPS:   1000,
 		eventsRateLimitBurst: 2000,
 	}

diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go
@@ -247,6 +247,7 @@ func main() {
 		servicePort                  int
 		webhookServerPort            int
 		backgroundServiceAccountName string
+		reportsServiceAccountName    string
 		maxAPICallResponseLength     int64
 		renewBefore                  time.Duration
 		maxAuditWorkers              int
@@ -267,7 +268,8 @@ func main() {
 	flagset.BoolVar(&admissionReports, "admissionReports", true, "Enable or disable admission reports.")
 	flagset.IntVar(&servicePort, "servicePort", 443, "Port used by the Kyverno Service resource and for webhook configurations.")
 	flagset.IntVar(&webhookServerPort, "webhookServerPort", 9443, "Port used by the webhook server.")
-	flagset.StringVar(&backgroundServiceAccountName, "backgroundServiceAccountName", "", "Background service account name.")
+	flagset.StringVar(&backgroundServiceAccountName, "backgroundServiceAccountName", "", "Background controller service account name.")
+	flagset.StringVar(&reportsServiceAccountName, "reportsServiceAccountName", "", "Reports controller service account name.")
 	flagset.StringVar(&caSecretName, "caSecretName", "", "Name of the secret containing CA.")
 	flagset.StringVar(&tlsSecretName, "tlsSecretName", "", "Name of the secret containing TLS pair.")
 	flagset.Int64Var(&maxAPICallResponseLength, "maxAPICallResponseLength", 10*1000*1000, "Configure the value of maximum allowed GET response size from API Calls")
@@ -516,6 +518,7 @@ func main() {
 			setup.KyvernoDynamicClient,
 			setup.KyvernoClient,
 			backgroundServiceAccountName,
+			reportsServiceAccountName,
 		)
 		ephrs, err := StartAdmissionReportsCounter(signalCtx, setup.MetadataClient)
 		if err != nil {
@@ -544,6 +547,7 @@ func main() {
 			eventGenerator,
 			admissionReports,
 			backgroundServiceAccountName,
+			reportsServiceAccountName,
 			setup.Jp,
 			maxAuditWorkers,
 			maxAuditCapacity,