From 5c3d1c9724f6c8752ecfc9c98f7a434ed2af7b07 Mon Sep 17 00:00:00 2001 From: David Eads Date: Wed, 6 Sep 2017 14:20:07 -0400 Subject: [PATCH 1/2] UPSTREAM: 49133: add controller permissions to set blockOwnerDeletion --- .../plugin/pkg/admission/gc/gc_admission.go | 5 +- .../pkg/admission/gc/gc_admission_test.go | 14 ++++- .../rbac/bootstrappolicy/controller_policy.go | 23 +++++--- .../testdata/controller-roles.yaml | 53 ++++++++++++++++--- 4 files changed, 77 insertions(+), 18 deletions(-) diff --git a/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission.go b/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission.go index 93834cb040a1..5c4287925b65 100644 --- a/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission.go +++ b/vendor/k8s.io/kubernetes/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: "", diff --git a/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission_test.go b/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission_test.go index 4d7a6aac6a44..691be3da3b66 100644 --- a/vendor/k8s.io/kubernetes/plugin/pkg/admission/gc/gc_admission_test.go +++ b/vendor/k8s.io/kubernetes/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 diff --git a/vendor/k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go b/vendor/k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go index a040a311f24d..942fc0ac0ed0 100644 --- a/vendor/k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go +++ b/vendor/k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go @@ -71,9 +71,10 @@ func init() { addControllerRole(rbac.ClusterRole{ ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "cronjob-controller"}, Rules: []rbac.PolicyRule{ - rbac.NewRule("get", "list", "watch", "update", "delete").Groups(batchGroup).Resources("cronjobs").RuleOrDie(), + 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(), }, @@ -81,8 +82,9 @@ func init() { addControllerRole(rbac.ClusterRole{ ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "daemon-set-controller"}, Rules: []rbac.PolicyRule{ - rbac.NewRule("get", "list", "watch", "delete").Groups(extensionsGroup).Resources("daemonsets").RuleOrDie(), - rbac.NewRule("update").Groups(extensionsGroup).Resources("daemonsets/status").RuleOrDie(), + 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(), @@ -93,8 +95,9 @@ func init() { addControllerRole(rbac.ClusterRole{ ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "deployment-controller"}, Rules: []rbac.PolicyRule{ - rbac.NewRule("get", "list", "watch", "update", "delete").Groups(extensionsGroup, appsGroup).Resources("deployments").RuleOrDie(), + 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. @@ -151,8 +154,9 @@ func init() { addControllerRole(rbac.ClusterRole{ ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "job-controller"}, Rules: []rbac.PolicyRule{ - rbac.NewRule("get", "list", "watch", "update", "delete").Groups(batchGroup).Resources("jobs").RuleOrDie(), + 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(), }, @@ -208,8 +212,9 @@ func init() { addControllerRole(rbac.ClusterRole{ ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "replicaset-controller"}, Rules: []rbac.PolicyRule{ - rbac.NewRule("get", "list", "watch", "update", "delete").Groups(extensionsGroup).Resources("replicasets").RuleOrDie(), + 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(), }, @@ -218,8 +223,9 @@ func init() { ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "replication-controller"}, Rules: []rbac.PolicyRule{ // 1.0 controllers needed get, update, so without these old controllers break on new servers - rbac.NewRule("get", "list", "watch", "update", "delete").Groups(legacyGroup).Resources("replicationcontrollers").RuleOrDie(), + 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(), }, @@ -261,8 +267,9 @@ func init() { ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + "statefulset-controller"}, Rules: []rbac.PolicyRule{ rbac.NewRule("list", "watch").Groups(legacyGroup).Resources("pods").RuleOrDie(), - rbac.NewRule("get", "list", "watch", "delete").Groups(appsGroup).Resources("statefulsets").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(), diff --git a/vendor/k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml b/vendor/k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml index fd67e7b7a41f..1a56a0a90185 100644 --- a/vendor/k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml +++ b/vendor/k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/controller-roles.yaml @@ -102,7 +102,6 @@ items: resources: - cronjobs verbs: - - delete - get - list - update @@ -125,6 +124,12 @@ items: - cronjobs/status verbs: - update + - apiGroups: + - batch + resources: + - cronjobs/finalizers + verbs: + - update - apiGroups: - "" resources: @@ -151,20 +156,28 @@ items: name: system:controller:daemon-set-controller rules: - apiGroups: + - apps - extensions resources: - daemonsets verbs: - - delete - get - list - watch - apiGroups: + - apps - extensions resources: - daemonsets/status verbs: - update + - apiGroups: + - apps + - extensions + resources: + - daemonsets/finalizers + verbs: + - update - apiGroups: - "" resources: @@ -223,7 +236,6 @@ items: resources: - deployments verbs: - - delete - get - list - update @@ -235,6 +247,13 @@ items: - deployments/status verbs: - update + - apiGroups: + - apps + - extensions + resources: + - deployments/finalizers + verbs: + - update - apiGroups: - extensions resources: @@ -495,7 +514,6 @@ items: resources: - jobs verbs: - - delete - get - list - update @@ -506,6 +524,12 @@ items: - jobs/status verbs: - update + - apiGroups: + - batch + resources: + - jobs/finalizers + verbs: + - update - apiGroups: - "" resources: @@ -742,7 +766,6 @@ items: resources: - replicasets verbs: - - delete - get - list - update @@ -753,6 +776,12 @@ items: - replicasets/status verbs: - update + - apiGroups: + - extensions + resources: + - replicasets/finalizers + verbs: + - update - apiGroups: - "" resources: @@ -786,7 +815,6 @@ items: resources: - replicationcontrollers verbs: - - delete - get - list - update @@ -797,6 +825,12 @@ items: - replicationcontrollers/status verbs: - update + - apiGroups: + - "" + resources: + - replicationcontrollers/finalizers + verbs: + - update - apiGroups: - "" resources: @@ -962,7 +996,6 @@ items: resources: - statefulsets verbs: - - delete - get - list - watch @@ -972,6 +1005,12 @@ items: - statefulsets/status verbs: - update + - apiGroups: + - apps + resources: + - statefulsets/finalizers + verbs: + - update - apiGroups: - "" resources: From 0818d5700f29ab66dcba88130b108c8c505224bc Mon Sep 17 00:00:00 2001 From: David Eads Date: Wed, 6 Sep 2017 14:20:54 -0400 Subject: [PATCH 2/2] update controller roles --- .../bootstrappolicy/controller_policy.go | 4 +- .../bootstrap_cluster_roles.yaml | 68 ++++++++++++++++--- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/server/bootstrappolicy/controller_policy.go b/pkg/cmd/server/bootstrappolicy/controller_policy.go index 1004a6ba346a..61a3cd9c2e7d 100644 --- a/pkg/cmd/server/bootstrappolicy/controller_policy.go +++ b/pkg/cmd/server/bootstrappolicy/controller_policy.go @@ -93,6 +93,7 @@ func init() { ObjectMeta: metav1.ObjectMeta{Name: saRolePrefix + InfraBuildControllerServiceAccountName}, Rules: []rbac.PolicyRule{ rbac.NewRule("get", "list", "watch", "patch", "update", "delete").Groups(buildGroup, legacyBuildGroup).Resources("builds").RuleOrDie(), + rbac.NewRule("update").Groups(buildGroup, legacyBuildGroup).Resources("builds/finalizers").RuleOrDie(), rbac.NewRule("get").Groups(buildGroup, legacyBuildGroup).Resources("buildconfigs").RuleOrDie(), rbac.NewRule("create").Groups(buildGroup, legacyBuildGroup).Resources("builds/optimizeddocker", "builds/docker", "builds/source", "builds/custom", "builds/jenkinspipeline").RuleOrDie(), rbac.NewRule("get", "list").Groups(imageGroup, legacyImageGroup).Resources("imagestreams").RuleOrDie(), @@ -136,7 +137,8 @@ func init() { Rules: []rbac.PolicyRule{ rbac.NewRule("create", "get", "list", "watch", "update", "patch", "delete").Groups(kapiGroup).Resources("replicationcontrollers").RuleOrDie(), rbac.NewRule("update").Groups(deployGroup, legacyDeployGroup).Resources("deploymentconfigs/status").RuleOrDie(), - rbac.NewRule("get", "list", "watch", "delete").Groups(deployGroup, legacyDeployGroup).Resources("deploymentconfigs").RuleOrDie(), + rbac.NewRule("update").Groups(deployGroup, legacyDeployGroup).Resources("deploymentconfigs/finalizers").RuleOrDie(), + rbac.NewRule("get", "list", "watch").Groups(deployGroup, legacyDeployGroup).Resources("deploymentconfigs").RuleOrDie(), eventsRule(), }, }) diff --git a/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml b/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml index 252e80fdc902..cde4fd8dd37a 100644 --- a/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml +++ b/test/testdata/bootstrappolicy/bootstrap_cluster_roles.yaml @@ -2784,6 +2784,13 @@ items: - patch - update - watch + - apiGroups: + - "" + - build.openshift.io + resources: + - builds/finalizers + verbs: + - update - apiGroups: - "" - build.openshift.io @@ -2955,13 +2962,19 @@ items: - deploymentconfigs/status verbs: - update + - apiGroups: + - "" + - apps.openshift.io + resources: + - deploymentconfigs/finalizers + verbs: + - update - apiGroups: - "" - apps.openshift.io resources: - deploymentconfigs verbs: - - delete - get - list - watch @@ -3799,7 +3812,6 @@ items: resources: - cronjobs verbs: - - delete - get - list - update @@ -3822,6 +3834,12 @@ items: - cronjobs/status verbs: - update + - apiGroups: + - batch + resources: + - cronjobs/finalizers + verbs: + - update - apiGroups: - "" resources: @@ -3849,20 +3867,28 @@ items: name: system:controller:daemon-set-controller rules: - apiGroups: + - apps - extensions resources: - daemonsets verbs: - - delete - get - list - watch - apiGroups: + - apps - extensions resources: - daemonsets/status verbs: - update + - apiGroups: + - apps + - extensions + resources: + - daemonsets/finalizers + verbs: + - update - apiGroups: - "" resources: @@ -3922,7 +3948,6 @@ items: resources: - deployments verbs: - - delete - get - list - update @@ -3934,6 +3959,13 @@ items: - deployments/status verbs: - update + - apiGroups: + - apps + - extensions + resources: + - deployments/finalizers + verbs: + - update - apiGroups: - extensions resources: @@ -4199,7 +4231,6 @@ items: resources: - jobs verbs: - - delete - get - list - update @@ -4210,6 +4241,12 @@ items: - jobs/status verbs: - update + - apiGroups: + - batch + resources: + - jobs/finalizers + verbs: + - update - apiGroups: - "" resources: @@ -4451,7 +4488,6 @@ items: resources: - replicasets verbs: - - delete - get - list - update @@ -4462,6 +4498,12 @@ items: - replicasets/status verbs: - update + - apiGroups: + - extensions + resources: + - replicasets/finalizers + verbs: + - update - apiGroups: - "" resources: @@ -4496,7 +4538,6 @@ items: resources: - replicationcontrollers verbs: - - delete - get - list - update @@ -4507,6 +4548,12 @@ items: - replicationcontrollers/status verbs: - update + - apiGroups: + - "" + resources: + - replicationcontrollers/finalizers + verbs: + - update - apiGroups: - "" resources: @@ -4677,7 +4724,6 @@ items: resources: - statefulsets verbs: - - delete - get - list - watch @@ -4687,6 +4733,12 @@ items: - statefulsets/status verbs: - update + - apiGroups: + - apps + resources: + - statefulsets/finalizers + verbs: + - update - apiGroups: - "" resources: