admission: complete plumbing of validation admission

plugin/pkg/admission/admit/admission.go

@@ -38,6 +38,11 @@ func (AlwaysAdmit) Admit(a admission.Attributes) (err error) {
 	return nil
 }
 
+// ValidatingAdmit makes an admission decision based on the request attributes.  It is NOT allowed to mutate
+func (AlwaysAdmit) ValidatingAdmit(a admission.Attributes) (err error) {
+	return nil
+}
+
 // Handles returns true if this admission controller can handle the given operation
 // where operation can be one of CREATE, UPDATE, DELETE, or CONNECT
 func (AlwaysAdmit) Handles(operation admission.Operation) bool {

staging/src/k8s.io/apiserver/pkg/admission/interfaces.go

@@ -67,6 +67,8 @@ type MutationInterface interface {
 
 // ValidationInterface is an abstract, pluggable interface for Admission Control decisions.
 type ValidationInterface interface {
+	Interface
+
 	// ValidatingAdmit makes an admission decision based on the request attributes.  It is NOT allowed to mutate
 	ValidatingAdmit(a Attributes) (err error)
 }

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go

@@ -34,7 +34,7 @@ import (
 	utiltrace "k8s.io/apiserver/pkg/util/trace"
 )
 
-func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.ObjectTyper, mutatingAdmission admission.Interface, validatingAdmission admission.ValidationInterface, includeName bool) http.HandlerFunc {
+func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.ObjectTyper, admit admission.Interface, includeName bool) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {
 		// For performance tracking purposes.
 		trace := utiltrace.New("Create " + req.URL.Path)
@@ -95,7 +95,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object
 
 		userInfo, _ := request.UserFrom(ctx)
 		admissionAttributes := admission.NewAttributesRecord(obj, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, userInfo)
-		if mutatingAdmission != nil && mutatingAdmission.Handles(admission.Create) {
+		if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && mutatingAdmission.Handles(admission.Create) {
 			err = mutatingAdmission.Admit(admissionAttributes)
 			if err != nil {
 				scope.err(err, w, req)
@@ -112,7 +112,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object
 				ctx,
 				name,
 				obj,
-				rest.AdmissionToValidateObjectFunc(validatingAdmission, admissionAttributes),
+				rest.AdmissionToValidateObjectFunc(admit, admissionAttributes),
 				includeUninitialized,
 			)
 		})
@@ -150,13 +150,13 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object
 }
 
 // CreateNamedResource returns a function that will handle a resource creation with name.
-func CreateNamedResource(r rest.NamedCreater, scope RequestScope, typer runtime.ObjectTyper, mutatingAdmission admission.Interface, validatingAdmission admission.ValidationInterface) http.HandlerFunc {
-	return createHandler(r, scope, typer, mutatingAdmission, validatingAdmission, true)
+func CreateNamedResource(r rest.NamedCreater, scope RequestScope, typer runtime.ObjectTyper, admission admission.Interface) http.HandlerFunc {
+	return createHandler(r, scope, typer, admission, true)
 }
 
 // CreateResource returns a function that will handle a resource creation.
-func CreateResource(r rest.Creater, scope RequestScope, typer runtime.ObjectTyper, mutatingAdmission admission.Interface, validatingAdmission admission.ValidationInterface) http.HandlerFunc {
-	return createHandler(&namedCreaterAdapter{r}, scope, typer, mutatingAdmission, validatingAdmission, false)
+func CreateResource(r rest.Creater, scope RequestScope, typer runtime.ObjectTyper, admission admission.Interface) http.HandlerFunc {
+	return createHandler(&namedCreaterAdapter{r}, scope, typer, admission, false)
 }
 
 type namedCreaterAdapter struct {

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go

@@ -35,7 +35,7 @@ import (
 
 // DeleteResource returns a function that will handle a resource deletion
 // TODO admission here becomes solely validating admission
-func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestScope, mutatingAdmission admission.Interface, validatingAdmission admission.ValidationInterface) http.HandlerFunc {
+func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestScope, admit admission.Interface) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {
 		// For performance tracking purposes.
 		trace := utiltrace.New("Delete " + req.URL.Path)
@@ -94,7 +94,7 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco
 		}
 
 		trace.Step("About to check admission control")
-		if mutatingAdmission != nil && mutatingAdmission.Handles(admission.Delete) {
+		if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && mutatingAdmission.Handles(admission.Delete) {
 			userInfo, _ := request.UserFrom(ctx)
 
 			err = mutatingAdmission.Admit(admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Delete, userInfo))
@@ -103,7 +103,8 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco
 				return
 			}
 		}
-		if validatingAdmission != nil && validatingAdmission.Handles(admission.Delete) {
+		// TODO: avoid calling Handles twice
+		if validatingAdmission, ok := admit.(admission.ValidationInterface); ok && validatingAdmission.Handles(admission.Delete) {
 			userInfo, _ := request.UserFrom(ctx)
 
 			err = validatingAdmission.ValidatingAdmit(admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Delete, userInfo))
@@ -167,7 +168,7 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco
 }
 
 // DeleteCollection returns a function that will handle a collection deletion
-func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestScope, mutatingAdmission admission.Interface, validatingAdmission admission.ValidationInterface) http.HandlerFunc {
+func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestScope, admit admission.Interface) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {
 		// TODO: we either want to remove timeout or document it (if we document, move timeout out of this function and declare it in api_installer)
 		timeout := parseTimeout(req.URL.Query().Get("timeout"))
@@ -181,7 +182,7 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco
 		ctx := scope.ContextFunc(req)
 		ctx = request.WithNamespace(ctx, namespace)
 
-		if mutatingAdmission != nil && mutatingAdmission.Handles(admission.Delete) {
+		if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && mutatingAdmission.Handles(admission.Delete) {
 			userInfo, _ := request.UserFrom(ctx)
 
 			err = mutatingAdmission.Admit(admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, "", scope.Resource, scope.Subresource, admission.Delete, userInfo))
@@ -190,7 +191,8 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco
 				return
 			}
 		}
-		if validatingAdmission != nil && validatingAdmission.Handles(admission.Delete) {
+		// TODO: avoid calling Handles twice
+		if validatingAdmission, ok := admit.(admission.ValidationInterface); ok && validatingAdmission.Handles(admission.Delete) {
 			userInfo, _ := request.UserFrom(ctx)
 
 			err = validatingAdmission.ValidatingAdmit(admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, "", scope.Resource, scope.Subresource, admission.Delete, userInfo))

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go

@@ -92,16 +92,25 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface
 			scope.Serializer.DecoderToVersion(s.Serializer, schema.GroupVersion{Group: gv.Group, Version: runtime.APIVersionInternal}),
 		)
 
-		updateAdmit := func(updatedObject runtime.Object, currentObject runtime.Object) error {
-			if admit != nil && admit.Handles(admission.Update) {
 		userInfo, _ := request.UserFrom(ctx)
-				return admit.Admit(admission.NewAttributesRecord(updatedObject, currentObject, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, userInfo))
+		staticAdmissionAttributes := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, userInfo)
+		updateMutation := func(updatedObject runtime.Object, currentObject runtime.Object) error {
+			if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && admit.Handles(admission.Update) {
+				return mutatingAdmission.Admit(admission.NewAttributesRecord(updatedObject, currentObject, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, userInfo))
 			}
-
 			return nil
 		}
 
-		result, err := patchResource(ctx, updateAdmit, timeout, versionedObj, r, name, patchType, patchJS,
+		result, err := patchResource(
+			ctx,
+			updateMutation,
+			rest.AdmissionToValidateObjectFunc(admit, staticAdmissionAttributes),
+			rest.AdmissionToValidateObjectUpdateFunc(admit, staticAdmissionAttributes),
+			timeout, versionedObj,
+			r,
+			name,
+			patchType,
+			patchJS,
 			scope.Namer, scope.Creater, scope.Defaulter, scope.UnsafeConvertor, scope.Kind, scope.Resource, codec)
 		if err != nil {
 			scope.err(err, w, req)
@@ -122,12 +131,14 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface
 	}
 }
 
-type updateAdmissionFunc func(updatedObject runtime.Object, currentObject runtime.Object) error
+type mutateObjectUpdateFunc func(obj, old runtime.Object) error
 
 // patchResource divides PatchResource for easier unit testing
 func patchResource(
 	ctx request.Context,
-	admit updateAdmissionFunc,
+	updateMutation mutateObjectUpdateFunc,
+	createValidation rest.ValidateObjectFunc,
+	updateValidation rest.ValidateObjectUpdateFunc,
 	timeout time.Duration,
 	versionedObj runtime.Object,
 	patcher rest.Patcher,
@@ -341,16 +352,15 @@ func patchResource(
 	// applyAdmission is called every time GuaranteedUpdate asks for the updated object,
 	// and is given the currently persisted object and the patched object as input.
 	applyAdmission := func(ctx request.Context, patchedObject runtime.Object, currentObject runtime.Object) (runtime.Object, error) {
-		return patchedObject, admit(patchedObject, currentObject)
+		return patchedObject, updateMutation(patchedObject, currentObject)
 	}
-
 	updatedObjectInfo := rest.DefaultUpdatedObjectInfo(nil, applyPatch, applyAdmission)
 
 	return finishRequest(timeout, func() (runtime.Object, error) {
-		updateObject, _, updateErr := patcher.Update(ctx, name, updatedObjectInfo)
+		updateObject, _, updateErr := patcher.Update(ctx, name, updatedObjectInfo, createValidation, updateValidation)
 		for i := 0; i < MaxRetryWhenPatchConflicts && (errors.IsConflict(updateErr)); i++ {
 			lastConflictErr = updateErr
-			updateObject, _, updateErr = patcher.Update(ctx, name, updatedObjectInfo)
+			updateObject, _, updateErr = patcher.Update(ctx, name, updatedObjectInfo, createValidation, updateValidation)
 		}
 		return updateObject, updateErr
 	})

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go

@@ -117,13 +117,22 @@ func ConnectResource(connecter rest.Connecter, scope RequestScope, admit admissi
 				ResourcePath: restPath,
 			}
 			userInfo, _ := request.UserFrom(ctx)
-
-			err = admit.Admit(admission.NewAttributesRecord(connectRequest, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Connect, userInfo))
+			// TODO: remove the mutating admission here as soon as we have ported all plugin that handle CONNECT
+			if mutatingAdmit, ok := admit.(admission.MutationInterface); ok {
+				err = mutatingAdmit.Admit(admission.NewAttributesRecord(connectRequest, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Connect, userInfo))
 				if err != nil {
 					scope.err(err, w, req)
 					return
 				}
 			}
+			if mutatingAdmit, ok := admit.(admission.ValidationInterface); ok {
+				err = mutatingAdmit.Validate(admission.NewAttributesRecord(connectRequest, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Connect, userInfo))
+				if err != nil {
+					scope.err(err, w, req)
+					return
+				}
+			}
+		}
 		requestInfo, _ := request.RequestInfoFrom(ctx)
 		metrics.RecordLongRunning(req, requestInfo, func() {
 			handler, err := connecter.Connect(ctx, name, opts, &responder{scope: scope, req: req, w: w})

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go

@@ -137,7 +137,7 @@ func (p *testPatcher) New() runtime.Object {
 	return &example.Pod{}
 }
 
-func (p *testPatcher) Update(ctx request.Context, name string, objInfo rest.UpdatedObjectInfo) (runtime.Object, bool, error) {
+func (p *testPatcher) Update(ctx request.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc) (runtime.Object, bool, error) {
 	currentPod := p.startingPod
 	if p.numUpdates > 0 {
 		currentPod = p.updatePod
@@ -202,7 +202,8 @@ type patchTestCase struct {
 	name string
 
 	// admission chain to use, nil is fine
-	admit updateAdmissionFunc
+	admissionMutation   mutateObjectUpdateFunc
+	admissionValidation rest.ValidateObjectUpdateFunc // TODO: add test for this
 
 	// startingPod is used as the starting point for the first Update
 	startingPod *example.Pod
@@ -224,9 +225,16 @@ func (tc *patchTestCase) Run(t *testing.T) {
 	name := tc.startingPod.Name
 
 	codec := codecs.LegacyCodec(examplev1.SchemeGroupVersion)
-	admit := tc.admit
-	if admit == nil {
-		admit = func(updatedObject runtime.Object, currentObject runtime.Object) error {
+
+	admissionMutation := tc.admissionMutation
+	if admissionMutation == nil {
+		admissionMutation = func(updatedObject runtime.Object, currentObject runtime.Object) error {
+			return nil
+		}
+	}
+	admissionValidation := tc.admissionValidation
+	if admissionValidation == nil {
+		admissionValidation = func(updatedObject runtime.Object, currentObject runtime.Object) error {
 			return nil
 		}
 	}
@@ -288,7 +296,18 @@ func (tc *patchTestCase) Run(t *testing.T) {
 
 		}
 
-		resultObj, err := patchResource(ctx, admit, 1*time.Second, versionedObj, testPatcher, name, patchType, patch, namer, creater, defaulter, convertor, kind, resource, codec)
+		resultObj, err := patchResource(
+			ctx,
+			admissionMutation,
+			rest.ValidateAllObjectFunc,
+			admissionValidation,
+			1*time.Second,
+			versionedObj,
+			testPatcher,
+			name,
+			patchType,
+			patch,
+			namer, creater, defaulter, convertor, kind, resource, 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)
@@ -511,7 +530,7 @@ func TestPatchWithAdmissionRejection(t *testing.T) {
 	tc := &patchTestCase{
 		name: "TestPatchWithAdmissionRejection",
 
-		admit: func(updatedObject runtime.Object, currentObject runtime.Object) error {
+		admissionMutation: func(updatedObject runtime.Object, currentObject runtime.Object) error {
 			return errors.New("admission failure")
 		},
 
@@ -550,7 +569,7 @@ func TestPatchWithVersionConflictThenAdmissionFailure(t *testing.T) {
 	tc := &patchTestCase{
 		name: "TestPatchWithVersionConflictThenAdmissionFailure",
 
-		admit: func(updatedObject runtime.Object, currentObject runtime.Object) error {
+		admissionMutation: func(updatedObject runtime.Object, currentObject runtime.Object) error {
 			if seen {
 				return errors.New("admission failure")
 			}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go

@@ -33,7 +33,7 @@ import (
 )
 
 // UpdateResource returns a function that will handle a resource update
-func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectTyper, mutatingAdmission admission.Interface, validatingAdmission admission.ValidationInterface) http.HandlerFunc {
+func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectTyper, admit admission.Interface) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {
 		// For performance tracking purposes.
 		trace := utiltrace.New("Update " + req.URL.Path)
@@ -89,7 +89,7 @@ func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectType
 		userInfo, _ := request.UserFrom(ctx)
 		staticAdmissionAttributes := admission.NewAttributesRecord(nil, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, userInfo)
 		var transformers []rest.TransformFunc
-		if mutatingAdmission != nil && mutatingAdmission.Handles(admission.Update) {
+		if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && mutatingAdmission.Handles(admission.Update) {
 			transformers = append(transformers, func(ctx request.Context, newObj, oldObj runtime.Object) (runtime.Object, error) {
 				return newObj, mutatingAdmission.Admit(admission.NewAttributesRecord(newObj, oldObj, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Update, userInfo))
 			})
@@ -102,8 +102,8 @@ func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectType
 				ctx,
 				name,
 				rest.DefaultUpdatedObjectInfo(obj, transformers...),
-				rest.AdmissionToValidateObjectFunc(validatingAdmission, staticAdmissionAttributes),
-				rest.AdmissionToValidateObjectUpdateFunc(validatingAdmission, staticAdmissionAttributes),
+				rest.AdmissionToValidateObjectFunc(admit, staticAdmissionAttributes),
+				rest.AdmissionToValidateObjectUpdateFunc(admit, staticAdmissionAttributes),
 			)
 			wasCreated = created
 			return obj, err

staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go

@@ -52,16 +52,6 @@ import (
 // object.
 type ObjectFunc func(obj runtime.Object) error
 
-// ValidateObjectFunc is a function to act on a given object. An error may be returned
-// if the hook cannot be completed. An ObjectFunc may NOT transform the provided
-// object.
-type ValidateObjectFunc func(obj runtime.Object) error
-
-// ValidateObjectUpdateFunc is a function to act on a given object and its predecessor.
-// An error may be returned if the hook cannot be completed. An UpdateObjectFunc
-// may NOT transform the provided object.
-type ValidateObjectUpdateFunc func(obj, old runtime.Object) error
-
 // GenericStore interface can be used for type assertions when we need to access the underlying strategies.
 type GenericStore interface {
 	GetCreateStrategy() rest.RESTCreateStrategy
@@ -315,7 +305,7 @@ func (e *Store) ListPredicate(ctx genericapirequest.Context, p storage.Selection
 }
 
 // Create inserts a new item according to the unique key from the object.
-func (e *Store) Create(ctx genericapirequest.Context, obj runtime.Object, createValidation ValidateObjectFunc, includeUninitialized bool) (runtime.Object, error) {
+func (e *Store) Create(ctx genericapirequest.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, includeUninitialized bool) (runtime.Object, error) {
 	if err := rest.BeforeCreate(e.CreateStrategy, ctx, obj); err != nil {
 		return nil, err
 	}
@@ -514,7 +504,7 @@ func (e *Store) deleteWithoutFinalizers(ctx genericapirequest.Context, name, key
 // Update performs an atomic update and set of the object. Returns the result of the update
 // or an error. If the registry allows create-on-update, the create flow will be executed.
 // A bool is returned along with the object and any errors, to indicate object creation.
-func (e *Store) Update(ctx genericapirequest.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation ValidateObjectFunc, updateValidation ValidateObjectUpdateFunc) (runtime.Object, bool, error) {
+func (e *Store) Update(ctx genericapirequest.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc) (runtime.Object, bool, error) {
 	key, err := e.KeyFunc(ctx, name)
 	if err != nil {
 		return nil, false, err

staging/src/k8s.io/apiserver/pkg/registry/rest/create.go

@@ -154,7 +154,11 @@ type NamespaceScopedStrategy interface {
 }
 
 // AdmissionToValidateObjectFunc converts validating admission to a rest validate object func
-func AdmissionToValidateObjectFunc(validatingAdmission admission.ValidationInterface, staticAttributes admission.Attributes) ValidateObjectFunc {
+func AdmissionToValidateObjectFunc(admit admission.Interface, staticAttributes admission.Attributes) ValidateObjectFunc {
+	validatingAdmission, ok := admit.(admission.ValidationInterface)
+	if !ok {
+		return func(obj runtime.Object) error { return nil }
+	}
 	return func(obj runtime.Object) error {
 		finalAttributes := admission.NewAttributesRecord(
 			obj,
@@ -170,6 +174,6 @@ func AdmissionToValidateObjectFunc(validatingAdmission admission.ValidationInter
 		if !validatingAdmission.Handles(finalAttributes.GetOperation()) {
 			return nil
 		}
-		return validatingAdmission.Validate(finalAttributes)
+		return validatingAdmission.ValidatingAdmit(finalAttributes)
 	}
 }

staging/src/k8s.io/apiserver/pkg/registry/rest/update.go

@@ -232,7 +232,11 @@ func (i *wrappedUpdatedObjectInfo) UpdatedObject(ctx genericapirequest.Context,
 }
 
 // AdmissionToValidateObjectUpdateFunc converts validating admission to a rest validate object update func
-func AdmissionToValidateObjectUpdateFunc(validatingAdmission admission.ValidationInterface, staticAttributes admission.Attributes) ValidateObjectUpdateFunc {
+func AdmissionToValidateObjectUpdateFunc(admit admission.Interface, staticAttributes admission.Attributes) ValidateObjectUpdateFunc {
+	validatingAdmission, ok := admit.(admission.ValidationInterface)
+	if !ok {
+		return func(obj, old runtime.Object) error { return nil }
+	}
 	return func(obj, old runtime.Object) error {
 		finalAttributes := admission.NewAttributesRecord(
 			obj,
@@ -248,6 +252,6 @@ func AdmissionToValidateObjectUpdateFunc(validatingAdmission admission.Validatio
 		if !validatingAdmission.Handles(finalAttributes.GetOperation()) {
 			return nil
 		}
-		return validatingAdmission.Validate(finalAttributes)
+		return validatingAdmission.ValidatingAdmit(finalAttributes)
 	}
 }