From e0fa78661f396f92b5d5f13a40906b4f4623a371 Mon Sep 17 00:00:00 2001
From: Jed Lejosne <jed@redhat.com>
Date: Mon, 10 Jun 2024 11:34:23 -0400
Subject: [PATCH] Collect component Role rules under operator Role instead of
 ClusterRole

Signed-off-by: Jed Lejosne <jed@redhat.com>
---
 manifests/generated/operator-csv.yaml.in      | 124 +++++++++---------
 .../rbac-operator.authorization.k8s.yaml.in   | 124 +++++++++---------
 .../resource/generate/rbac/operator.go        |  35 +++--
 .../resource/generate/rbac/operator_test.go   |  18 +++
 4 files changed, 169 insertions(+), 132 deletions(-)

diff --git a/manifests/generated/operator-csv.yaml.in b/manifests/generated/operator-csv.yaml.in
index d6fc92e61..29759029e 100644
--- a/manifests/generated/operator-csv.yaml.in
+++ b/manifests/generated/operator-csv.yaml.in
@@ -464,14 +464,6 @@ spec:
           - create
           - list
           - get
-        - apiGroups:
-          - ""
-          resources:
-          - configmaps
-          verbs:
-          - get
-          - list
-          - watch
         - apiGroups:
           - ""
           resources:
@@ -721,42 +713,6 @@ spec:
           verbs:
           - list
           - watch
-        - apiGroups:
-          - route.openshift.io
-          resources:
-          - routes
-          verbs:
-          - list
-          - get
-          - watch
-        - apiGroups:
-          - ""
-          resources:
-          - secrets
-          verbs:
-          - list
-          - get
-          - watch
-        - apiGroups:
-          - networking.k8s.io
-          resources:
-          - ingresses
-          verbs:
-          - list
-          - get
-          - watch
-        - apiGroups:
-          - coordination.k8s.io
-          resources:
-          - leases
-          verbs:
-          - get
-          - list
-          - watch
-          - delete
-          - update
-          - create
-          - patch
         - apiGroups:
           - kubevirt.io
           resources:
@@ -813,14 +769,6 @@ spec:
           - get
           - list
           - watch
-        - apiGroups:
-          - ""
-          resources:
-          - configmaps
-          verbs:
-          - get
-          - list
-          - watch
         - apiGroups:
           - export.kubevirt.io
           resources:
@@ -836,16 +784,6 @@ spec:
           verbs:
           - list
           - watch
-        - apiGroups:
-          - ""
-          resourceNames:
-          - kubevirt-export-ca
-          resources:
-          - configmaps
-          verbs:
-          - get
-          - list
-          - watch
         - apiGroups:
           - subresources.kubevirt.io
           resources:
@@ -1438,6 +1376,68 @@ spec:
           - update
           - create
           - patch
+        - apiGroups:
+          - ""
+          resources:
+          - configmaps
+          verbs:
+          - get
+          - list
+          - watch
+        - apiGroups:
+          - route.openshift.io
+          resources:
+          - routes
+          verbs:
+          - list
+          - get
+          - watch
+        - apiGroups:
+          - ""
+          resources:
+          - secrets
+          verbs:
+          - list
+          - get
+          - watch
+        - apiGroups:
+          - networking.k8s.io
+          resources:
+          - ingresses
+          verbs:
+          - list
+          - get
+          - watch
+        - apiGroups:
+          - coordination.k8s.io
+          resources:
+          - leases
+          verbs:
+          - get
+          - list
+          - watch
+          - delete
+          - update
+          - create
+          - patch
+        - apiGroups:
+          - ""
+          resources:
+          - configmaps
+          verbs:
+          - get
+          - list
+          - watch
+        - apiGroups:
+          - ""
+          resourceNames:
+          - kubevirt-export-ca
+          resources:
+          - configmaps
+          verbs:
+          - get
+          - list
+          - watch
         serviceAccountName: kubevirt-operator
     strategy: deployment
   installModes:
diff --git a/manifests/generated/rbac-operator.authorization.k8s.yaml.in b/manifests/generated/rbac-operator.authorization.k8s.yaml.in
index 847c290d3..be10854c0 100644
--- a/manifests/generated/rbac-operator.authorization.k8s.yaml.in
+++ b/manifests/generated/rbac-operator.authorization.k8s.yaml.in
@@ -75,6 +75,68 @@ rules:
   - update
   - create
   - patch
+- apiGroups:
+  - ""
+  resources:
+  - configmaps
+  verbs:
+  - get
+  - list
+  - watch
+- apiGroups:
+  - route.openshift.io
+  resources:
+  - routes
+  verbs:
+  - list
+  - get
+  - watch
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - list
+  - get
+  - watch
+- apiGroups:
+  - networking.k8s.io
+  resources:
+  - ingresses
+  verbs:
+  - list
+  - get
+  - watch
+- apiGroups:
+  - coordination.k8s.io
+  resources:
+  - leases
+  verbs:
+  - get
+  - list
+  - watch
+  - delete
+  - update
+  - create
+  - patch
+- apiGroups:
+  - ""
+  resources:
+  - configmaps
+  verbs:
+  - get
+  - list
+  - watch
+- apiGroups:
+  - ""
+  resourceNames:
+  - kubevirt-export-ca
+  resources:
+  - configmaps
+  verbs:
+  - get
+  - list
+  - watch
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
@@ -404,14 +466,6 @@ rules:
   - create
   - list
   - get
-- apiGroups:
-  - ""
-  resources:
-  - configmaps
-  verbs:
-  - get
-  - list
-  - watch
 - apiGroups:
   - ""
   resources:
@@ -661,42 +715,6 @@ rules:
   verbs:
   - list
   - watch
-- apiGroups:
-  - route.openshift.io
-  resources:
-  - routes
-  verbs:
-  - list
-  - get
-  - watch
-- apiGroups:
-  - ""
-  resources:
-  - secrets
-  verbs:
-  - list
-  - get
-  - watch
-- apiGroups:
-  - networking.k8s.io
-  resources:
-  - ingresses
-  verbs:
-  - list
-  - get
-  - watch
-- apiGroups:
-  - coordination.k8s.io
-  resources:
-  - leases
-  verbs:
-  - get
-  - list
-  - watch
-  - delete
-  - update
-  - create
-  - patch
 - apiGroups:
   - kubevirt.io
   resources:
@@ -753,14 +771,6 @@ rules:
   - get
   - list
   - watch
-- apiGroups:
-  - ""
-  resources:
-  - configmaps
-  verbs:
-  - get
-  - list
-  - watch
 - apiGroups:
   - export.kubevirt.io
   resources:
@@ -776,16 +786,6 @@ rules:
   verbs:
   - list
   - watch
-- apiGroups:
-  - ""
-  resourceNames:
-  - kubevirt-export-ca
-  resources:
-  - configmaps
-  verbs:
-  - get
-  - list
-  - watch
 - apiGroups:
   - subresources.kubevirt.io
   resources:
diff --git a/pkg/virt-operator/resource/generate/rbac/operator.go b/pkg/virt-operator/resource/generate/rbac/operator.go
index 365fb0600..b90a5fae8 100644
--- a/pkg/virt-operator/resource/generate/rbac/operator.go
+++ b/pkg/virt-operator/resource/generate/rbac/operator.go
@@ -317,15 +317,14 @@ func NewOperatorClusterRole() *rbacv1.ClusterRole {
 	}
 
 	// now append all rules needed by KubeVirt's components
-	operatorRole.Rules = append(operatorRole.Rules, getKubeVirtComponentsRules()...)
+	operatorRole.Rules = append(operatorRole.Rules, getKubeVirtComponentsClusterRules()...)
 	return operatorRole
 }
 
-func getKubeVirtComponentsRules() []rbacv1.PolicyRule {
-
+func getKubeVirtComponentsClusterRules() []rbacv1.PolicyRule {
 	var rules []rbacv1.PolicyRule
 
-	// namespace doesn't matter, we are only interested in the rules of both Roles and ClusterRoles
+	// namespace doesn't matter, we are only interested in the rules of ClusterRoles
 	all := GetAllApiServer("")
 	all = append(all, GetAllController("")...)
 	all = append(all, GetAllHandler("")...)
@@ -337,9 +336,6 @@ func getKubeVirtComponentsRules() []rbacv1.PolicyRule {
 		case *rbacv1.ClusterRole:
 			role, _ := resource.(*rbacv1.ClusterRole)
 			rules = append(rules, role.Rules...)
-		case *rbacv1.Role:
-			role, _ := resource.(*rbacv1.Role)
-			rules = append(rules, role.Rules...)
 		}
 	}
 
@@ -375,6 +371,27 @@ func getKubeVirtComponentsRules() []rbacv1.PolicyRule {
 	return rules
 }
 
+func getKubeVirtComponentsRules() []rbacv1.PolicyRule {
+	var rules []rbacv1.PolicyRule
+
+	// namespace doesn't matter, we are only interested in the rules
+	all := GetAllApiServer("")
+	all = append(all, GetAllController("")...)
+	all = append(all, GetAllHandler("")...)
+	all = append(all, GetAllExportProxy("")...)
+	all = append(all, GetAllCluster()...)
+
+	for _, resource := range all {
+		switch resource.(type) {
+		case *rbacv1.Role:
+			role, _ := resource.(*rbacv1.Role)
+			rules = append(rules, role.Rules...)
+		}
+	}
+
+	return rules
+}
+
 func newOperatorClusterRoleBinding(namespace string) *rbacv1.ClusterRoleBinding {
 	return &rbacv1.ClusterRoleBinding{
 		TypeMeta: metav1.TypeMeta{
@@ -432,7 +449,7 @@ func newOperatorRoleBinding(namespace string) *rbacv1.RoleBinding {
 
 // NewOperatorRole creates a Role object for kubevirt-operator.
 func NewOperatorRole(namespace string) *rbacv1.Role {
-	return &rbacv1.Role{
+	operatorRole := &rbacv1.Role{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: VersionNamev1,
 			Kind:       "Role",
@@ -527,6 +544,8 @@ func NewOperatorRole(namespace string) *rbacv1.Role {
 			},
 		},
 	}
+	operatorRole.Rules = append(operatorRole.Rules, getKubeVirtComponentsRules()...)
+	return operatorRole
 }
 
 func GetKubevirtComponentsServiceAccounts(namespace string) map[string]bool {
diff --git a/pkg/virt-operator/resource/generate/rbac/operator_test.go b/pkg/virt-operator/resource/generate/rbac/operator_test.go
index 51bd479cc..22c7d30c0 100644
--- a/pkg/virt-operator/resource/generate/rbac/operator_test.go
+++ b/pkg/virt-operator/resource/generate/rbac/operator_test.go
@@ -67,6 +67,11 @@ var _ = Describe("RBAC", func() {
 			Expect(clusterRoleBinding.Subjects[0].Namespace).To(BeEquivalentTo(expectedNamespace))
 		})
 
+		It("doesn't have critical cluster-wide permissions", func() {
+			clusterRole := getFirstItemOfType(forOperator, reflect.TypeOf(&rbacv1.ClusterRole{})).(*rbacv1.ClusterRole)
+			Expect(clusterRole).ToNot(BeNil())
+			expectExactRuleDoesntExists(clusterRole.Rules, "", "secrets", "get", "list", "watch")
+		})
 	})
 
 	Context("GetKubevirtComponentsServiceAccounts", func() {
@@ -96,3 +101,16 @@ func getFirstItemOfType(items []interface{}, tp reflect.Type) interface{} {
 	}
 	return nil
 }
+
+func expectExactRuleDoesntExists(rules []rbacv1.PolicyRule, apiGroup, resource string, verbs ...string) {
+	for _, rule := range rules {
+		if contains(rule.APIGroups, apiGroup) &&
+			contains(rule.Resources, resource) {
+			for _, verb := range verbs {
+				if contains(rule.Verbs, verb) {
+					Fail(fmt.Sprintf("Found rule (apiGroup: %s, resource: %s, verbs: %v)", apiGroup, resource, rule.Verbs))
+				}
+			}
+		}
+	}
+}
-- 
2.45.2

