make patch call update admission chain after applying the patch

pkg/apiserver/resthandler.go

@@ -466,21 +466,9 @@ func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper
 			return
 		}
 
-		obj := r.New()
 		ctx := scope.ContextFunc(req)
 		ctx = api.WithNamespace(ctx, namespace)
 
-		// PATCH requires same permission as UPDATE
-		if admit.Handles(admission.Update) {
-			userInfo, _ := api.UserFrom(ctx)
-
-			err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind.GroupKind(), namespace, name, scope.Resource.GroupResource(), scope.Subresource, admission.Update, userInfo))
-			if err != nil {
-				errorJSON(err, scope.Codec, w)
-				return
-			}
-		}
-
 		versionedObj, err := converter.ConvertToVersion(r.New(), scope.Kind.GroupVersion().String())
 		if err != nil {
 			errorJSON(err, scope.Codec, w)
@@ -500,7 +488,16 @@ func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper
 			return
 		}
 
-		result, err := patchResource(ctx, timeout, versionedObj, r, name, patchType, patchJS, scope.Namer, scope.Codec)
+		updateAdmit := func(updatedObject runtime.Object) error {
+			if admit != nil && admit.Handles(admission.Update) {
+				userInfo, _ := api.UserFrom(ctx)
+				return admit.Admit(admission.NewAttributesRecord(updatedObject, scope.Kind.GroupKind(), namespace, name, scope.Resource.GroupResource(), scope.Subresource, admission.Update, userInfo))
+			}
+
+			return nil
+		}
+
+		result, err := patchResource(ctx, updateAdmit, timeout, versionedObj, r, name, patchType, patchJS, scope.Namer, scope.Codec)
 		if err != nil {
 			errorJSON(err, scope.Codec, w)
 			return
@@ -516,8 +513,10 @@ func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper
 
 }
 
+type updateAdmissionFunc func(updatedObject runtime.Object) error
+
 // patchResource divides PatchResource for easier unit testing
-func patchResource(ctx api.Context, timeout time.Duration, versionedObj runtime.Object, patcher rest.Patcher, name string, patchType api.PatchType, patchJS []byte, namer ScopeNamer, codec runtime.Codec) (runtime.Object, error) {
+func patchResource(ctx api.Context, admit updateAdmissionFunc, timeout time.Duration, versionedObj runtime.Object, patcher rest.Patcher, name string, patchType api.PatchType, patchJS []byte, namer ScopeNamer, codec runtime.Codec) (runtime.Object, error) {
 	namespace := api.NamespaceValue(ctx)
 
 	original, err := patcher.Get(ctx, name)
@@ -543,6 +542,10 @@ func patchResource(ctx api.Context, timeout time.Duration, versionedObj runtime.
 	}
 
 	return finishRequest(timeout, func() (runtime.Object, error) {
+		if err := admit(objToUpdate); err != nil {
+			return nil, err
+		}
+
 		// update should never create as previous get would fail
 		updateObject, _, updateErr := patcher.Update(ctx, objToUpdate)
 		for i := 0; i < MaxPatchConflicts && (errors.IsConflict(updateErr)); i++ {
@@ -597,6 +600,10 @@ func patchResource(ctx api.Context, timeout time.Duration, versionedObj runtime.
 				return nil, err
 			}
 
+			if err := admit(objToUpdate); err != nil {
+				return nil, err
+			}
+
 			updateObject, _, updateErr = patcher.Update(ctx, objToUpdate)
 		}
 

pkg/apiserver/resthandler_test.go

@@ -133,6 +133,9 @@ func (p *testNamer) GenerateListLink(req *restful.Request) (path, query string,
 type patchTestCase struct {
 	name string
 
+	// admission chain to use, nil is fine
+	admit updateAdmissionFunc
+
 	// startingPod is used for the first Get
 	startingPod *api.Pod
 	// changedPod is the "destination" pod for the patch.  The test will create a patch from the startingPod to the changedPod
@@ -153,6 +156,12 @@ func (tc *patchTestCase) Run(t *testing.T) {
 	name := tc.startingPod.Name
 
 	codec := latest.GroupOrDie(api.GroupName).Codec
+	admit := tc.admit
+	if admit == nil {
+		admit = func(updatedObject runtime.Object) error {
+			return nil
+		}
+	}
 
 	testPatcher := &testPatcher{}
 	testPatcher.startingPod = tc.startingPod
@@ -208,7 +217,7 @@ func (tc *patchTestCase) Run(t *testing.T) {
 
 		}
 
-		resultObj, err := patchResource(ctx, 1*time.Second, versionedObj, testPatcher, name, patchType, patch, namer, codec)
+		resultObj, err := patchResource(ctx, admit, 1*time.Second, versionedObj, testPatcher, name, patchType, patch, namer, codec)
 		if len(tc.expectedError) != 0 {
 			if err == nil || err.Error() != tc.expectedError {
 				t.Errorf("%s: expected error %v, but got %v", tc.name, tc.expectedError, err)
@@ -329,3 +338,86 @@ func TestPatchResourceWithConflict(t *testing.T) {
 
 	tc.Run(t)
 }
+
+func TestPatchWithAdmissionRejection(t *testing.T) {
+	namespace := "bar"
+	name := "foo"
+	fifteen := int64(15)
+	thirty := int64(30)
+
+	tc := &patchTestCase{
+		name: "TestPatchWithAdmissionRejection",
+
+		admit: func(updatedObject runtime.Object) error {
+			return errors.New("admission failure")
+		},
+
+		startingPod: &api.Pod{},
+		changedPod:  &api.Pod{},
+		updatePod:   &api.Pod{},
+
+		expectedError: "admission failure",
+	}
+
+	tc.startingPod.Name = name
+	tc.startingPod.Namespace = namespace
+	tc.startingPod.ResourceVersion = "1"
+	tc.startingPod.APIVersion = "v1"
+	tc.startingPod.Spec.ActiveDeadlineSeconds = &fifteen
+
+	tc.changedPod.Name = name
+	tc.changedPod.Namespace = namespace
+	tc.changedPod.ResourceVersion = "1"
+	tc.changedPod.APIVersion = "v1"
+	tc.changedPod.Spec.ActiveDeadlineSeconds = &thirty
+
+	tc.Run(t)
+}
+
+func TestPatchWithVersionConflictThenAdmissionFailure(t *testing.T) {
+	namespace := "bar"
+	name := "foo"
+	fifteen := int64(15)
+	thirty := int64(30)
+	seen := false
+
+	tc := &patchTestCase{
+		name: "TestPatchWithVersionConflictThenAdmissionFailure",
+
+		admit: func(updatedObject runtime.Object) error {
+			if seen {
+				return errors.New("admission failure")
+			}
+
+			seen = true
+			return nil
+		},
+
+		startingPod: &api.Pod{},
+		changedPod:  &api.Pod{},
+		updatePod:   &api.Pod{},
+
+		expectedError: "admission failure",
+	}
+
+	tc.startingPod.Name = name
+	tc.startingPod.Namespace = namespace
+	tc.startingPod.ResourceVersion = "1"
+	tc.startingPod.APIVersion = "v1"
+	tc.startingPod.Spec.ActiveDeadlineSeconds = &fifteen
+
+	tc.changedPod.Name = name
+	tc.changedPod.Namespace = namespace
+	tc.changedPod.ResourceVersion = "1"
+	tc.changedPod.APIVersion = "v1"
+	tc.changedPod.Spec.ActiveDeadlineSeconds = &thirty
+
+	tc.updatePod.Name = name
+	tc.updatePod.Namespace = namespace
+	tc.updatePod.ResourceVersion = "2"
+	tc.updatePod.APIVersion = "v1"
+	tc.updatePod.Spec.ActiveDeadlineSeconds = &fifteen
+	tc.updatePod.Spec.NodeName = "anywhere"
+
+	tc.Run(t)
+}