diff --git a/pkg/registry/rbac/BUILD b/pkg/registry/rbac/BUILD index 9bab26372e..48ae4f78f8 100644 --- a/pkg/registry/rbac/BUILD +++ b/pkg/registry/rbac/BUILD @@ -5,14 +5,22 @@ licenses(["notice"]) load( "@io_bazel_rules_go//go:def.bzl", "go_library", + "go_test", ) go_library( name = "go_default_library", - srcs = ["escalation_check.go"], + srcs = [ + "escalation_check.go", + "helpers.go", + ], tags = ["automanaged"], deps = [ + "//pkg/api:go_default_library", "//pkg/apis/rbac:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/conversion:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library", "//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library", @@ -41,3 +49,16 @@ filegroup( ], tags = ["automanaged"], ) + +go_test( + name = "go_default_test", + srcs = ["helpers_test.go"], + library = ":go_default_library", + tags = ["automanaged"], + deps = [ + "//pkg/api:go_default_library", + "//pkg/api/helper:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + ], +) diff --git a/pkg/registry/rbac/clusterrole/policybased/BUILD b/pkg/registry/rbac/clusterrole/policybased/BUILD index 5da018d8d5..be6e794444 100644 --- a/pkg/registry/rbac/clusterrole/policybased/BUILD +++ b/pkg/registry/rbac/clusterrole/policybased/BUILD @@ -12,6 +12,7 @@ go_library( srcs = ["storage.go"], tags = ["automanaged"], deps = [ + "//pkg/api/helper:go_default_library", "//pkg/apis/rbac:go_default_library", "//pkg/registry/rbac:go_default_library", "//pkg/registry/rbac/validation:go_default_library", diff --git a/pkg/registry/rbac/clusterrole/policybased/storage.go b/pkg/registry/rbac/clusterrole/policybased/storage.go index 7350e91695..36650cbb78 100644 --- a/pkg/registry/rbac/clusterrole/policybased/storage.go +++ b/pkg/registry/rbac/clusterrole/policybased/storage.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" + kapihelper "k8s.io/kubernetes/pkg/api/helper" "k8s.io/kubernetes/pkg/apis/rbac" rbacregistry "k8s.io/kubernetes/pkg/registry/rbac" rbacregistryvalidation "k8s.io/kubernetes/pkg/registry/rbac/validation" @@ -60,6 +61,11 @@ func (s *Storage) Update(ctx genericapirequest.Context, name string, obj rest.Up nonEscalatingInfo := rest.WrapUpdatedObjectInfo(obj, func(ctx genericapirequest.Context, obj runtime.Object, oldObj runtime.Object) (runtime.Object, error) { clusterRole := obj.(*rbac.ClusterRole) + // if we're only mutating fields needed for the GC to eventually delete this obj, return + if rbacregistry.IsOnlyMutatingGCFields(obj, oldObj, kapihelper.Semantic) { + return obj, nil + } + rules := clusterRole.Rules if err := rbacregistryvalidation.ConfirmNoEscalation(ctx, s.ruleResolver, rules); err != nil { return nil, errors.NewForbidden(groupResource, clusterRole.Name, err) diff --git a/pkg/registry/rbac/clusterrolebinding/policybased/BUILD b/pkg/registry/rbac/clusterrolebinding/policybased/BUILD index 8d05b5bc40..8d8ebefd17 100644 --- a/pkg/registry/rbac/clusterrolebinding/policybased/BUILD +++ b/pkg/registry/rbac/clusterrolebinding/policybased/BUILD @@ -12,6 +12,7 @@ go_library( srcs = ["storage.go"], tags = ["automanaged"], deps = [ + "//pkg/api/helper:go_default_library", "//pkg/apis/rbac:go_default_library", "//pkg/registry/rbac:go_default_library", "//pkg/registry/rbac/validation:go_default_library", diff --git a/pkg/registry/rbac/clusterrolebinding/policybased/storage.go b/pkg/registry/rbac/clusterrolebinding/policybased/storage.go index 00f197a6f0..95130b8c9c 100644 --- a/pkg/registry/rbac/clusterrolebinding/policybased/storage.go +++ b/pkg/registry/rbac/clusterrolebinding/policybased/storage.go @@ -24,6 +24,7 @@ import ( "k8s.io/apiserver/pkg/authorization/authorizer" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" + kapihelper "k8s.io/kubernetes/pkg/api/helper" "k8s.io/kubernetes/pkg/apis/rbac" rbacregistry "k8s.io/kubernetes/pkg/registry/rbac" rbacregistryvalidation "k8s.io/kubernetes/pkg/registry/rbac/validation" @@ -71,6 +72,11 @@ func (s *Storage) Update(ctx genericapirequest.Context, name string, obj rest.Up nonEscalatingInfo := rest.WrapUpdatedObjectInfo(obj, func(ctx genericapirequest.Context, obj runtime.Object, oldObj runtime.Object) (runtime.Object, error) { clusterRoleBinding := obj.(*rbac.ClusterRoleBinding) + // if we're only mutating fields needed for the GC to eventually delete this obj, return + if rbacregistry.IsOnlyMutatingGCFields(obj, oldObj, kapihelper.Semantic) { + return obj, nil + } + // if we're explicitly authorized to bind this clusterrole, return if rbacregistry.BindingAuthorized(ctx, clusterRoleBinding.RoleRef, metav1.NamespaceNone, s.authorizer) { return obj, nil diff --git a/pkg/registry/rbac/helpers.go b/pkg/registry/rbac/helpers.go new file mode 100644 index 0000000000..011d12ea05 --- /dev/null +++ b/pkg/registry/rbac/helpers.go @@ -0,0 +1,48 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rbac + +import ( + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/conversion" + "k8s.io/apimachinery/pkg/runtime" + kapi "k8s.io/kubernetes/pkg/api" +) + +// IsOnlyMutatingGCFields checks finalizers and ownerrefs which GC manipulates +// and indicates that only those fields are changing +func IsOnlyMutatingGCFields(obj, old runtime.Object, equalities conversion.Equalities) bool { + // make a copy of the newObj so that we can stomp for comparison + copied, err := kapi.Scheme.Copy(obj) + if err != nil { + // if we couldn't copy, don't fail, just make it do the check + return false + } + copiedMeta, err := meta.Accessor(copied) + if err != nil { + return false + } + oldMeta, err := meta.Accessor(old) + if err != nil { + return false + } + copiedMeta.SetOwnerReferences(oldMeta.GetOwnerReferences()) + copiedMeta.SetFinalizers(oldMeta.GetFinalizers()) + copiedMeta.SetSelfLink(oldMeta.GetSelfLink()) + + return equalities.DeepEqual(copied, old) +} diff --git a/pkg/registry/rbac/helpers_test.go b/pkg/registry/rbac/helpers_test.go new file mode 100644 index 0000000000..523f2de12a --- /dev/null +++ b/pkg/registry/rbac/helpers_test.go @@ -0,0 +1,139 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rbac + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + kapi "k8s.io/kubernetes/pkg/api" + kapihelper "k8s.io/kubernetes/pkg/api/helper" +) + +func newPod() *kapi.Pod { + return &kapi.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + Name: "foo", + OwnerReferences: []metav1.OwnerReference{}, + }, + } + +} + +func TestIsOnlyMutatingGCFields(t *testing.T) { + tests := []struct { + name string + obj func() runtime.Object + old func() runtime.Object + expected bool + }{ + { + name: "same", + obj: func() runtime.Object { + return newPod() + }, + old: func() runtime.Object { + return newPod() + }, + expected: true, + }, + { + name: "only annotations", + obj: func() runtime.Object { + obj := newPod() + obj.Annotations["foo"] = "bar" + return obj + }, + old: func() runtime.Object { + return newPod() + }, + expected: false, + }, + { + name: "only other", + obj: func() runtime.Object { + obj := newPod() + obj.Spec.RestartPolicy = kapi.RestartPolicyAlways + return obj + }, + old: func() runtime.Object { + return newPod() + }, + expected: false, + }, + { + name: "only ownerRef", + obj: func() runtime.Object { + obj := newPod() + obj.OwnerReferences = append(obj.OwnerReferences, metav1.OwnerReference{Name: "foo"}) + return obj + }, + old: func() runtime.Object { + return newPod() + }, + expected: true, + }, + { + name: "ownerRef and finalizer", + obj: func() runtime.Object { + obj := newPod() + obj.OwnerReferences = append(obj.OwnerReferences, metav1.OwnerReference{Name: "foo"}) + obj.Finalizers = []string{"final"} + return obj + }, + old: func() runtime.Object { + return newPod() + }, + expected: true, + }, + { + name: "and annotations", + obj: func() runtime.Object { + obj := newPod() + obj.OwnerReferences = append(obj.OwnerReferences, metav1.OwnerReference{Name: "foo"}) + obj.Annotations["foo"] = "bar" + return obj + }, + old: func() runtime.Object { + return newPod() + }, + expected: false, + }, + { + name: "and other", + obj: func() runtime.Object { + obj := newPod() + obj.OwnerReferences = append(obj.OwnerReferences, metav1.OwnerReference{Name: "foo"}) + obj.Spec.RestartPolicy = kapi.RestartPolicyAlways + return obj + }, + old: func() runtime.Object { + return newPod() + }, + expected: false, + }, + } + + for _, tc := range tests { + actual := IsOnlyMutatingGCFields(tc.obj(), tc.old(), kapihelper.Semantic) + if tc.expected != actual { + t.Errorf("%s: expected %v, got %v", tc.name, tc.expected, actual) + } + } +} diff --git a/pkg/registry/rbac/role/policybased/BUILD b/pkg/registry/rbac/role/policybased/BUILD index 5da018d8d5..be6e794444 100644 --- a/pkg/registry/rbac/role/policybased/BUILD +++ b/pkg/registry/rbac/role/policybased/BUILD @@ -12,6 +12,7 @@ go_library( srcs = ["storage.go"], tags = ["automanaged"], deps = [ + "//pkg/api/helper:go_default_library", "//pkg/apis/rbac:go_default_library", "//pkg/registry/rbac:go_default_library", "//pkg/registry/rbac/validation:go_default_library", diff --git a/pkg/registry/rbac/role/policybased/storage.go b/pkg/registry/rbac/role/policybased/storage.go index c2b1a34977..1f2443d2b0 100644 --- a/pkg/registry/rbac/role/policybased/storage.go +++ b/pkg/registry/rbac/role/policybased/storage.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" + kapihelper "k8s.io/kubernetes/pkg/api/helper" "k8s.io/kubernetes/pkg/apis/rbac" rbacregistry "k8s.io/kubernetes/pkg/registry/rbac" rbacregistryvalidation "k8s.io/kubernetes/pkg/registry/rbac/validation" @@ -60,6 +61,11 @@ func (s *Storage) Update(ctx genericapirequest.Context, name string, obj rest.Up nonEscalatingInfo := rest.WrapUpdatedObjectInfo(obj, func(ctx genericapirequest.Context, obj runtime.Object, oldObj runtime.Object) (runtime.Object, error) { role := obj.(*rbac.Role) + // if we're only mutating fields needed for the GC to eventually delete this obj, return + if rbacregistry.IsOnlyMutatingGCFields(obj, oldObj, kapihelper.Semantic) { + return obj, nil + } + rules := role.Rules if err := rbacregistryvalidation.ConfirmNoEscalation(ctx, s.ruleResolver, rules); err != nil { return nil, errors.NewForbidden(groupResource, role.Name, err) diff --git a/pkg/registry/rbac/rolebinding/policybased/BUILD b/pkg/registry/rbac/rolebinding/policybased/BUILD index cfe041c157..2e940d1e77 100644 --- a/pkg/registry/rbac/rolebinding/policybased/BUILD +++ b/pkg/registry/rbac/rolebinding/policybased/BUILD @@ -12,6 +12,7 @@ go_library( srcs = ["storage.go"], tags = ["automanaged"], deps = [ + "//pkg/api/helper:go_default_library", "//pkg/apis/rbac:go_default_library", "//pkg/registry/rbac:go_default_library", "//pkg/registry/rbac/validation:go_default_library", diff --git a/pkg/registry/rbac/rolebinding/policybased/storage.go b/pkg/registry/rbac/rolebinding/policybased/storage.go index 7f72a0be63..5f775d5c49 100644 --- a/pkg/registry/rbac/rolebinding/policybased/storage.go +++ b/pkg/registry/rbac/rolebinding/policybased/storage.go @@ -23,6 +23,7 @@ import ( "k8s.io/apiserver/pkg/authorization/authorizer" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" + kapihelper "k8s.io/kubernetes/pkg/api/helper" "k8s.io/kubernetes/pkg/apis/rbac" rbacregistry "k8s.io/kubernetes/pkg/registry/rbac" rbacregistryvalidation "k8s.io/kubernetes/pkg/registry/rbac/validation" @@ -84,6 +85,11 @@ func (s *Storage) Update(ctx genericapirequest.Context, name string, obj rest.Up roleBinding := obj.(*rbac.RoleBinding) + // if we're only mutating fields needed for the GC to eventually delete this obj, return + if rbacregistry.IsOnlyMutatingGCFields(obj, oldObj, kapihelper.Semantic) { + return obj, nil + } + // if we're explicitly authorized to bind this role, return if rbacregistry.BindingAuthorized(ctx, roleBinding.RoleRef, namespace, s.authorizer) { return obj, nil