don't prevent updates that only touch ownerrefs

pkg/build/admission/strategyrestrictions/admission.go

@@ -46,6 +46,14 @@ func (a *buildByStrategy) Admit(attr admission.Attributes) error {
 	if buildapi.IsResourceOrLegacy("builds", gr) && attr.GetSubresource() == "details" {
 		return nil
 	}
+
+	// if this is an update, see if we are only updating the ownerRef.  Garbage collection does this
+	// and we should allow it in general, since you had the power to update and the power to delete.
+	// The worst that happens is that you delete something, but you aren't controlling the privileged object itself
+	if attr.GetOldObject() != nil && oadmission.IsOnlyMutatingOwnerRefs(attr.GetObject(), attr.GetOldObject()) {
+		return nil
+	}
+
 	switch obj := attr.GetObject().(type) {
 	case *buildapi.Build:
 		return a.checkBuildAuthorization(obj, attr)

pkg/cmd/server/admission/helpers.go

@@ -0,0 +1,27 @@
+package admission
+
+import (
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime"
+	kapi "k8s.io/kubernetes/pkg/api"
+)
+
+func IsOnlyMutatingOwnerRefs(obj, old runtime.Object) 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())
+
+	return kapi.Semantic.DeepEqual(copied, old)
+}

pkg/cmd/server/admission/helpers_test.go

@@ -0,0 +1,109 @@
+package admission
+
+import (
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	kapi "k8s.io/kubernetes/pkg/api"
+)
+
+func newPod() *kapi.Pod {
+	return &kapi.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Annotations:     map[string]string{},
+			Name:            "foo",
+			OwnerReferences: []metav1.OwnerReference{},
+		},
+	}
+
+}
+
+func TestIsOnlyMutatingOwnerRefs(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: "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 := IsOnlyMutatingOwnerRefs(tc.obj(), tc.old())
+		if tc.expected != actual {
+			t.Errorf("%s: expected %v, got %v", tc.name, tc.expected, actual)
+		}
+	}
+}

pkg/security/admission/admission.go

@@ -6,7 +6,9 @@ import (
 	"sort"
 	"strings"
 
-	oscc "github.com/openshift/origin/pkg/security/scc"
+	"github.com/golang/glog"
+
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	admission "k8s.io/apiserver/pkg/admission"
 	kapi "k8s.io/kubernetes/pkg/api"
 	kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
@@ -16,10 +18,9 @@ import (
 	scc "k8s.io/kubernetes/pkg/securitycontextconstraints"
 	"k8s.io/kubernetes/pkg/serviceaccount"
 
+	oadmission "github.com/openshift/origin/pkg/cmd/server/admission"
 	allocator "github.com/openshift/origin/pkg/security"
-	"k8s.io/apimachinery/pkg/util/validation/field"
-
-	"github.com/golang/glog"
+	oscc "github.com/openshift/origin/pkg/security/scc"
 )
 
 func init() {
@@ -73,6 +74,13 @@ func (c *constraint) Admit(a admission.Attributes) error {
 		return nil
 	}
 
+	// if this is an update, see if we are only updating the ownerRef.  Garbage collection does this
+	// and we should allow it in general, since you had the power to update and the power to delete.
+	// The worst that happens is that you delete something, but you aren't controlling the privileged object itself
+	if a.GetOldObject() != nil && oadmission.IsOnlyMutatingOwnerRefs(a.GetObject(), a.GetOldObject()) {
+		return nil
+	}
+
 	// get all constraints that are usable by the user
 	glog.V(4).Infof("getting security context constraints for pod %s (generate: %s) in namespace %s with user info %v", pod.Name, pod.GenerateName, a.GetNamespace(), a.GetUserInfo())
 

pkg/security/admission/scc_exec.go

@@ -49,7 +49,7 @@ func (d *sccExecRestrictions) Admit(a admission.Attributes) (err error) {
 
 	// TODO, if we want to actually limit who can use which service account, then we'll need to add logic here to make sure that
 	// we're allowed to use the SA the pod is using.  Otherwise, user-A creates pod and user-B (who can't use the SA) can exec into it.
-	createAttributes := admission.NewAttributesRecord(pod, pod, kapi.Kind("Pod").WithVersion(""), a.GetNamespace(), a.GetName(), a.GetResource(), "", admission.Create, a.GetUserInfo())
+	createAttributes := admission.NewAttributesRecord(pod, nil, kapi.Kind("Pod").WithVersion(""), a.GetNamespace(), a.GetName(), a.GetResource(), "", admission.Create, a.GetUserInfo())
 	if err := d.constraintAdmission.Admit(createAttributes); err != nil {
 		return admission.NewForbidden(a, fmt.Errorf("%s operation is not allowed because the pod's security context exceeds your permissions: %v", a.GetSubresource(), err))
 	}