Merge pull request #130461 from kariya-mitsuru/automated-cherry-pick-of-#130405-upstream-release-1.32

Automated cherry pick of #130405: Add Watch to controller roles

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go

@@ -95,7 +95,7 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding)
 			rbacv1helpers.NewRule("get", "list", "watch", "create", "update", "delete", "patch").Groups(batchGroup).Resources("jobs").RuleOrDie(),
 			rbacv1helpers.NewRule("update").Groups(batchGroup).Resources("cronjobs/status").RuleOrDie(),
 			rbacv1helpers.NewRule("update").Groups(batchGroup).Resources("cronjobs/finalizers").RuleOrDie(),
-			rbacv1helpers.NewRule("list", "delete").Groups(legacyGroup).Resources("pods").RuleOrDie(),
+			rbacv1helpers.NewRule("list", "watch", "delete").Groups(legacyGroup).Resources("pods").RuleOrDie(),
 			eventsRule(),
 		},
 	})
@@ -146,7 +146,7 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding)
 		ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "endpoint-controller"},
 		Rules: []rbacv1.PolicyRule{
 			rbacv1helpers.NewRule("get", "list", "watch").Groups(legacyGroup).Resources("services", "pods").RuleOrDie(),
-			rbacv1helpers.NewRule("get", "list", "create", "update", "delete").Groups(legacyGroup).Resources("endpoints").RuleOrDie(),
+			rbacv1helpers.NewRule("get", "list", "watch", "create", "update", "delete").Groups(legacyGroup).Resources("endpoints").RuleOrDie(),
 			rbacv1helpers.NewRule("create").Groups(legacyGroup).Resources("endpoints/restricted").RuleOrDie(),
 			eventsRule(),
 		},
@@ -159,7 +159,7 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding)
 			// The controller needs to be able to set a service's finalizers to be able to create an EndpointSlice
 			// resource that is owned by the service and sets blockOwnerDeletion=true in its ownerRef.
 			rbacv1helpers.NewRule("update").Groups(legacyGroup).Resources("services/finalizers").RuleOrDie(),
-			rbacv1helpers.NewRule("get", "list", "create", "update", "delete").Groups(discoveryGroup).Resources("endpointslices").RuleOrDie(),
+			rbacv1helpers.NewRule("get", "list", "watch", "create", "update", "delete").Groups(discoveryGroup).Resources("endpointslices").RuleOrDie(),
 			eventsRule(),
 		},
 	})
@@ -175,7 +175,7 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding)
 			// resource that is owned by the endpoint and sets blockOwnerDeletion=true in its ownerRef.
 			// see https://github.com/openshift/kubernetes/blob/8691466059314c3f7d6dcffcbb76d14596ca716c/pkg/controller/endpointslicemirroring/utils.go#L87-L88
 			rbacv1helpers.NewRule("update").Groups(legacyGroup).Resources("endpoints/finalizers").RuleOrDie(),
-			rbacv1helpers.NewRule("get", "list", "create", "update", "delete").Groups(discoveryGroup).Resources("endpointslices").RuleOrDie(),
+			rbacv1helpers.NewRule("get", "list", "watch", "create", "update", "delete").Groups(discoveryGroup).Resources("endpointslices").RuleOrDie(),
 			eventsRule(),
 		},
 	})
@@ -231,11 +231,11 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding)
 			rbacv1helpers.NewRule("get", "list", "watch").Groups(autoscalingGroup).Resources("horizontalpodautoscalers").RuleOrDie(),
 			rbacv1helpers.NewRule("update").Groups(autoscalingGroup).Resources("horizontalpodautoscalers/status").RuleOrDie(),
 			rbacv1helpers.NewRule("get", "update").Groups("*").Resources("*/scale").RuleOrDie(),
-			rbacv1helpers.NewRule("list").Groups(legacyGroup).Resources("pods").RuleOrDie(),
+			rbacv1helpers.NewRule("list", "watch").Groups(legacyGroup).Resources("pods").RuleOrDie(),
 			// allow listing resource, custom, and external metrics
-			rbacv1helpers.NewRule("list").Groups(resMetricsGroup).Resources("pods").RuleOrDie(),
-			rbacv1helpers.NewRule("get", "list").Groups(customMetricsGroup).Resources("*").RuleOrDie(),
-			rbacv1helpers.NewRule("get", "list").Groups(externalMetricsGroup).Resources("*").RuleOrDie(),
+			rbacv1helpers.NewRule("list", "watch").Groups(resMetricsGroup).Resources("pods").RuleOrDie(),
+			rbacv1helpers.NewRule("get", "list", "watch").Groups(customMetricsGroup).Resources("*").RuleOrDie(),
+			rbacv1helpers.NewRule("get", "list", "watch").Groups(externalMetricsGroup).Resources("*").RuleOrDie(),
 			eventsRule(),
 		},
 	})
@@ -261,11 +261,11 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding)
 		role := rbacv1.ClusterRole{
 			ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "node-controller"},
 			Rules: []rbacv1.PolicyRule{
-				rbacv1helpers.NewRule("get", "list", "update", "delete", "patch").Groups(legacyGroup).Resources("nodes").RuleOrDie(),
+				rbacv1helpers.NewRule("get", "list", "watch", "update", "delete", "patch").Groups(legacyGroup).Resources("nodes").RuleOrDie(),
 				rbacv1helpers.NewRule("patch", "update").Groups(legacyGroup).Resources("nodes/status").RuleOrDie(),
 				// used for pod deletion
 				rbacv1helpers.NewRule("patch", "update").Groups(legacyGroup).Resources("pods/status").RuleOrDie(),
-				rbacv1helpers.NewRule("list", "get", "delete").Groups(legacyGroup).Resources("pods").RuleOrDie(),
+				rbacv1helpers.NewRule("list", "watch", "get", "delete").Groups(legacyGroup).Resources("pods").RuleOrDie(),
 				eventsRule(),
 			},
 		}
@@ -295,7 +295,7 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding)
 			ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "pod-garbage-collector"},
 			Rules: []rbacv1.PolicyRule{
 				rbacv1helpers.NewRule("list", "watch", "delete").Groups(legacyGroup).Resources("pods").RuleOrDie(),
-				rbacv1helpers.NewRule("get", "list").Groups(legacyGroup).Resources("nodes").RuleOrDie(),
+				rbacv1helpers.NewRule("get", "list", "watch").Groups(legacyGroup).Resources("nodes").RuleOrDie(),
 				rbacv1helpers.NewRule("patch").Groups(legacyGroup).Resources("pods/status").RuleOrDie(),
 			},
 		}
@@ -507,7 +507,7 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding)
 				// need list to get current RV for any resource
 				// need patch for SSA of any resource
 				// need create because SSA of a deleted resource will be interpreted as a create request, these always fail with a conflict error because UID is set
-				rbacv1helpers.NewRule("list", "create", "patch").Groups("*").Resources("*").RuleOrDie(),
+				rbacv1helpers.NewRule("list", "watch", "create", "patch").Groups("*").Resources("*").RuleOrDie(),
 				rbacv1helpers.NewRule("update").Groups(storageVersionMigrationGroup).Resources("storageversionmigrations/status").RuleOrDie(),
 			},
 		})

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy_test.go

@@ -18,6 +18,7 @@ package bootstrappolicy
 
 import (
 	"reflect"
+	"slices"
 	"testing"
 
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -91,3 +92,15 @@ func TestControllerRoleLabel(t *testing.T) {
 		}
 	}
 }
+
+func TestControllerRoleVerbsConsistency(t *testing.T) {
+	roles := ControllerRoles()
+	for _, role := range roles {
+		for _, rule := range role.Rules {
+			verbs := rule.Verbs
+			if slices.Contains(verbs, "list") && !slices.Contains(verbs, "watch") {
+				t.Errorf("The ClusterRole %s has Verb `List` but does not have Verb `Watch`.", role.Name)
+			}
+		}
+	}
+}

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy_test.go

@@ -20,6 +20,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"slices"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -285,3 +286,39 @@ func TestClusterRoleLabel(t *testing.T) {
 		}
 	}
 }
+
+func TestNodeRuleVerbsConsistency(t *testing.T) {
+	rules := bootstrappolicy.NodeRules()
+	for _, rule := range rules {
+		verbs := rule.Verbs
+		if slices.Contains(verbs, "list") && !slices.Contains(verbs, "watch") {
+			t.Errorf("The NodeRule has Verb `List` but does not have Verb `Watch`.")
+		}
+	}
+}
+
+func TestClusterRoleVerbsConsistency(t *testing.T) {
+	roles := bootstrappolicy.ClusterRoles()
+	for _, role := range roles {
+		for _, rule := range role.Rules {
+			verbs := rule.Verbs
+			if slices.Contains(verbs, "list") && !slices.Contains(verbs, "watch") {
+				t.Errorf("The ClusterRole %s has Verb `List` but does not have Verb `Watch`.", role.Name)
+			}
+		}
+	}
+}
+
+func TestNamespaceRoleVerbsConsistency(t *testing.T) {
+	namespaceRoles := bootstrappolicy.NamespaceRoles()
+	for namespace, roles := range namespaceRoles {
+		for _, role := range roles {
+			for _, rule := range role.Rules {
+				verbs := rule.Verbs
+				if slices.Contains(verbs, "list") && !slices.Contains(verbs, "watch") {
+					t.Errorf("The Role %s/%s has Verb `List` but does not have Verb `Watch`.", namespace, role.Name)
+				}
+			}
+		}
+	}
+}

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml

@@ -206,6 +206,7 @@ items:
     verbs:
     - delete
     - list
+    - watch
   - apiGroups:
     - ""
     - events.k8s.io
@@ -466,6 +467,7 @@ items:
     - get
     - list
     - update
+    - watch
   - apiGroups:
     - ""
     resources:
@@ -517,6 +519,7 @@ items:
     - get
     - list
     - update
+    - watch
   - apiGroups:
     - ""
     - events.k8s.io
@@ -567,6 +570,7 @@ items:
     - get
     - list
     - update
+    - watch
   - apiGroups:
     - ""
     - events.k8s.io
@@ -735,12 +739,14 @@ items:
     - pods
     verbs:
     - list
+    - watch
   - apiGroups:
     - metrics.k8s.io
     resources:
     - pods
     verbs:
     - list
+    - watch
   - apiGroups:
     - custom.metrics.k8s.io
     resources:
@@ -748,6 +754,7 @@ items:
     verbs:
     - get
     - list
+    - watch
   - apiGroups:
     - external.metrics.k8s.io
     resources:
@@ -755,6 +762,7 @@ items:
     verbs:
     - get
     - list
+    - watch
   - apiGroups:
     - ""
     - events.k8s.io
@@ -896,6 +904,7 @@ items:
     - list
     - patch
     - update
+    - watch
   - apiGroups:
     - ""
     resources:
@@ -918,6 +927,7 @@ items:
     - delete
     - get
     - list
+    - watch
   - apiGroups:
     - ""
     - events.k8s.io
@@ -1040,6 +1050,7 @@ items:
     verbs:
     - get
     - list
+    - watch
   - apiGroups:
     - ""
     resources: