Merge pull request #43876 from caesarxuchao/blockOwnerDeletion-admission

Automatic merge from submit-queue (batch tested with PRs 44440, 44038, 44302, 44316, 43876)

Extend the gc admission plugin to check ownerReference.blockOwnerDeletion

#Extend the gc admission plugin to prevent user who doesn't have delete permission of the *owner* from changing blockOwnerDeletion field of existing ownerReferences, or adding ownerReference with blockOwnerDeletion=true.

The plugin need a RESTMapper to translate ownerRef.Kind to Resource. It should be using a dynamic one. However, as discussed in https://github.com/kubernetes/kubernetes/pull/42615, such a RESTMapper will be built after watchable discovery API is implemented, so in this PR the plugin is using the `api.Registry.RESTMapper()`, which is also [used](https://github.com/kubernetes/kubernetes/blob/master/cmd/kube-controller-manager/app/core.go#L165-L166) by the garbage collector currently.

```release-note
Extending the gc admission plugin so that a user who doesn't have delete permission of the *owner* cannot modify blockOwnerDeletion field of existing ownerReferences, or add new ownerReference with blockOwnerDeletion=true
```

cc @lavalamp

cmd/kube-apiserver/app/server.go

@@ -384,7 +384,9 @@ func BuildAdmission(s *options.ServerRunOptions, plugins *admission.Plugins, cli
 			glog.Fatalf("Error reading from cloud configuration file %s: %#v", s.CloudProvider.CloudConfigFile, err)
 		}
 	}
-	pluginInitializer := kubeapiserveradmission.NewPluginInitializer(client, sharedInformers, apiAuthorizer, cloudConfig)
+	// TODO: use a dynamic restmapper. See https://github.com/kubernetes/kubernetes/pull/42615.
+	restMapper := api.Registry.RESTMapper()
+	pluginInitializer := kubeapiserveradmission.NewPluginInitializer(client, sharedInformers, apiAuthorizer, cloudConfig, restMapper)
 	admissionConfigProvider, err := admission.ReadAdmissionConfiguration(admissionControlPluginNames, s.GenericServerRunOptions.AdmissionControlConfigFile)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read plugin config: %v", err)

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

@@ -194,7 +194,7 @@ func NonBlockingRun(s *options.ServerRunOptions, stopCh <-chan struct{}) error {
 			glog.Fatalf("Error reading from cloud configuration file %s: %#v", s.CloudProvider.CloudConfigFile, err)
 		}
 	}
-	pluginInitializer := kubeapiserveradmission.NewPluginInitializer(client, sharedInformers, apiAuthorizer, cloudConfig)
+	pluginInitializer := kubeapiserveradmission.NewPluginInitializer(client, sharedInformers, apiAuthorizer, cloudConfig, nil)
 	admissionConfigProvider, err := admission.ReadAdmissionConfiguration(admissionControlPluginNames, s.GenericServerRunOptions.AdmissionControlConfigFile)
 	if err != nil {
 		return fmt.Errorf("failed to read plugin config: %v", err)

pkg/kubeapiserver/admission/BUILD

@@ -29,6 +29,7 @@ go_library(
     deps = [
         "//pkg/client/clientset_generated/internalclientset:go_default_library",
         "//pkg/client/informers/informers_generated/internalversion:go_default_library",
+        "//vendor:k8s.io/apimachinery/pkg/api/meta",
         "//vendor:k8s.io/apiserver/pkg/admission",
         "//vendor:k8s.io/apiserver/pkg/authorization/authorizer",
     ],

pkg/kubeapiserver/admission/init_test.go

@@ -51,7 +51,7 @@ var _ WantsAuthorizer = &WantAuthorizerAdmission{}
 // TestWantsAuthorizer ensures that the authorizer is injected when the WantsAuthorizer
 // interface is implemented.
 func TestWantsAuthorizer(t *testing.T) {
-	initializer := NewPluginInitializer(nil, nil, &TestAuthorizer{}, nil)
+	initializer := NewPluginInitializer(nil, nil, &TestAuthorizer{}, nil, nil)
 	wantAuthorizerAdmission := &WantAuthorizerAdmission{}
 	initializer.Initialize(wantAuthorizerAdmission)
 	if wantAuthorizerAdmission.auth == nil {
@@ -73,7 +73,7 @@ func (self *WantsCloudConfigAdmissionPlugin) Validate() error
 
 func TestCloudConfigAdmissionPlugin(t *testing.T) {
 	cloudConfig := []byte("cloud-configuration")
-	initializer := NewPluginInitializer(nil, nil, &TestAuthorizer{}, cloudConfig)
+	initializer := NewPluginInitializer(nil, nil, &TestAuthorizer{}, cloudConfig, nil)
 	wantsCloudConfigAdmission := &WantsCloudConfigAdmissionPlugin{}
 	initializer.Initialize(wantsCloudConfigAdmission)
 

pkg/kubeapiserver/admission/initializer.go

@@ -17,6 +17,7 @@ limitations under the License.
 package admission
 
 import (
+	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
@@ -48,22 +49,29 @@ type WantsCloudConfig interface {
 	SetCloudConfig([]byte)
 }
 
+// WantsRESTMapper defines a function which sets RESTMapper for admission plugins that need it.
+type WantsRESTMapper interface {
+	SetRESTMapper(meta.RESTMapper)
+}
+
 type pluginInitializer struct {
 	internalClient internalclientset.Interface
 	informers      informers.SharedInformerFactory
 	authorizer     authorizer.Authorizer
 	cloudConfig    []byte
+	restMapper     meta.RESTMapper
 }
 
 var _ admission.PluginInitializer = pluginInitializer{}
 
 // NewPluginInitializer constructs new instance of PluginInitializer
-func NewPluginInitializer(internalClient internalclientset.Interface, sharedInformers informers.SharedInformerFactory, authz authorizer.Authorizer, cloudConfig []byte) admission.PluginInitializer {
+func NewPluginInitializer(internalClient internalclientset.Interface, sharedInformers informers.SharedInformerFactory, authz authorizer.Authorizer, cloudConfig []byte, restMapper meta.RESTMapper) admission.PluginInitializer {
 	return pluginInitializer{
 		internalClient: internalClient,
 		informers:      sharedInformers,
 		authorizer:     authz,
 		cloudConfig:    cloudConfig,
+		restMapper:     restMapper,
 	}
 }
 
@@ -85,4 +93,8 @@ func (i pluginInitializer) Initialize(plugin admission.Interface) {
 	if wants, ok := plugin.(WantsCloudConfig); ok {
 		wants.SetCloudConfig(i.cloudConfig)
 	}
+
+	if wants, ok := plugin.(WantsRESTMapper); ok {
+		wants.SetRESTMapper(i.restMapper)
+	}
 }

plugin/pkg/admission/gc/BUILD

@@ -16,7 +16,10 @@ go_library(
         "//pkg/kubeapiserver/admission:go_default_library",
         "//vendor:k8s.io/apimachinery/pkg/api/equality",
         "//vendor:k8s.io/apimachinery/pkg/api/meta",
+        "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
         "//vendor:k8s.io/apimachinery/pkg/runtime",
+        "//vendor:k8s.io/apimachinery/pkg/runtime/schema",
+        "//vendor:k8s.io/apimachinery/pkg/types",
         "//vendor:k8s.io/apiserver/pkg/admission",
         "//vendor:k8s.io/apiserver/pkg/authorization/authorizer",
     ],
@@ -29,6 +32,7 @@ go_test(
     tags = ["automanaged"],
     deps = [
         "//pkg/api:go_default_library",
+        "//pkg/kubeapiserver/admission:go_default_library",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
         "//vendor:k8s.io/apimachinery/pkg/runtime",
         "//vendor:k8s.io/apimachinery/pkg/runtime/schema",

plugin/pkg/admission/gc/gc_admission.go

@@ -22,7 +22,10 @@ import (
 
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
@@ -41,6 +44,8 @@ type gcPermissionsEnforcement struct {
 	*admission.Handler
 
 	authorizer authorizer.Authorizer
+
+	restMapper meta.RESTMapper
 }
 
 func (a *gcPermissionsEnforcement) Admit(attributes admission.Attributes) (err error) {
@@ -62,11 +67,32 @@ func (a *gcPermissionsEnforcement) Admit(attributes admission.Attributes) (err e
 		Path:            "",
 	}
 	allowed, reason, err := a.authorizer.Authorize(deleteAttributes)
-	if allowed {
-		return nil
+	if !allowed {
+		return admission.NewForbidden(attributes, fmt.Errorf("cannot set an ownerRef on a resource you can't delete: %v, %v", reason, err))
 	}
 
-	return admission.NewForbidden(attributes, fmt.Errorf("cannot set an ownerRef on a resource you can't delete: %v, %v", reason, err))
+	// Further check if the user is setting ownerReference.blockOwnerDeletion to
+	// true. If so, only allows the change if the user has delete permission of
+	// the _OWNER_
+	newBlockingRefs := newBlockingOwnerDeletionRefs(attributes.GetObject(), attributes.GetOldObject())
+	for _, ref := range newBlockingRefs {
+		records, err := a.ownerRefToDeleteAttributeRecords(ref, attributes)
+		if err != nil {
+			return admission.NewForbidden(attributes, fmt.Errorf("cannot set blockOwnerDeletion in this case because cannot find RESTMapping for APIVersion %s Kind %s: %v, %v", ref.APIVersion, ref.Kind, reason, err))
+		}
+		// Multiple records are returned if ref.Kind could map to multiple
+		// resources. User needs to have delete permission on all the
+		// matched Resources.
+		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 nil
+
 }
 
 func isChangingOwnerReference(newObj, oldObj runtime.Object) bool {
@@ -100,13 +126,109 @@ func isChangingOwnerReference(newObj, oldObj runtime.Object) bool {
 	return false
 }
 
+// Translates ref to a DeleteAttribute deleting the object referred by the ref.
+// OwnerReference only records the object kind, which might map to multiple
+// resources, so multiple DeleteAttribute might be returned.
+func (a *gcPermissionsEnforcement) ownerRefToDeleteAttributeRecords(ref metav1.OwnerReference, attributes admission.Attributes) ([]authorizer.AttributesRecord, error) {
+	var ret []authorizer.AttributesRecord
+	groupVersion, err := schema.ParseGroupVersion(ref.APIVersion)
+	if err != nil {
+		return ret, err
+	}
+	mappings, err := a.restMapper.RESTMappings(schema.GroupKind{Group: groupVersion.Group, Kind: ref.Kind}, groupVersion.Version)
+	if err != nil {
+		return ret, err
+	}
+	for _, mapping := range mappings {
+		ret = append(ret, authorizer.AttributesRecord{
+			User: attributes.GetUserInfo(),
+			Verb: "delete",
+			// 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,
+			Name:            ref.Name,
+			ResourceRequest: true,
+			Path:            "",
+		})
+	}
+	return ret, nil
+}
+
+// only keeps the blocking refs
+func blockingOwnerRefs(refs []metav1.OwnerReference) []metav1.OwnerReference {
+	var ret []metav1.OwnerReference
+	for _, ref := range refs {
+		if ref.BlockOwnerDeletion != nil && *ref.BlockOwnerDeletion == true {
+			ret = append(ret, ref)
+		}
+	}
+	return ret
+}
+
+func indexByUID(refs []metav1.OwnerReference) map[types.UID]metav1.OwnerReference {
+	ret := make(map[types.UID]metav1.OwnerReference)
+	for _, ref := range refs {
+		ret[ref.UID] = ref
+	}
+	return ret
+}
+
+// Returns new blocking ownerReferences, and references whose blockOwnerDeletion
+// field is changed from nil or false to true.
+func newBlockingOwnerDeletionRefs(newObj, oldObj runtime.Object) []metav1.OwnerReference {
+	newMeta, err := meta.Accessor(newObj)
+	if err != nil {
+		// if we don't have objectmeta, we don't have the object reference
+		return nil
+	}
+	newRefs := newMeta.GetOwnerReferences()
+	blockingNewRefs := blockingOwnerRefs(newRefs)
+	if len(blockingNewRefs) == 0 {
+		return nil
+	}
+
+	if oldObj == nil {
+		return blockingNewRefs
+	}
+	oldMeta, err := meta.Accessor(oldObj)
+	if err != nil {
+		// if we don't have objectmeta, treat it as if all the ownerReference are newly created
+		return blockingNewRefs
+	}
+
+	var ret []metav1.OwnerReference
+	indexedOldRefs := indexByUID(oldMeta.GetOwnerReferences())
+	for _, ref := range blockingNewRefs {
+		oldRef, ok := indexedOldRefs[ref.UID]
+		if !ok {
+			// if ref is newly added, and it's blocking, then returns it.
+			ret = append(ret, ref)
+			continue
+		}
+		wasNotBlocking := oldRef.BlockOwnerDeletion == nil || *oldRef.BlockOwnerDeletion == false
+		if wasNotBlocking {
+			ret = append(ret, ref)
+		}
+	}
+	return ret
+}
+
 func (a *gcPermissionsEnforcement) SetAuthorizer(authorizer authorizer.Authorizer) {
 	a.authorizer = authorizer
 }
 
+func (a *gcPermissionsEnforcement) SetRESTMapper(restMapper meta.RESTMapper) {
+	a.restMapper = restMapper
+}
+
 func (a *gcPermissionsEnforcement) Validate() error {
 	if a.authorizer == nil {
 		return fmt.Errorf("missing authorizer")
 	}
+	if a.restMapper == nil {
+		return fmt.Errorf("missing restMapper")
+	}
 	return nil
 }

plugin/pkg/admission/gc/gc_admission_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package gc
 
 import (
+	"strings"
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -26,6 +27,7 @@ import (
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/kubernetes/pkg/api"
+	kubeadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
 )
 
 type fakeAuthorizer struct{}
@@ -47,10 +49,33 @@ func (fakeAuthorizer) Authorize(a authorizer.Attributes) (bool, string, error) {
 		return true, "", nil
 	}
 
+	if username == "non-rc-deleter" {
+		if a.GetVerb() == "delete" && a.GetResource() == "replicationcontrollers" {
+			return false, "", nil
+		}
+		return true, "", nil
+	}
+
 	return true, "", nil
 }
 
+// newGCPermissionsEnforcement returns the admission controller configured for testing.
+func newGCPermissionsEnforcement() *gcPermissionsEnforcement {
+	gcAdmit := &gcPermissionsEnforcement{
+		Handler: admission.NewHandler(admission.Create, admission.Update),
+	}
+	pluginInitializer := kubeadmission.NewPluginInitializer(nil, nil, fakeAuthorizer{}, nil, api.Registry.RESTMapper())
+	pluginInitializer.Initialize(gcAdmit)
+	return gcAdmit
+}
+
 func TestGCAdmission(t *testing.T) {
+	expectNoError := func(err error) bool {
+		return err == nil
+	}
+	expectCantSetOwnerRefError := func(err error) bool {
+		return strings.Contains(err.Error(), "cannot set an ownerRef on a resource you can't delete")
+	}
 	tests := []struct {
 		name     string
 		username string
@@ -58,143 +83,140 @@ func TestGCAdmission(t *testing.T) {
 		oldObj   runtime.Object
 		newObj   runtime.Object
 
-		expectedAllowed bool
+		checkError func(error) bool
 	}{
 		{
-			name:            "super-user, create, no objectref change",
-			username:        "super",
-			resource:        api.SchemeGroupVersion.WithResource("pods"),
-			newObj:          &api.Pod{},
-			expectedAllowed: true,
+			name:       "super-user, create, no objectref change",
+			username:   "super",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     &api.Pod{},
+			checkError: expectNoError,
 		},
 		{
-			name:            "super-user, create, objectref change",
-			username:        "super",
-			resource:        api.SchemeGroupVersion.WithResource("pods"),
-			newObj:          &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
-			expectedAllowed: true,
+			name:       "super-user, create, objectref change",
+			username:   "super",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			checkError: expectNoError,
 		},
 		{
-			name:            "non-deleter, create, no objectref change",
-			username:        "non-deleter",
-			resource:        api.SchemeGroupVersion.WithResource("pods"),
-			newObj:          &api.Pod{},
-			expectedAllowed: true,
+			name:       "non-deleter, create, no objectref change",
+			username:   "non-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     &api.Pod{},
+			checkError: expectNoError,
 		},
 		{
-			name:            "non-deleter, create, objectref change",
-			username:        "non-deleter",
-			resource:        api.SchemeGroupVersion.WithResource("pods"),
-			newObj:          &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
-			expectedAllowed: false,
+			name:       "non-deleter, create, objectref change",
+			username:   "non-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			checkError: expectCantSetOwnerRefError,
 		},
 		{
-			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, no objectref change",
+			username:   "non-pod-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     &api.Pod{},
+			checkError: expectNoError,
 		},
 		{
-			name:            "non-pod-deleter, create, objectref change",
-			username:        "non-pod-deleter",
-			resource:        api.SchemeGroupVersion.WithResource("pods"),
-			newObj:          &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
-			expectedAllowed: false,
+			name:       "non-pod-deleter, create, objectref change",
+			username:   "non-pod-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			checkError: expectCantSetOwnerRefError,
 		},
 		{
-			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: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
-			expectedAllowed: true,
+			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: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			checkError: expectNoError,
 		},
 
 		{
-			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",
+			username:   "super",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     &api.Pod{},
+			newObj:     &api.Pod{},
+			checkError: expectNoError,
 		},
 		{
-			name:            "super-user, update, no objectref change two",
-			username:        "super",
-			resource:        api.SchemeGroupVersion.WithResource("pods"),
-			oldObj:          &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
-			newObj:          &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
-			expectedAllowed: true,
+			name:       "super-user, update, no objectref change two",
+			username:   "super",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			newObj:     &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			checkError: expectNoError,
 		},
 		{
-			name:            "super-user, update, objectref change",
-			username:        "super",
-			resource:        api.SchemeGroupVersion.WithResource("pods"),
-			oldObj:          &api.Pod{},
-			newObj:          &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.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: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			checkError: expectNoError,
 		},
 		{
-			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",
+			username:   "non-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     &api.Pod{},
+			newObj:     &api.Pod{},
+			checkError: expectNoError,
 		},
 		{
-			name:            "non-deleter, update, no objectref change two",
-			username:        "non-deleter",
-			resource:        api.SchemeGroupVersion.WithResource("pods"),
-			oldObj:          &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
-			newObj:          &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
-			expectedAllowed: true,
+			name:       "non-deleter, update, no objectref change two",
+			username:   "non-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			newObj:     &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			checkError: expectNoError,
 		},
 		{
-			name:            "non-deleter, update, objectref change",
-			username:        "non-deleter",
-			resource:        api.SchemeGroupVersion.WithResource("pods"),
-			oldObj:          &api.Pod{},
-			newObj:          &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
-			expectedAllowed: false,
+			name:       "non-deleter, update, objectref change",
+			username:   "non-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     &api.Pod{},
+			newObj:     &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			checkError: expectCantSetOwnerRefError,
 		},
 		{
-			name:            "non-deleter, update, objectref change two",
-			username:        "non-deleter",
-			resource:        api.SchemeGroupVersion.WithResource("pods"),
-			oldObj:          &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
-			newObj:          &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}, {Name: "second"}}}},
-			expectedAllowed: false,
+			name:       "non-deleter, update, objectref change two",
+			username:   "non-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			newObj:     &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}, {Name: "second"}}}},
+			checkError: expectCantSetOwnerRefError,
 		},
 		{
-			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, no objectref change",
+			username:   "non-pod-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     &api.Pod{},
+			newObj:     &api.Pod{},
+			checkError: expectNoError,
 		},
 		{
-			name:            "non-pod-deleter, update, objectref change",
-			username:        "non-pod-deleter",
-			resource:        api.SchemeGroupVersion.WithResource("pods"),
-			oldObj:          &api.Pod{},
-			newObj:          &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
-			expectedAllowed: false,
+			name:       "non-pod-deleter, update, objectref change",
+			username:   "non-pod-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     &api.Pod{},
+			newObj:     &api.Pod{ObjectMeta: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			checkError: expectCantSetOwnerRefError,
 		},
 		{
-			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: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
-			expectedAllowed: true,
+			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: metav1.ObjectMeta{OwnerReferences: []metav1.OwnerReference{{Name: "first"}}}},
+			checkError: expectNoError,
 		},
 	}
-	gcAdmit := &gcPermissionsEnforcement{
-		Handler:    admission.NewHandler(admission.Create, admission.Update),
-		authorizer: fakeAuthorizer{},
-	}
+	gcAdmit := newGCPermissionsEnforcement()
 
 	for _, tc := range tests {
 		operation := admission.Create
@@ -205,13 +227,250 @@ func TestGCAdmission(t *testing.T) {
 		attributes := admission.NewAttributesRecord(tc.newObj, tc.oldObj, schema.GroupVersionKind{}, metav1.NamespaceDefault, "foo", tc.resource, "", operation, user)
 
 		err := gcAdmit.Admit(attributes)
-		switch {
-		case err != nil && !tc.expectedAllowed:
-		case err != nil && tc.expectedAllowed:
+		if !tc.checkError(err) {
+			t.Errorf("%v: unexpected err: %v", tc.name, err)
+		}
+	}
+}
+
+func TestBlockOwnerDeletionAdmission(t *testing.T) {
+	podWithOwnerRefs := func(refs ...metav1.OwnerReference) *api.Pod {
+		var refSlice []metav1.OwnerReference
+		for _, ref := range refs {
+			refSlice = append(refSlice, ref)
+		}
+		return &api.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				OwnerReferences: refSlice,
+			},
+		}
+	}
+
+	getTrueVar := func() *bool {
+		ret := true
+		return &ret
+	}
+
+	getFalseVar := func() *bool {
+		ret := false
+		return &ret
+	}
+	blockRC1 := metav1.OwnerReference{
+		APIVersion:         "v1",
+		Kind:               "ReplicationController",
+		Name:               "rc1",
+		BlockOwnerDeletion: getTrueVar(),
+	}
+	blockRC2 := metav1.OwnerReference{
+		APIVersion:         "v1",
+		Kind:               "ReplicationController",
+		Name:               "rc2",
+		BlockOwnerDeletion: getTrueVar(),
+	}
+	notBlockRC1 := metav1.OwnerReference{
+		APIVersion:         "v1",
+		Kind:               "ReplicationController",
+		Name:               "rc1",
+		BlockOwnerDeletion: getFalseVar(),
+	}
+	notBlockRC2 := metav1.OwnerReference{
+		APIVersion:         "v1",
+		Kind:               "ReplicationController",
+		Name:               "rc2",
+		BlockOwnerDeletion: getFalseVar(),
+	}
+	nilBlockRC1 := metav1.OwnerReference{
+		APIVersion: "v1",
+		Kind:       "ReplicationController",
+		Name:       "rc1",
+	}
+	nilBlockRC2 := metav1.OwnerReference{
+		APIVersion: "v1",
+		Kind:       "ReplicationController",
+		Name:       "rc2",
+	}
+	blockDS1 := metav1.OwnerReference{
+		APIVersion:         "extensions/v1beta1",
+		Kind:               "DaemonSet",
+		Name:               "ds1",
+		BlockOwnerDeletion: getTrueVar(),
+	}
+	notBlockDS1 := metav1.OwnerReference{
+		APIVersion:         "extensions/v1beta1",
+		Kind:               "DaemonSet",
+		Name:               "ds1",
+		BlockOwnerDeletion: getFalseVar(),
+	}
+
+	expectNoError := func(err error) bool {
+		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")
+	}
+	tests := []struct {
+		name     string
+		username string
+		resource schema.GroupVersionResource
+		oldObj   runtime.Object
+		newObj   runtime.Object
+
+		checkError func(error) bool
+	}{
+		// cases for create
+		{
+			name:       "super-user, create, no ownerReferences",
+			username:   "super",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     podWithOwnerRefs(),
+			checkError: expectNoError,
+		},
+		{
+			name:       "super-user, create, all ownerReferences have blockOwnerDeletion=false",
+			username:   "super",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     podWithOwnerRefs(notBlockRC1, notBlockRC2),
+			checkError: expectNoError,
+		},
+		{
+			name:       "super-user, create, some ownerReferences have blockOwnerDeletion=true",
+			username:   "super",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     podWithOwnerRefs(blockRC1, blockRC2),
+			checkError: expectNoError,
+		},
+		{
+			name:       "non-rc-deleter, create, no ownerReferences",
+			username:   "non-rc-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     podWithOwnerRefs(),
+			checkError: expectNoError,
+		},
+		{
+			name:       "non-rc-deleter, create, all ownerReferences have blockOwnerDeletion=false or nil",
+			username:   "non-rc-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     podWithOwnerRefs(notBlockRC1, nilBlockRC2),
+			checkError: expectNoError,
+		},
+		{
+			name:       "non-rc-deleter, create, some ownerReferences have blockOwnerDeletion=true",
+			username:   "non-rc-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     podWithOwnerRefs(blockRC1, notBlockRC2),
+			checkError: expectCantSetBlockOwnerDeletionError,
+		},
+		{
+			name:       "non-rc-deleter, create, some ownerReferences have blockOwnerDeletion=true, but are pointing to daemonset",
+			username:   "non-rc-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			newObj:     podWithOwnerRefs(blockDS1),
+			checkError: expectNoError,
+		},
+		// cases are for update
+		{
+			name:       "super-user, update, no ownerReferences change blockOwnerDeletion",
+			username:   "super",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     podWithOwnerRefs(nilBlockRC1),
+			newObj:     podWithOwnerRefs(notBlockRC1),
+			checkError: expectNoError,
+		},
+		{
+			name:       "super-user, update, some ownerReferences change to blockOwnerDeletion=true",
+			username:   "super",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     podWithOwnerRefs(notBlockRC1),
+			newObj:     podWithOwnerRefs(blockRC1),
+			checkError: expectNoError,
+		},
+		{
+			name:       "super-user, update, add new ownerReferences with blockOwnerDeletion=true",
+			username:   "super",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     podWithOwnerRefs(),
+			newObj:     podWithOwnerRefs(blockRC1),
+			checkError: expectNoError,
+		},
+		{
+			name:       "non-rc-deleter, update, no ownerReferences change blockOwnerDeletion",
+			username:   "non-rc-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     podWithOwnerRefs(nilBlockRC1),
+			newObj:     podWithOwnerRefs(notBlockRC1),
+			checkError: expectNoError,
+		},
+		{
+			name:       "non-rc-deleter, update, some ownerReferences change from blockOwnerDeletion=false to true",
+			username:   "non-rc-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     podWithOwnerRefs(notBlockRC1),
+			newObj:     podWithOwnerRefs(blockRC1),
+			checkError: expectCantSetBlockOwnerDeletionError,
+		},
+		{
+			name:       "non-rc-deleter, update, some ownerReferences change from blockOwnerDeletion=nil to true",
+			username:   "non-rc-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     podWithOwnerRefs(nilBlockRC1),
+			newObj:     podWithOwnerRefs(blockRC1),
+			checkError: expectCantSetBlockOwnerDeletionError,
+		},
+		{
+			name:       "non-rc-deleter, update, some ownerReferences change from blockOwnerDeletion=true to false",
+			username:   "non-rc-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     podWithOwnerRefs(blockRC1),
+			newObj:     podWithOwnerRefs(notBlockRC1),
+			checkError: expectNoError,
+		},
+		{
+			name:       "non-rc-deleter, update, some ownerReferences change blockOwnerDeletion, but all such references are to daemonset",
+			username:   "non-rc-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     podWithOwnerRefs(notBlockDS1),
+			newObj:     podWithOwnerRefs(blockDS1),
+			checkError: expectNoError,
+		},
+		{
+			name:       "non-rc-deleter, update, add new ownerReferences with blockOwnerDeletion=nil or false",
+			username:   "non-rc-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     podWithOwnerRefs(),
+			newObj:     podWithOwnerRefs(notBlockRC1, nilBlockRC2),
+			checkError: expectNoError,
+		},
+		{
+			name:       "non-rc-deleter, update, add new ownerReferences with blockOwnerDeletion=true",
+			username:   "non-rc-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     podWithOwnerRefs(),
+			newObj:     podWithOwnerRefs(blockRC1),
+			checkError: expectCantSetBlockOwnerDeletionError,
+		},
+		{
+			name:       "non-rc-deleter, update, add new ownerReferences with blockOwnerDeletion=true, but the references are to daemonset",
+			username:   "non-rc-deleter",
+			resource:   api.SchemeGroupVersion.WithResource("pods"),
+			oldObj:     podWithOwnerRefs(),
+			newObj:     podWithOwnerRefs(blockDS1),
+			checkError: expectNoError,
+		},
+	}
+
+	gcAdmit := newGCPermissionsEnforcement()
+
+	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, schema.GroupVersionKind{}, metav1.NamespaceDefault, "foo", tc.resource, "", operation, user)
+
+		err := gcAdmit.Admit(attributes)
+		if !tc.checkError(err) {
 			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:
 		}
 	}
 }

plugin/pkg/admission/limitranger/admission_test.go

@@ -595,7 +595,7 @@ func newHandlerForTest(c clientset.Interface) (admission.Interface, informers.Sh
 	if err != nil {
 		return nil, f, err
 	}
-	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil)
+	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil, nil)
 	pluginInitializer.Initialize(handler)
 	err = admission.Validate(handler)
 	return handler, f, err

plugin/pkg/admission/namespace/autoprovision/admission_test.go

@@ -38,7 +38,7 @@ import (
 func newHandlerForTest(c clientset.Interface) (admission.Interface, informers.SharedInformerFactory, error) {
 	f := informers.NewSharedInformerFactory(c, 5*time.Minute)
 	handler := NewProvision()
-	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil)
+	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil, nil)
 	pluginInitializer.Initialize(handler)
 	err := admission.Validate(handler)
 	return handler, f, err

plugin/pkg/admission/namespace/exists/admission_test.go

@@ -37,7 +37,7 @@ import (
 func newHandlerForTest(c clientset.Interface) (admission.Interface, informers.SharedInformerFactory, error) {
 	f := informers.NewSharedInformerFactory(c, 5*time.Minute)
 	handler := NewExists()
-	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil)
+	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil, nil)
 	pluginInitializer.Initialize(handler)
 	err := admission.Validate(handler)
 	return handler, f, err

plugin/pkg/admission/namespace/lifecycle/admission_test.go

@@ -48,7 +48,7 @@ func newHandlerForTestWithClock(c clientset.Interface, cacheClock clock.Clock) (
 	if err != nil {
 		return nil, f, err
 	}
-	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil)
+	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil, nil)
 	pluginInitializer.Initialize(handler)
 	err = admission.Validate(handler)
 	return handler, f, err

plugin/pkg/admission/podnodeselector/admission_test.go

@@ -191,7 +191,7 @@ func TestHandles(t *testing.T) {
 func newHandlerForTest(c clientset.Interface) (*podNodeSelector, informers.SharedInformerFactory, error) {
 	f := informers.NewSharedInformerFactory(c, 5*time.Minute)
 	handler := NewPodNodeSelector(nil)
-	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil)
+	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil, nil)
 	pluginInitializer.Initialize(handler)
 	err := admission.Validate(handler)
 	return handler, f, err

plugin/pkg/admission/podtolerationrestriction/admission_test.go

@@ -193,7 +193,7 @@ func newHandlerForTest(c clientset.Interface) (*podTolerationsPlugin, informers.
 		return nil, nil, err
 	}
 	handler := NewPodTolerationsPlugin(pluginConfig)
-	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil)
+	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil, nil)
 	pluginInitializer.Initialize(handler)
 	err = admission.Validate(handler)
 	return handler, f, err