Merge pull request #34829 from deads2k/api-25-gc-admission

Automatic merge from submit-queue

add ownerref permission checks

Adds an admission plugin that ensures that anyone adding an `ownerReference` to a resource has delete rights on the resource they're setting up a delete for.

@caesarxuchao example admission plugin that tests for ownerReference diffs and uses an authorizer to drive the decision.

@liggitt @ncdc we've talked about this before

cmd/kube-apiserver/app/plugins.go

@@ -29,6 +29,7 @@ import (
 	_ "k8s.io/kubernetes/plugin/pkg/admission/antiaffinity"
 	_ "k8s.io/kubernetes/plugin/pkg/admission/deny"
 	_ "k8s.io/kubernetes/plugin/pkg/admission/exec"
+	_ "k8s.io/kubernetes/plugin/pkg/admission/gc"
 	_ "k8s.io/kubernetes/plugin/pkg/admission/imagepolicy"
 	_ "k8s.io/kubernetes/plugin/pkg/admission/initialresources"
 	_ "k8s.io/kubernetes/plugin/pkg/admission/limitranger"

federation/cmd/federation-apiserver/app/plugins.go

@@ -26,5 +26,6 @@ import (
 	// Admission policies
 	_ "k8s.io/kubernetes/plugin/pkg/admission/admit"
 	_ "k8s.io/kubernetes/plugin/pkg/admission/deny"
+	_ "k8s.io/kubernetes/plugin/pkg/admission/gc"
 	_ "k8s.io/kubernetes/plugin/pkg/admission/namespace/lifecycle"
 )

hack/.linted_packages

@@ -203,6 +203,7 @@ plugin/pkg/admission/admit
 plugin/pkg/admission/alwayspullimages
 plugin/pkg/admission/deny
 plugin/pkg/admission/exec
+plugin/pkg/admission/gc
 plugin/pkg/admission/imagepolicy
 plugin/pkg/admission/namespace/autoprovision
 plugin/pkg/admission/namespace/exists

plugin/pkg/admission/gc/gc_admission.go

@@ -0,0 +1,112 @@
+/*
+Copyright 2016 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 gc
+
+import (
+	"fmt"
+	"io"
+
+	"k8s.io/kubernetes/pkg/admission"
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/meta"
+	"k8s.io/kubernetes/pkg/auth/authorizer"
+	clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
+	"k8s.io/kubernetes/pkg/runtime"
+)
+
+func init() {
+	admission.RegisterPlugin("OwnerReferencesPermissionEnforcement", func(client clientset.Interface, config io.Reader) (admission.Interface, error) {
+		return &gcPermissionsEnforcement{
+			Handler: admission.NewHandler(admission.Create, admission.Update),
+		}, nil
+	})
+}
+
+// gcPermissionsEnforcement is an implementation of admission.Interface.
+type gcPermissionsEnforcement struct {
+	*admission.Handler
+
+	authorizer authorizer.Authorizer
+}
+
+func (a *gcPermissionsEnforcement) Admit(attributes admission.Attributes) (err error) {
+	// if we aren't changing owner references, then the edit is always allowed
+	if !isChangingOwnerReference(attributes.GetObject(), attributes.GetOldObject()) {
+		return nil
+	}
+
+	deleteAttributes := authorizer.AttributesRecord{
+		User:            attributes.GetUserInfo(),
+		Verb:            "delete",
+		Namespace:       attributes.GetNamespace(),
+		APIGroup:        attributes.GetResource().Group,
+		APIVersion:      attributes.GetResource().Version,
+		Resource:        attributes.GetResource().Resource,
+		Subresource:     attributes.GetSubresource(),
+		Name:            attributes.GetName(),
+		ResourceRequest: true,
+		Path:            "",
+	}
+	allowed, reason, err := a.authorizer.Authorize(deleteAttributes)
+	if allowed {
+		return nil
+	}
+
+	return admission.NewForbidden(attributes, fmt.Errorf("cannot set an ownerRef on a resource you can't delete: %v, %v", reason, err))
+}
+
+func isChangingOwnerReference(newObj, oldObj runtime.Object) bool {
+	newMeta, err := meta.Accessor(newObj)
+	if err != nil {
+		// if we don't have objectmeta, we don't have the object reference
+		return false
+	}
+
+	if oldObj == nil {
+		return len(newMeta.GetOwnerReferences()) > 0
+	}
+	oldMeta, err := meta.Accessor(oldObj)
+	if err != nil {
+		// if we don't have objectmeta, we don't have the object reference
+		return false
+	}
+
+	// compare the old and new.  If they aren't the same, then we're trying to change an ownerRef
+	oldOwners := oldMeta.GetOwnerReferences()
+	newOwners := newMeta.GetOwnerReferences()
+	if len(oldOwners) != len(newOwners) {
+		return true
+	}
+	for i := range oldOwners {
+		if !api.Semantic.DeepEqual(oldOwners[i], newOwners[i]) {
+			return true
+		}
+	}
+
+	return false
+}
+
+func (a *gcPermissionsEnforcement) SetAuthorizer(authorizer authorizer.Authorizer) {
+	a.authorizer = authorizer
+}
+
+func (a *gcPermissionsEnforcement) Validate() error {
+	if a.authorizer == nil {
+		return fmt.Errorf("missing authorizer")
+	}
+	return nil
+}

plugin/pkg/admission/gc/gc_admission_test.go

@@ -0,0 +1,216 @@
+/*
+Copyright 2016 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 gc
+
+import (
+	"testing"
+
+	"k8s.io/kubernetes/pkg/admission"
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/unversioned"
+	"k8s.io/kubernetes/pkg/auth/authorizer"
+	"k8s.io/kubernetes/pkg/auth/user"
+	"k8s.io/kubernetes/pkg/runtime"
+)
+
+type fakeAuthorizer struct{}
+
+func (fakeAuthorizer) Authorize(a authorizer.Attributes) (bool, string, error) {
+	username := a.GetUser().GetName()
+
+	if username == "non-deleter" {
+		if a.GetVerb() == "delete" {
+			return false, "", nil
+		}
+		return true, "", nil
+	}
+
+	if username == "non-pod-deleter" {
+		if a.GetVerb() == "delete" && a.GetResource() == "pods" {
+			return false, "", nil
+		}
+		return true, "", nil
+	}
+
+	return true, "", nil
+}
+
+func TestGCAdmission(t *testing.T) {
+	tests := []struct {
+		name     string
+		username string
+		resource unversioned.GroupVersionResource
+		oldObj   runtime.Object
+		newObj   runtime.Object
+
+		expectedAllowed bool
+	}{
+		{
+			name:            "super-user, create, no objectref change",
+			username:        "super",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			newObj:          &api.Pod{},
+			expectedAllowed: true,
+		},
+		{
+			name:            "super-user, create, objectref change",
+			username:        "super",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			newObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			expectedAllowed: true,
+		},
+		{
+			name:            "non-deleter, create, no objectref change",
+			username:        "non-deleter",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			newObj:          &api.Pod{},
+			expectedAllowed: true,
+		},
+		{
+			name:            "non-deleter, create, objectref change",
+			username:        "non-deleter",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			newObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			expectedAllowed: false,
+		},
+		{
+			name:            "non-pod-deleter, create, no objectref change",
+			username:        "non-pod-deleter",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			newObj:          &api.Pod{},
+			expectedAllowed: true,
+		},
+		{
+			name:            "non-pod-deleter, create, objectref change",
+			username:        "non-pod-deleter",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			newObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			expectedAllowed: false,
+		},
+		{
+			name:            "non-pod-deleter, create, objectref change, but not a pod",
+			username:        "non-pod-deleter",
+			resource:        api.SchemeGroupVersion.WithResource("not-pods"),
+			newObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			expectedAllowed: true,
+		},
+
+		{
+			name:            "super-user, update, no objectref change",
+			username:        "super",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:          &api.Pod{},
+			newObj:          &api.Pod{},
+			expectedAllowed: true,
+		},
+		{
+			name:            "super-user, update, no objectref change two",
+			username:        "super",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			newObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			expectedAllowed: true,
+		},
+		{
+			name:            "super-user, update, objectref change",
+			username:        "super",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:          &api.Pod{},
+			newObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			expectedAllowed: true,
+		},
+		{
+			name:            "non-deleter, update, no objectref change",
+			username:        "non-deleter",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:          &api.Pod{},
+			newObj:          &api.Pod{},
+			expectedAllowed: true,
+		},
+		{
+			name:            "non-deleter, update, no objectref change two",
+			username:        "non-deleter",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			newObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			expectedAllowed: true,
+		},
+		{
+			name:            "non-deleter, update, objectref change",
+			username:        "non-deleter",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:          &api.Pod{},
+			newObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			expectedAllowed: false,
+		},
+		{
+			name:            "non-deleter, update, objectref change two",
+			username:        "non-deleter",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			newObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}, {Name: "second"}}}},
+			expectedAllowed: false,
+		},
+		{
+			name:            "non-pod-deleter, update, no objectref change",
+			username:        "non-pod-deleter",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:          &api.Pod{},
+			newObj:          &api.Pod{},
+			expectedAllowed: true,
+		},
+		{
+			name:            "non-pod-deleter, update, objectref change",
+			username:        "non-pod-deleter",
+			resource:        api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:          &api.Pod{},
+			newObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			expectedAllowed: false,
+		},
+		{
+			name:            "non-pod-deleter, update, objectref change, but not a pod",
+			username:        "non-pod-deleter",
+			resource:        api.SchemeGroupVersion.WithResource("not-pods"),
+			oldObj:          &api.Pod{},
+			newObj:          &api.Pod{ObjectMeta: api.ObjectMeta{OwnerReferences: []api.OwnerReference{{Name: "first"}}}},
+			expectedAllowed: true,
+		},
+	}
+	gcAdmit := &gcPermissionsEnforcement{
+		Handler:    admission.NewHandler(admission.Create, admission.Update),
+		authorizer: fakeAuthorizer{},
+	}
+
+	for _, tc := range tests {
+		operation := admission.Create
+		if tc.oldObj != nil {
+			operation = admission.Update
+		}
+		user := &user.DefaultInfo{Name: tc.username}
+		attributes := admission.NewAttributesRecord(tc.newObj, tc.oldObj, unversioned.GroupVersionKind{}, api.NamespaceDefault, "foo", tc.resource, "", operation, user)
+
+		err := gcAdmit.Admit(attributes)
+		switch {
+		case err != nil && !tc.expectedAllowed:
+		case err != nil && tc.expectedAllowed:
+			t.Errorf("%v: unexpected err: %v", tc.name, err)
+		case err == nil && !tc.expectedAllowed:
+			t.Errorf("%v: missing err", tc.name)
+		case err == nil && tc.expectedAllowed:
+		}
+	}
+}