Merge pull request #49133 from deads2k/gc-02-permissions

Automatic merge from submit-queue (batch tested with PRs 49133, 51557, 51749, 50842, 52018)

add controller permissions to set blockOwnerDeletion

fixes https://github.com/kubernetes/kubernetes/issues/51970

`blockOwnerDeletion` requires delete permissions on the owner object.  This adds that permission for our controllers.

@kubernetes/sig-auth-misc 

```release-note
The OwnerReferencesPermissionEnforcement admission plugin now requires `update` permission on the `finalizers` subresource of the referenced owner in order to set `blockOwnerDeletion` on an owner reference.
```

plugin/pkg/admission/gc/gc_admission.go

@@ -122,7 +122,7 @@ func (a *gcPermissionsEnforcement) Admit(attributes admission.Attributes) (err e
 		for _, record := range records {
 			allowed, reason, err := a.authorizer.Authorize(record)
 			if !allowed {
-				return admission.NewForbidden(attributes, fmt.Errorf("cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't delete: %v, %v", reason, err))
+				return admission.NewForbidden(attributes, fmt.Errorf("cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on: %v, %v", reason, err))
 			}
 		}
 	}
@@ -178,12 +178,13 @@ func (a *gcPermissionsEnforcement) ownerRefToDeleteAttributeRecords(ref metav1.O
 	for _, mapping := range mappings {
 		ret = append(ret, authorizer.AttributesRecord{
 			User: attributes.GetUserInfo(),
-			Verb: "delete",
+			Verb: "update",
 			// ownerReference can only refer to an object in the same namespace, so attributes.GetNamespace() equals to the owner's namespace
 			Namespace:       attributes.GetNamespace(),
 			APIGroup:        groupVersion.Group,
 			APIVersion:      groupVersion.Version,
 			Resource:        mapping.Resource,
+			Subresource:     "finalizers",
 			Name:            ref.Name,
 			ResourceRequest: true,
 			Path:            "",

plugin/pkg/admission/gc/gc_admission_test.go

@@ -39,6 +39,9 @@ func (fakeAuthorizer) Authorize(a authorizer.Attributes) (bool, string, error) {
 		if a.GetVerb() == "delete" {
 			return false, "", nil
 		}
+		if a.GetVerb() == "update" && a.GetSubresource() == "finalizers" {
+			return false, "", nil
+		}
 		return true, "", nil
 	}
 
@@ -46,6 +49,9 @@ func (fakeAuthorizer) Authorize(a authorizer.Attributes) (bool, string, error) {
 		if a.GetVerb() == "delete" && a.GetResource() == "pods" {
 			return false, "", nil
 		}
+		if a.GetVerb() == "update" && a.GetResource() == "pods" && a.GetSubresource() == "finalizers" {
+			return false, "", nil
+		}
 		return true, "", nil
 	}
 
@@ -53,6 +59,9 @@ func (fakeAuthorizer) Authorize(a authorizer.Attributes) (bool, string, error) {
 		if a.GetVerb() == "delete" && a.GetResource() == "replicationcontrollers" {
 			return false, "", nil
 		}
+		if a.GetVerb() == "update" && a.GetResource() == "replicationcontrollers" && a.GetSubresource() == "finalizers" {
+			return false, "", nil
+		}
 		return true, "", nil
 	}
 
@@ -326,7 +335,10 @@ func TestBlockOwnerDeletionAdmission(t *testing.T) {
 		return err == nil
 	}
 	expectCantSetBlockOwnerDeletionError := func(err error) bool {
-		return strings.Contains(err.Error(), "cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't delete")
+		if err == nil {
+			return false
+		}
+		return strings.Contains(err.Error(), "cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on")
 	}
 	tests := []struct {
 		name        string

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

@@ -74,6 +74,7 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) {
 			rbac.NewRule("get", "list", "watch", "update").Groups(batchGroup).Resources("cronjobs").RuleOrDie(),
 			rbac.NewRule("get", "list", "watch", "create", "update", "delete", "patch").Groups(batchGroup).Resources("jobs").RuleOrDie(),
 			rbac.NewRule("update").Groups(batchGroup).Resources("cronjobs/status").RuleOrDie(),
+			rbac.NewRule("update").Groups(batchGroup).Resources("cronjobs/finalizers").RuleOrDie(),
 			rbac.NewRule("list", "delete").Groups(legacyGroup).Resources("pods").RuleOrDie(),
 			eventsRule(),
 		},
@@ -83,6 +84,7 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) {
 		Rules: []rbac.PolicyRule{
 			rbac.NewRule("get", "list", "watch").Groups(extensionsGroup, appsGroup).Resources("daemonsets").RuleOrDie(),
 			rbac.NewRule("update").Groups(extensionsGroup, appsGroup).Resources("daemonsets/status").RuleOrDie(),
+			rbac.NewRule("update").Groups(extensionsGroup, appsGroup).Resources("daemonsets/finalizers").RuleOrDie(),
 			rbac.NewRule("list", "watch").Groups(legacyGroup).Resources("nodes").RuleOrDie(),
 			rbac.NewRule("list", "watch", "create", "delete", "patch").Groups(legacyGroup).Resources("pods").RuleOrDie(),
 			rbac.NewRule("create").Groups(legacyGroup).Resources("pods/binding").RuleOrDie(),
@@ -95,6 +97,7 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) {
 		Rules: []rbac.PolicyRule{
 			rbac.NewRule("get", "list", "watch", "update").Groups(extensionsGroup, appsGroup).Resources("deployments").RuleOrDie(),
 			rbac.NewRule("update").Groups(extensionsGroup, appsGroup).Resources("deployments/status").RuleOrDie(),
+			rbac.NewRule("update").Groups(extensionsGroup, appsGroup).Resources("deployments/finalizers").RuleOrDie(),
 			rbac.NewRule("get", "list", "watch", "create", "update", "patch", "delete").Groups(extensionsGroup).Resources("replicasets").RuleOrDie(),
 			// TODO: remove "update" once
 			// https://github.com/kubernetes/kubernetes/issues/36897 is resolved.
@@ -168,6 +171,7 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) {
 		Rules: []rbac.PolicyRule{
 			rbac.NewRule("get", "list", "watch", "update").Groups(batchGroup).Resources("jobs").RuleOrDie(),
 			rbac.NewRule("update").Groups(batchGroup).Resources("jobs/status").RuleOrDie(),
+			rbac.NewRule("update").Groups(batchGroup).Resources("jobs/finalizers").RuleOrDie(),
 			rbac.NewRule("list", "watch", "create", "delete", "patch").Groups(legacyGroup).Resources("pods").RuleOrDie(),
 			eventsRule(),
 		},
@@ -225,6 +229,7 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) {
 		Rules: []rbac.PolicyRule{
 			rbac.NewRule("get", "list", "watch", "update").Groups(extensionsGroup).Resources("replicasets").RuleOrDie(),
 			rbac.NewRule("update").Groups(extensionsGroup).Resources("replicasets/status").RuleOrDie(),
+			rbac.NewRule("update").Groups(extensionsGroup).Resources("replicasets/finalizers").RuleOrDie(),
 			rbac.NewRule("list", "watch", "patch", "create", "delete").Groups(legacyGroup).Resources("pods").RuleOrDie(),
 			eventsRule(),
 		},
@@ -235,6 +240,7 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) {
 			// 1.0 controllers needed get, update, so without these old controllers break on new servers
 			rbac.NewRule("get", "list", "watch", "update").Groups(legacyGroup).Resources("replicationcontrollers").RuleOrDie(),
 			rbac.NewRule("update").Groups(legacyGroup).Resources("replicationcontrollers/status").RuleOrDie(),
+			rbac.NewRule("update").Groups(legacyGroup).Resources("replicationcontrollers/finalizers").RuleOrDie(),
 			rbac.NewRule("list", "watch", "patch", "create", "delete").Groups(legacyGroup).Resources("pods").RuleOrDie(),
 			eventsRule(),
 		},
@@ -278,6 +284,7 @@ func buildControllerRoles() ([]rbac.ClusterRole, []rbac.ClusterRoleBinding) {
 			rbac.NewRule("list", "watch").Groups(legacyGroup).Resources("pods").RuleOrDie(),
 			rbac.NewRule("get", "list", "watch").Groups(appsGroup).Resources("statefulsets").RuleOrDie(),
 			rbac.NewRule("update").Groups(appsGroup).Resources("statefulsets/status").RuleOrDie(),
+			rbac.NewRule("update").Groups(appsGroup).Resources("statefulsets/finalizers").RuleOrDie(),
 			rbac.NewRule("get", "create", "delete", "update", "patch").Groups(legacyGroup).Resources("pods").RuleOrDie(),
 			rbac.NewRule("get", "create", "delete", "update", "patch", "list", "watch").Groups(appsGroup).Resources("controllerrevisions").RuleOrDie(),
 			rbac.NewRule("get", "create").Groups(legacyGroup).Resources("persistentvolumeclaims").RuleOrDie(),

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

@@ -124,6 +124,12 @@ items:
     - cronjobs/status
     verbs:
     - update
+  - apiGroups:
+    - batch
+    resources:
+    - cronjobs/finalizers
+    verbs:
+    - update
   - apiGroups:
     - ""
     resources:
@@ -165,6 +171,13 @@ items:
     - daemonsets/status
     verbs:
     - update
+  - apiGroups:
+    - apps
+    - extensions
+    resources:
+    - daemonsets/finalizers
+    verbs:
+    - update
   - apiGroups:
     - ""
     resources:
@@ -234,6 +247,13 @@ items:
     - deployments/status
     verbs:
     - update
+  - apiGroups:
+    - apps
+    - extensions
+    resources:
+    - deployments/finalizers
+    verbs:
+    - update
   - apiGroups:
     - extensions
     resources:
@@ -495,6 +515,12 @@ items:
     - jobs/status
     verbs:
     - update
+  - apiGroups:
+    - batch
+    resources:
+    - jobs/finalizers
+    verbs:
+    - update
   - apiGroups:
     - ""
     resources:
@@ -741,6 +767,12 @@ items:
     - replicasets/status
     verbs:
     - update
+  - apiGroups:
+    - extensions
+    resources:
+    - replicasets/finalizers
+    verbs:
+    - update
   - apiGroups:
     - ""
     resources:
@@ -784,6 +816,12 @@ items:
     - replicationcontrollers/status
     verbs:
     - update
+  - apiGroups:
+    - ""
+    resources:
+    - replicationcontrollers/finalizers
+    verbs:
+    - update
   - apiGroups:
     - ""
     resources:
@@ -958,6 +996,12 @@ items:
     - statefulsets/status
     verbs:
     - update
+  - apiGroups:
+    - apps
+    resources:
+    - statefulsets/finalizers
+    verbs:
+    - update
   - apiGroups:
     - ""
     resources: