Add mutating admission webhook reinvocation

pkg/apis/admissionregistration/fuzzer/fuzzer.go

@@ -55,6 +55,8 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
 			obj.MatchPolicy = &m
 			s := admissionregistration.SideEffectClassUnknown
 			obj.SideEffects = &s
+			n := admissionregistration.NeverReinvocationPolicy
+			obj.ReinvocationPolicy = &n
 			if obj.TimeoutSeconds == nil {
 				i := int32(30)
 				obj.TimeoutSeconds = &i

pkg/apis/admissionregistration/types.go

@@ -383,8 +383,39 @@ type MutatingWebhook struct {
 	// does not understand, calls to the webhook will fail and be subject to the failure policy.
 	// +optional
 	AdmissionReviewVersions []string
+
+	// reinvocationPolicy indicates whether this webhook should be called multiple times as part of a single admission evaluation.
+	// Allowed values are "Never" and "IfNeeded".
+	//
+	// Never: the webhook will not be called more than once in a single admission evaluation.
+	//
+	// IfNeeded: the webhook will be called at least one additional time as part of the admission evaluation
+	// if the object being admitted is modified by other admission plugins after the initial webhook call.
+	// Webhooks that specify this option *must* be idempotent, and hence able to process objects they previously admitted.
+	// Note:
+	// * the number of additional invocations is not guaranteed to be exactly one.
+	// * if additional invocations result in further modifications to the object, webhooks are not guaranteed to be invoked again.
+	// * webhooks that use this option may be reordered to minimize the number of additional invocations.
+	// * to validate an object after all mutations are guaranteed complete, use a validating admission webhook instead.
+	//
+	// Defaults to "Never".
+	// +optional
+	ReinvocationPolicy *ReinvocationPolicyType
 }
 
+// ReinvocationPolicyType specifies what type of policy the admission hook uses.
+type ReinvocationPolicyType string
+
+var (
+	// NeverReinvocationPolicy indicates that the webhook must not be called more than once in a
+	// single admission evaluation.
+	NeverReinvocationPolicy ReinvocationPolicyType = "Never"
+	// IfNeededReinvocationPolicy indicates that the webhook may be called at least one
+	// additional time as part of the admission evaluation if the object being admitted is
+	// modified by other admission plugins after the initial webhook call.
+	IfNeededReinvocationPolicy ReinvocationPolicyType = "IfNeeded"
+)
+
 // RuleWithOperations is a tuple of Operations and Resources. It is recommended to make
 // sure that all the tuple expansions are valid.
 type RuleWithOperations struct {

pkg/apis/admissionregistration/v1beta1/defaults.go

@@ -77,6 +77,10 @@ func SetDefaults_MutatingWebhook(obj *admissionregistrationv1beta1.MutatingWebho
 		obj.TimeoutSeconds = new(int32)
 		*obj.TimeoutSeconds = 30
 	}
+	if obj.ReinvocationPolicy == nil {
+		never := admissionregistrationv1beta1.NeverReinvocationPolicy
+		obj.ReinvocationPolicy = &never
+	}
 
 	if len(obj.AdmissionReviewVersions) == 0 {
 		obj.AdmissionReviewVersions = []string{admissionregistrationv1beta1.SchemeGroupVersion.Version}

pkg/apis/admissionregistration/validation/validation.go

@@ -281,6 +281,9 @@ func validateMutatingWebhook(hook *admissionregistration.MutatingWebhook, fldPat
 	if hook.NamespaceSelector != nil {
 		allErrors = append(allErrors, metav1validation.ValidateLabelSelector(hook.NamespaceSelector, fldPath.Child("namespaceSelector"))...)
 	}
+	if hook.ReinvocationPolicy != nil && !supportedReinvocationPolicies.Has(string(*hook.ReinvocationPolicy)) {
+		allErrors = append(allErrors, field.NotSupported(fldPath.Child("reinvocationPolicy"), *hook.ReinvocationPolicy, supportedReinvocationPolicies.List()))
+	}
 
 	cc := hook.ClientConfig
 	switch {
@@ -319,6 +322,11 @@ var supportedOperations = sets.NewString(
 	string(admissionregistration.Connect),
 )
 
+var supportedReinvocationPolicies = sets.NewString(
+	string(admissionregistration.NeverReinvocationPolicy),
+	string(admissionregistration.IfNeededReinvocationPolicy),
+)
+
 func hasWildcardOperation(operations []admissionregistration.OperationType) bool {
 	for _, o := range operations {
 		if o == admissionregistration.OperationAll {

staging/src/k8s.io/api/admissionregistration/v1beta1/types.go

@@ -404,8 +404,39 @@ type MutatingWebhook struct {
 	// Default to `['v1beta1']`.
 	// +optional
 	AdmissionReviewVersions []string `json:"admissionReviewVersions,omitempty" protobuf:"bytes,8,rep,name=admissionReviewVersions"`
+
+	// reinvocationPolicy indicates whether this webhook should be called multiple times as part of a single admission evaluation.
+	// Allowed values are "Never" and "IfNeeded".
+	//
+	// Never: the webhook will not be called more than once in a single admission evaluation.
+	//
+	// IfNeeded: the webhook will be called at least one additional time as part of the admission evaluation
+	// if the object being admitted is modified by other admission plugins after the initial webhook call.
+	// Webhooks that specify this option *must* be idempotent, able to process objects they previously admitted.
+	// Note:
+	// * the number of additional invocations is not guaranteed to be exactly one.
+	// * if additional invocations result in further modifications to the object, webhooks are not guaranteed to be invoked again.
+	// * webhooks that use this option may be reordered to minimize the number of additional invocations.
+	// * to validate an object after all mutations are guaranteed complete, use a validating admission webhook instead.
+	//
+	// Defaults to "Never".
+	// +optional
+	ReinvocationPolicy *ReinvocationPolicyType `json:"reinvocationPolicy,omitempty" protobuf:"bytes,10,opt,name=reinvocationPolicy,casttype=ReinvocationPolicyType"`
 }
 
+// ReinvocationPolicyType specifies what type of policy the admission hook uses.
+type ReinvocationPolicyType string
+
+const (
+	// NeverReinvocationPolicy indicates that the webhook must not be called more than once in a
+	// single admission evaluation.
+	NeverReinvocationPolicy ReinvocationPolicyType = "Never"
+	// IfNeededReinvocationPolicy indicates that the webhook may be called at least one
+	// additional time as part of the admission evaluation if the object being admitted is
+	// modified by other admission plugins after the initial webhook call.
+	IfNeededReinvocationPolicy ReinvocationPolicyType = "IfNeeded"
+)
+
 // RuleWithOperations is a tuple of Operations and Resources. It is recommended to make
 // sure that all the tuple expansions are valid.
 type RuleWithOperations struct {

staging/src/k8s.io/apiserver/pkg/admission/BUILD

@@ -42,6 +42,7 @@ go_library(
         "handler.go",
         "interfaces.go",
         "plugins.go",
+        "reinvocation.go",
         "util.go",
     ],
     importmap = "k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission",

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

@@ -44,21 +44,24 @@ type attributesRecord struct {
 	// But ValidatingAdmissionWebhook add annotations concurrently.
 	annotations     map[string]string
 	annotationsLock sync.RWMutex
+
+	reinvocationContext ReinvocationContext
 }
 
 func NewAttributesRecord(object runtime.Object, oldObject runtime.Object, kind schema.GroupVersionKind, namespace, name string, resource schema.GroupVersionResource, subresource string, operation Operation, operationOptions runtime.Object, dryRun bool, userInfo user.Info) Attributes {
 	return &attributesRecord{
-		kind:        kind,
-		namespace:   namespace,
-		name:        name,
-		resource:    resource,
-		subresource: subresource,
-		operation:   operation,
-		options:     operationOptions,
-		dryRun:      dryRun,
-		object:      object,
-		oldObject:   oldObject,
-		userInfo:    userInfo,
+		kind:                kind,
+		namespace:           namespace,
+		name:                name,
+		resource:            resource,
+		subresource:         subresource,
+		operation:           operation,
+		options:             operationOptions,
+		dryRun:              dryRun,
+		object:              object,
+		oldObject:           oldObject,
+		userInfo:            userInfo,
+		reinvocationContext: &reinvocationContext{},
 	}
 }
 
@@ -140,6 +143,46 @@ func (record *attributesRecord) AddAnnotation(key, value string) error {
 	return nil
 }
 
+func (record *attributesRecord) GetReinvocationContext() ReinvocationContext {
+	return record.reinvocationContext
+}
+
+type reinvocationContext struct {
+	// isReinvoke is true when admission plugins are being reinvoked
+	isReinvoke bool
+	// reinvokeRequested is true when an admission plugin requested a re-invocation of the chain
+	reinvokeRequested bool
+	// values stores reinvoke context values per plugin.
+	values map[string]interface{}
+}
+
+func (rc *reinvocationContext) IsReinvoke() bool {
+	return rc.isReinvoke
+}
+
+func (rc *reinvocationContext) SetIsReinvoke() {
+	rc.isReinvoke = true
+}
+
+func (rc *reinvocationContext) ShouldReinvoke() bool {
+	return rc.reinvokeRequested
+}
+
+func (rc *reinvocationContext) SetShouldReinvoke() {
+	rc.reinvokeRequested = true
+}
+
+func (rc *reinvocationContext) SetValue(plugin string, v interface{}) {
+	if rc.values == nil {
+		rc.values = map[string]interface{}{}
+	}
+	rc.values[plugin] = v
+}
+
+func (rc *reinvocationContext) Value(plugin string) interface{} {
+	return rc.values[plugin]
+}
+
 func checkKeyFormat(key string) error {
 	parts := strings.Split(key, "/")
 	if len(parts) != 2 {

staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.go

@@ -86,8 +86,14 @@ func mergeMutatingWebhookConfigurations(configurations []*v1beta1.MutatingWebhoo
 	sort.SliceStable(configurations, MutatingWebhookConfigurationSorter(configurations).ByName)
 	accessors := []webhook.WebhookAccessor{}
 	for _, c := range configurations {
+		// webhook names are not validated for uniqueness, so we check for duplicates and
+		// add a int suffix to distinguish between them
+		names := map[string]int{}
 		for i := range c.Webhooks {
-			accessors = append(accessors, webhook.NewMutatingWebhookAccessor(&c.Webhooks[i]))
+			n := c.Webhooks[i].Name
+			uid := fmt.Sprintf("%s/%s/%d", c.Name, n, names[n])
+			names[n]++
+			accessors = append(accessors, webhook.NewMutatingWebhookAccessor(uid, &c.Webhooks[i]))
 		}
 	}
 	return accessors

staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go

@@ -84,8 +84,14 @@ func mergeValidatingWebhookConfigurations(configurations []*v1beta1.ValidatingWe
 	sort.SliceStable(configurations, ValidatingWebhookConfigurationSorter(configurations).ByName)
 	accessors := []webhook.WebhookAccessor{}
 	for _, c := range configurations {
+		// webhook names are not validated for uniqueness, so we check for duplicates and
+		// add a int suffix to distinguish between them
+		names := map[string]int{}
 		for i := range c.Webhooks {
-			accessors = append(accessors, webhook.NewValidatingWebhookAccessor(&c.Webhooks[i]))
+			n := c.Webhooks[i].Name
+			uid := fmt.Sprintf("%s/%s/%d", c.Name, n, names[n])
+			names[n]++
+			accessors = append(accessors, webhook.NewValidatingWebhookAccessor(uid, &c.Webhooks[i]))
 		}
 	}
 	return accessors

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

@@ -62,6 +62,9 @@ type Attributes interface {
 	// An error is returned if the format of key is invalid. When trying to overwrite annotation with a new value, an error is returned.
 	// Both ValidationInterface and MutationInterface are allowed to add Annotations.
 	AddAnnotation(key, value string) error
+
+	// GetReinvocationContext tracks the admission request information relevant to the re-invocation policy.
+	GetReinvocationContext() ReinvocationContext
 }
 
 // ObjectInterfaces is an interface used by AdmissionController to get object interfaces
@@ -91,6 +94,22 @@ type AnnotationsGetter interface {
 	GetAnnotations() map[string]string
 }
 
+// ReinvocationContext provides access to the admission related state required to implement the re-invocation policy.
+type ReinvocationContext interface {
+	// IsReinvoke returns true if the current admission check is a re-invocation.
+	IsReinvoke() bool
+	// SetIsReinvoke sets the current admission check as a re-invocation.
+	SetIsReinvoke()
+	// ShouldReinvoke returns true if any plugin has requested a re-invocation.
+	ShouldReinvoke() bool
+	// SetShouldReinvoke signals that a re-invocation is desired.
+	SetShouldReinvoke()
+	// AddValue set a value for a plugin name, possibly overriding a previous value.
+	SetValue(plugin string, v interface{})
+	// Value reads a value for a webhook.
+	Value(plugin string) interface{}
+}
+
 // Interface is an abstract, pluggable interface for Admission Control decisions.
 type Interface interface {
 	// Handles returns true if this admission controller can handle the given operation

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/accessors.go

@@ -23,7 +23,12 @@ import (
 
 // WebhookAccessor provides a common interface to both mutating and validating webhook types.
 type WebhookAccessor interface {
-	// GetName gets the webhook Name field.
+	// GetUID gets a string that uniquely identifies the webhook.
+	GetUID() string
+
+	// GetName gets the webhook Name field. Note that the name is scoped to the webhook
+	// configuration and does not provide a globally unique identity, if a unique identity is
+	// needed, use GetUID.
 	GetName() string
 	// GetClientConfig gets the webhook ClientConfig field.
 	GetClientConfig() v1beta1.WebhookClientConfig
@@ -49,14 +54,18 @@ type WebhookAccessor interface {
 }
 
 // NewMutatingWebhookAccessor creates an accessor for a MutatingWebhook.
-func NewMutatingWebhookAccessor(h *v1beta1.MutatingWebhook) WebhookAccessor {
-	return mutatingWebhookAccessor{h}
+func NewMutatingWebhookAccessor(uid string, h *v1beta1.MutatingWebhook) WebhookAccessor {
+	return mutatingWebhookAccessor{uid: uid, MutatingWebhook: h}
 }
 
 type mutatingWebhookAccessor struct {
 	*v1beta1.MutatingWebhook
+	uid string
 }
 
+func (m mutatingWebhookAccessor) GetUID() string {
+	return m.Name
+}
 func (m mutatingWebhookAccessor) GetName() string {
 	return m.Name
 }
@@ -94,14 +103,18 @@ func (m mutatingWebhookAccessor) GetValidatingWebhook() (*v1beta1.ValidatingWebh
 }
 
 // NewValidatingWebhookAccessor creates an accessor for a ValidatingWebhook.
-func NewValidatingWebhookAccessor(h *v1beta1.ValidatingWebhook) WebhookAccessor {
-	return validatingWebhookAccessor{h}
+func NewValidatingWebhookAccessor(uid string, h *v1beta1.ValidatingWebhook) WebhookAccessor {
+	return validatingWebhookAccessor{uid: uid, ValidatingWebhook: h}
 }
 
 type validatingWebhookAccessor struct {
 	*v1beta1.ValidatingWebhook
+	uid string
 }
 
+func (v validatingWebhookAccessor) GetUID() string {
+	return v.uid
+}
 func (v validatingWebhookAccessor) GetName() string {
 	return v.Name
 }

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package generic
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
@@ -277,9 +278,9 @@ func TestShouldCallHook(t *testing.T) {
 		},
 	}
 
-	for _, testcase := range testcases {
+	for i, testcase := range testcases {
 		t.Run(testcase.name, func(t *testing.T) {
-			invocation, err := a.shouldCallHook(webhook.NewValidatingWebhookAccessor(testcase.webhook), testcase.attrs, interfaces)
+			invocation, err := a.shouldCallHook(webhook.NewValidatingWebhookAccessor(fmt.Sprintf("webhook-%d", i), testcase.webhook), testcase.attrs, interfaces)
 			if err != nil {
 				if len(testcase.expectErr) == 0 {
 					t.Fatal(err)

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD

@@ -6,6 +6,7 @@ go_library(
         "dispatcher.go",
         "doc.go",
         "plugin.go",
+        "reinvocationcontext.go",
     ],
     importmap = "k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating",
     importpath = "k8s.io/apiserver/pkg/admission/plugin/webhook/mutating",
@@ -13,11 +14,13 @@ go_library(
     deps = [
         "//staging/src/k8s.io/api/admission/v1beta1:go_default_library",
         "//staging/src/k8s.io/api/admissionregistration/v1beta1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission/configuration:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission/metrics:go_default_library",

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go

@@ -24,6 +24,7 @@ import (
 	"time"
 
 	jsonpatch "github.com/evanphx/json-patch"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/klog"
 
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
@@ -56,12 +57,32 @@ func newMutatingDispatcher(p *Plugin) func(cm *webhookutil.ClientManager) generi
 var _ generic.Dispatcher = &mutatingDispatcher{}
 
 func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr admission.Attributes, o admission.ObjectInterfaces, relevantHooks []*generic.WebhookInvocation) error {
+	reinvokeCtx := attr.GetReinvocationContext()
+	var webhookReinvokeCtx *webhookReinvokeContext
+	if v := reinvokeCtx.Value(PluginName); v != nil {
+		webhookReinvokeCtx = v.(*webhookReinvokeContext)
+	} else {
+		webhookReinvokeCtx = &webhookReinvokeContext{}
+		reinvokeCtx.SetValue(PluginName, webhookReinvokeCtx)
+	}
+
+	if reinvokeCtx.IsReinvoke() && webhookReinvokeCtx.IsOutputChangedSinceLastWebhookInvocation(attr.GetObject()) {
+		// If the object has changed, we know the in-tree plugin re-invocations have mutated the object,
+		// and we need to reinvoke all eligible webhooks.
+		webhookReinvokeCtx.RequireReinvokingPreviouslyInvokedPlugins()
+	}
+	defer func() {
+		webhookReinvokeCtx.SetLastWebhookInvocationOutput(attr.GetObject())
+	}()
 	var versionedAttr *generic.VersionedAttributes
 	for _, invocation := range relevantHooks {
 		hook, ok := invocation.Webhook.GetMutatingWebhook()
 		if !ok {
 			return fmt.Errorf("mutating webhook dispatch requires v1beta1.MutatingWebhook, but got %T", hook)
 		}
+		if reinvokeCtx.IsReinvoke() && !webhookReinvokeCtx.ShouldReinvokeWebhook(invocation.Webhook.GetUID()) {
+			continue
+		}
 		if versionedAttr == nil {
 			// First webhook, create versioned attributes
 			var err error
@@ -76,8 +97,17 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr admission.Attrib
 		}
 
 		t := time.Now()
-		err := a.callAttrMutatingHook(ctx, hook, invocation, versionedAttr, o)
+
+		changed, err := a.callAttrMutatingHook(ctx, hook, invocation, versionedAttr, o)
 		admissionmetrics.Metrics.ObserveWebhook(time.Since(t), err != nil, versionedAttr.Attributes, "admit", hook.Name)
+		if changed {
+			// Patch had changed the object. Prepare to reinvoke all previous webhooks that are eligible for re-invocation.
+			webhookReinvokeCtx.RequireReinvokingPreviouslyInvokedPlugins()
+			reinvokeCtx.SetShouldReinvoke()
+		}
+		if hook.ReinvocationPolicy != nil && *hook.ReinvocationPolicy == v1beta1.IfNeededReinvocationPolicy {
+			webhookReinvokeCtx.AddReinvocableWebhookToPreviouslyInvoked(invocation.Webhook.GetUID())
+		}
 		if err == nil {
 			continue
 		}
@@ -99,32 +129,33 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr admission.Attrib
 	if versionedAttr != nil && versionedAttr.VersionedObject != nil && versionedAttr.Dirty {
 		return o.GetObjectConvertor().Convert(versionedAttr.VersionedObject, versionedAttr.Attributes.GetObject(), nil)
 	}
+
 	return nil
 }
 
 // note that callAttrMutatingHook updates attr
 
-func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta1.MutatingWebhook, invocation *generic.WebhookInvocation, attr *generic.VersionedAttributes, o admission.ObjectInterfaces) error {
+func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta1.MutatingWebhook, invocation *generic.WebhookInvocation, attr *generic.VersionedAttributes, o admission.ObjectInterfaces) (bool, error) {
 	if attr.Attributes.IsDryRun() {
 		if h.SideEffects == nil {
-			return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook SideEffects is nil")}
+			return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook SideEffects is nil")}
 		}
 		if !(*h.SideEffects == v1beta1.SideEffectClassNone || *h.SideEffects == v1beta1.SideEffectClassNoneOnDryRun) {
-			return webhookerrors.NewDryRunUnsupportedErr(h.Name)
+			return false, webhookerrors.NewDryRunUnsupportedErr(h.Name)
 		}
 	}
 
 	// Currently dispatcher only supports `v1beta1` AdmissionReview
 	// TODO: Make the dispatcher capable of sending multiple AdmissionReview versions
 	if !util.HasAdmissionReviewVersion(v1beta1.SchemeGroupVersion.Version, invocation.Webhook) {
-		return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("webhook does not accept v1beta1 AdmissionReview")}
+		return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("webhook does not accept v1beta1 AdmissionReview")}
 	}
 
 	// Make the webhook request
 	request := request.CreateAdmissionReview(attr, invocation)
 	client, err := a.cm.HookClient(util.HookClientConfigForWebhook(invocation.Webhook))
 	if err != nil {
-		return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err}
+		return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err}
 	}
 	response := &admissionv1beta1.AdmissionReview{}
 	r := client.Post().Context(ctx).Body(&request)
@@ -132,11 +163,11 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta
 		r = r.Timeout(time.Duration(*h.TimeoutSeconds) * time.Second)
 	}
 	if err := r.Do().Into(response); err != nil {
-		return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err}
+		return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err}
 	}
 
 	if response.Response == nil {
-		return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook response was absent")}
+		return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook response was absent")}
 	}
 
 	for k, v := range response.Response.AuditAnnotations {
@@ -147,34 +178,34 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta
 	}
 
 	if !response.Response.Allowed {
-		return webhookerrors.ToStatusErr(h.Name, response.Response.Result)
+		return false, webhookerrors.ToStatusErr(h.Name, response.Response.Result)
 	}
 
 	patchJS := response.Response.Patch
 	if len(patchJS) == 0 {
-		return nil
+		return false, nil
 	}
 	patchObj, err := jsonpatch.DecodePatch(patchJS)
 	if err != nil {
-		return apierrors.NewInternalError(err)
+		return false, apierrors.NewInternalError(err)
 	}
 	if len(patchObj) == 0 {
-		return nil
+		return false, nil
 	}
 
 	// if a non-empty patch was provided, and we have no object we can apply it to (e.g. a DELETE admission operation), error
 	if attr.VersionedObject == nil {
-		return apierrors.NewInternalError(fmt.Errorf("admission webhook %q attempted to modify the object, which is not supported for this operation", h.Name))
+		return false, apierrors.NewInternalError(fmt.Errorf("admission webhook %q attempted to modify the object, which is not supported for this operation", h.Name))
 	}
 
 	jsonSerializer := json.NewSerializer(json.DefaultMetaFactory, o.GetObjectCreater(), o.GetObjectTyper(), false)
 	objJS, err := runtime.Encode(jsonSerializer, attr.VersionedObject)
 	if err != nil {
-		return apierrors.NewInternalError(err)
+		return false, apierrors.NewInternalError(err)
 	}
 	patchedJS, err := patchObj.Apply(objJS)
 	if err != nil {
-		return apierrors.NewInternalError(err)
+		return false, apierrors.NewInternalError(err)
 	}
 
 	var newVersionedObject runtime.Object
@@ -185,16 +216,20 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta
 	} else {
 		newVersionedObject, err = o.GetObjectCreater().New(attr.VersionedKind)
 		if err != nil {
-			return apierrors.NewInternalError(err)
+			return false, apierrors.NewInternalError(err)
 		}
 	}
+
 	// TODO: if we have multiple mutating webhooks, we can remember the json
 	// instead of encoding and decoding for each one.
 	if newVersionedObject, _, err = jsonSerializer.Decode(patchedJS, nil, newVersionedObject); err != nil {
-		return apierrors.NewInternalError(err)
+		return false, apierrors.NewInternalError(err)
 	}
+
+	changed := !apiequality.Semantic.DeepEqual(attr.VersionedObject, newVersionedObject)
+
 	attr.Dirty = true
 	attr.VersionedObject = newVersionedObject
 	o.GetObjectDefaulter().Default(attr.VersionedObject)
-	return nil
+	return changed, nil
 }

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go

@@ -49,67 +49,77 @@ func TestAdmit(t *testing.T) {
 		webhooktesting.ConvertToMutatingTestCases(webhooktesting.NewNonMutatingTestCases(serverURL))...)
 
 	for _, tt := range testCases {
-		wh, err := NewMutatingWebhook(nil)
-		if err != nil {
-			t.Errorf("%s: failed to create mutating webhook: %v", tt.Name, err)
-			continue
-		}
-
-		ns := "webhook-test"
-		client, informer := webhooktesting.NewFakeMutatingDataSource(ns, tt.Webhooks, stopCh)
-
-		wh.SetAuthenticationInfoResolverWrapper(webhooktesting.Wrapper(webhooktesting.NewAuthenticationInfoResolver(new(int32))))
-		wh.SetServiceResolver(webhooktesting.NewServiceResolver(*serverURL))
-		wh.SetExternalKubeClientSet(client)
-		wh.SetExternalKubeInformerFactory(informer)
-
-		informer.Start(stopCh)
-		informer.WaitForCacheSync(stopCh)
-
-		if err = wh.ValidateInitialization(); err != nil {
-			t.Errorf("%s: failed to validate initialization: %v", tt.Name, err)
-			continue
-		}
-
-		var attr admission.Attributes
-		if tt.IsCRD {
-			attr = webhooktesting.NewAttributeUnstructured(ns, tt.AdditionalLabels, tt.IsDryRun)
-		} else {
-			attr = webhooktesting.NewAttribute(ns, tt.AdditionalLabels, tt.IsDryRun)
-		}
-
-		err = wh.Admit(attr, objectInterfaces)
-		if tt.ExpectAllow != (err == nil) {
-			t.Errorf("%s: expected allowed=%v, but got err=%v", tt.Name, tt.ExpectAllow, err)
-		}
-		if tt.ExpectLabels != nil {
-			if !reflect.DeepEqual(tt.ExpectLabels, attr.GetObject().(metav1.Object).GetLabels()) {
-				t.Errorf("%s: expected labels '%v', but got '%v'", tt.Name, tt.ExpectLabels, attr.GetObject().(metav1.Object).GetLabels())
+		t.Run(tt.Name, func(t *testing.T) {
+			wh, err := NewMutatingWebhook(nil)
+			if err != nil {
+				t.Errorf("failed to create mutating webhook: %v", err)
+				return
 			}
-		}
-		// ErrWebhookRejected is not an error for our purposes
-		if tt.ErrorContains != "" {
-			if err == nil || !strings.Contains(err.Error(), tt.ErrorContains) {
-				t.Errorf("%s: expected an error saying %q, but got: %v", tt.Name, tt.ErrorContains, err)
+
+			ns := "webhook-test"
+			client, informer := webhooktesting.NewFakeMutatingDataSource(ns, tt.Webhooks, stopCh)
+
+			wh.SetAuthenticationInfoResolverWrapper(webhooktesting.Wrapper(webhooktesting.NewAuthenticationInfoResolver(new(int32))))
+			wh.SetServiceResolver(webhooktesting.NewServiceResolver(*serverURL))
+			wh.SetExternalKubeClientSet(client)
+			wh.SetExternalKubeInformerFactory(informer)
+
+			informer.Start(stopCh)
+			informer.WaitForCacheSync(stopCh)
+
+			if err = wh.ValidateInitialization(); err != nil {
+				t.Errorf("failed to validate initialization: %v", err)
+				return
 			}
-		}
-		if statusErr, isStatusErr := err.(*errors.StatusError); err != nil && !isStatusErr {
-			t.Errorf("%s: expected a StatusError, got %T", tt.Name, err)
-		} else if isStatusErr {
-			if statusErr.ErrStatus.Code != tt.ExpectStatusCode {
-				t.Errorf("%s: expected status code %d, got %d", tt.Name, tt.ExpectStatusCode, statusErr.ErrStatus.Code)
+
+			var attr admission.Attributes
+			if tt.IsCRD {
+				attr = webhooktesting.NewAttributeUnstructured(ns, tt.AdditionalLabels, tt.IsDryRun)
+			} else {
+				attr = webhooktesting.NewAttribute(ns, tt.AdditionalLabels, tt.IsDryRun)
 			}
-		}
-		fakeAttr, ok := attr.(*webhooktesting.FakeAttributes)
-		if !ok {
-			t.Errorf("Unexpected error, failed to convert attr to webhooktesting.FakeAttributes")
-			continue
-		}
-		if len(tt.ExpectAnnotations) == 0 {
-			assert.Empty(t, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.")
-		} else {
-			assert.Equal(t, tt.ExpectAnnotations, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.")
-		}
+
+			err = wh.Admit(attr, objectInterfaces)
+			if tt.ExpectAllow != (err == nil) {
+				t.Errorf("expected allowed=%v, but got err=%v", tt.ExpectAllow, err)
+			}
+			if tt.ExpectLabels != nil {
+				if !reflect.DeepEqual(tt.ExpectLabels, attr.GetObject().(metav1.Object).GetLabels()) {
+					t.Errorf("expected labels '%v', but got '%v'", tt.ExpectLabels, attr.GetObject().(metav1.Object).GetLabels())
+				}
+			}
+			// ErrWebhookRejected is not an error for our purposes
+			if tt.ErrorContains != "" {
+				if err == nil || !strings.Contains(err.Error(), tt.ErrorContains) {
+					t.Errorf("expected an error saying %q, but got: %v", tt.ErrorContains, err)
+				}
+			}
+			if statusErr, isStatusErr := err.(*errors.StatusError); err != nil && !isStatusErr {
+				t.Errorf("expected a StatusError, got %T", err)
+			} else if isStatusErr {
+				if statusErr.ErrStatus.Code != tt.ExpectStatusCode {
+					t.Errorf("expected status code %d, got %d", tt.ExpectStatusCode, statusErr.ErrStatus.Code)
+				}
+			}
+			fakeAttr, ok := attr.(*webhooktesting.FakeAttributes)
+			if !ok {
+				t.Errorf("Unexpected error, failed to convert attr to webhooktesting.FakeAttributes")
+				return
+			}
+			if len(tt.ExpectAnnotations) == 0 {
+				assert.Empty(t, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.")
+			} else {
+				assert.Equal(t, tt.ExpectAnnotations, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.")
+			}
+			reinvocationCtx := fakeAttr.Attributes.GetReinvocationContext()
+			reinvocationCtx.SetIsReinvoke()
+			for webhook, expectReinvoke := range tt.ExpectReinvokeWebhooks {
+				shouldReinvoke := reinvocationCtx.Value(PluginName).(*webhookReinvokeContext).ShouldReinvokeWebhook(webhook)
+				if expectReinvoke != shouldReinvoke {
+					t.Errorf("expected reinvocationContext.ShouldReinvokeWebhook(%s)=%t, but got %t", webhook, expectReinvoke, shouldReinvoke)
+				}
+			}
+		})
 	}
 }
 

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/reinvocationcontext.go

@@ -0,0 +1,68 @@
+/*
+Copyright 2019 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 mutating
+
+import (
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
+)
+
+type webhookReinvokeContext struct {
+	// lastWebhookOutput holds the result of the last webhook admission plugin call
+	lastWebhookOutput runtime.Object
+	// previouslyInvokedReinvocableWebhooks holds the set of webhooks that have been invoked and
+	// should be reinvoked if a later mutation occurs
+	previouslyInvokedReinvocableWebhooks sets.String
+	// reinvokeWebhooks holds the set of webhooks that should be reinvoked
+	reinvokeWebhooks sets.String
+}
+
+func (rc *webhookReinvokeContext) ShouldReinvokeWebhook(webhook string) bool {
+	return rc.reinvokeWebhooks.Has(webhook)
+}
+
+func (rc *webhookReinvokeContext) IsOutputChangedSinceLastWebhookInvocation(object runtime.Object) bool {
+	return !apiequality.Semantic.DeepEqual(rc.lastWebhookOutput, object)
+}
+
+func (rc *webhookReinvokeContext) SetLastWebhookInvocationOutput(object runtime.Object) {
+	if object == nil {
+		rc.lastWebhookOutput = nil
+		return
+	}
+	rc.lastWebhookOutput = object.DeepCopyObject()
+}
+
+func (rc *webhookReinvokeContext) AddReinvocableWebhookToPreviouslyInvoked(webhook string) {
+	if rc.previouslyInvokedReinvocableWebhooks == nil {
+		rc.previouslyInvokedReinvocableWebhooks = sets.NewString()
+	}
+	rc.previouslyInvokedReinvocableWebhooks.Insert(webhook)
+}
+
+func (rc *webhookReinvokeContext) RequireReinvokingPreviouslyInvokedPlugins() {
+	if len(rc.previouslyInvokedReinvocableWebhooks) > 0 {
+		if rc.reinvokeWebhooks == nil {
+			rc.reinvokeWebhooks = sets.NewString()
+		}
+		for s := range rc.previouslyInvokedReinvocableWebhooks {
+			rc.reinvokeWebhooks.Insert(s)
+		}
+		rc.previouslyInvokedReinvocableWebhooks = sets.NewString()
+	}
+}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace/matcher_test.go

@@ -120,7 +120,7 @@ func TestNotExemptClusterScopedResource(t *testing.T) {
 	}
 	attr := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, "", "mock-name", schema.GroupVersionResource{Version: "v1", Resource: "nodes"}, "", admission.Create, &metav1.CreateOptions{}, false, nil)
 	matcher := Matcher{}
-	matches, err := matcher.MatchNamespaceSelector(webhook.NewValidatingWebhookAccessor(hook), attr)
+	matches, err := matcher.MatchNamespaceSelector(webhook.NewValidatingWebhookAccessor("mock-hook", hook), attr)
 	if err != nil {
 		t.Fatal(err)
 	}

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go

@@ -49,6 +49,9 @@ var sideEffectsNone = registrationv1beta1.SideEffectClassNone
 var sideEffectsSome = registrationv1beta1.SideEffectClassSome
 var sideEffectsNoneOnDryRun = registrationv1beta1.SideEffectClassNoneOnDryRun
 
+var reinvokeNever = registrationv1beta1.NeverReinvocationPolicy
+var reinvokeIfNeeded = registrationv1beta1.IfNeededReinvocationPolicy
+
 // NewFakeValidatingDataSource returns a mock client and informer returning the given webhooks.
 func NewFakeValidatingDataSource(name string, webhooks []registrationv1beta1.ValidatingWebhook, stopCh <-chan struct{}) (clientset kubernetes.Interface, factory informers.SharedInformerFactory) {
 	var objs = []runtime.Object{
@@ -199,39 +202,41 @@ func (c urlConfigGenerator) ccfgURL(urlPath string) registrationv1beta1.WebhookC
 
 // ValidatingTest is a validating webhook test case.
 type ValidatingTest struct {
-	Name              string
-	Webhooks          []registrationv1beta1.ValidatingWebhook
-	Path              string
-	IsCRD             bool
-	IsDryRun          bool
-	AdditionalLabels  map[string]string
-	ExpectLabels      map[string]string
-	ExpectAllow       bool
-	ErrorContains     string
-	ExpectAnnotations map[string]string
-	ExpectStatusCode  int32
+	Name                   string
+	Webhooks               []registrationv1beta1.ValidatingWebhook
+	Path                   string
+	IsCRD                  bool
+	IsDryRun               bool
+	AdditionalLabels       map[string]string
+	ExpectLabels           map[string]string
+	ExpectAllow            bool
+	ErrorContains          string
+	ExpectAnnotations      map[string]string
+	ExpectStatusCode       int32
+	ExpectReinvokeWebhooks map[string]bool
 }
 
 // MutatingTest is a mutating webhook test case.
 type MutatingTest struct {
-	Name              string
-	Webhooks          []registrationv1beta1.MutatingWebhook
-	Path              string
-	IsCRD             bool
-	IsDryRun          bool
-	AdditionalLabels  map[string]string
-	ExpectLabels      map[string]string
-	ExpectAllow       bool
-	ErrorContains     string
-	ExpectAnnotations map[string]string
-	ExpectStatusCode  int32
+	Name                   string
+	Webhooks               []registrationv1beta1.MutatingWebhook
+	Path                   string
+	IsCRD                  bool
+	IsDryRun               bool
+	AdditionalLabels       map[string]string
+	ExpectLabels           map[string]string
+	ExpectAllow            bool
+	ErrorContains          string
+	ExpectAnnotations      map[string]string
+	ExpectStatusCode       int32
+	ExpectReinvokeWebhooks map[string]bool
 }
 
 // ConvertToMutatingTestCases converts a validating test case to a mutating one for test purposes.
 func ConvertToMutatingTestCases(tests []ValidatingTest) []MutatingTest {
 	r := make([]MutatingTest, len(tests))
 	for i, t := range tests {
-		r[i] = MutatingTest{t.Name, ConvertToMutatingWebhooks(t.Webhooks), t.Path, t.IsCRD, t.IsDryRun, t.AdditionalLabels, t.ExpectLabels, t.ExpectAllow, t.ErrorContains, t.ExpectAnnotations, t.ExpectStatusCode}
+		r[i] = MutatingTest{t.Name, ConvertToMutatingWebhooks(t.Webhooks), t.Path, t.IsCRD, t.IsDryRun, t.AdditionalLabels, t.ExpectLabels, t.ExpectAllow, t.ErrorContains, t.ExpectAnnotations, t.ExpectStatusCode, t.ExpectReinvokeWebhooks}
 	}
 	return r
 }
@@ -240,7 +245,7 @@ func ConvertToMutatingTestCases(tests []ValidatingTest) []MutatingTest {
 func ConvertToMutatingWebhooks(webhooks []registrationv1beta1.ValidatingWebhook) []registrationv1beta1.MutatingWebhook {
 	mutating := make([]registrationv1beta1.MutatingWebhook, len(webhooks))
 	for i, h := range webhooks {
-		mutating[i] = registrationv1beta1.MutatingWebhook{h.Name, h.ClientConfig, h.Rules, h.FailurePolicy, h.MatchPolicy, h.NamespaceSelector, h.SideEffects, h.TimeoutSeconds, h.AdmissionReviewVersions}
+		mutating[i] = registrationv1beta1.MutatingWebhook{h.Name, h.ClientConfig, h.Rules, h.FailurePolicy, h.MatchPolicy, h.NamespaceSelector, h.SideEffects, h.TimeoutSeconds, h.AdmissionReviewVersions, nil}
 	}
 	return mutating
 }
@@ -639,6 +644,63 @@ func NewMutatingTestCases(url *url.URL) []MutatingTest {
 		},
 		// No need to test everything with the url case, since only the
 		// connection is different.
+		{
+			Name: "match & reinvoke if needed policy",
+			Webhooks: []registrationv1beta1.MutatingWebhook{{
+				Name:                    "addLabel",
+				ClientConfig:            ccfgSVC("addLabel"),
+				Rules:                   matchEverythingRules,
+				NamespaceSelector:       &metav1.LabelSelector{},
+				AdmissionReviewVersions: []string{"v1beta1"},
+				ReinvocationPolicy:      &reinvokeIfNeeded,
+			}, {
+				Name:                    "removeLabel",
+				ClientConfig:            ccfgSVC("removeLabel"),
+				Rules:                   matchEverythingRules,
+				NamespaceSelector:       &metav1.LabelSelector{},
+				AdmissionReviewVersions: []string{"v1beta1"},
+				ReinvocationPolicy:      &reinvokeIfNeeded,
+			}},
+			AdditionalLabels:       map[string]string{"remove": "me"},
+			ExpectAllow:            true,
+			ExpectReinvokeWebhooks: map[string]bool{"addLabel": true},
+		},
+		{
+			Name: "match & never reinvoke policy",
+			Webhooks: []registrationv1beta1.MutatingWebhook{{
+				Name:                    "addLabel",
+				ClientConfig:            ccfgSVC("addLabel"),
+				Rules:                   matchEverythingRules,
+				NamespaceSelector:       &metav1.LabelSelector{},
+				AdmissionReviewVersions: []string{"v1beta1"},
+				ReinvocationPolicy:      &reinvokeNever,
+			}},
+			ExpectAllow:            true,
+			ExpectReinvokeWebhooks: map[string]bool{"addLabel": false},
+		},
+		{
+			Name: "match & never reinvoke policy (by default)",
+			Webhooks: []registrationv1beta1.MutatingWebhook{{
+				Name:                    "addLabel",
+				ClientConfig:            ccfgSVC("addLabel"),
+				Rules:                   matchEverythingRules,
+				NamespaceSelector:       &metav1.LabelSelector{},
+				AdmissionReviewVersions: []string{"v1beta1"},
+			}},
+			ExpectAllow:            true,
+			ExpectReinvokeWebhooks: map[string]bool{"addLabel": false},
+		},
+		{
+			Name: "match & no reinvoke",
+			Webhooks: []registrationv1beta1.MutatingWebhook{{
+				Name:                    "noop",
+				ClientConfig:            ccfgSVC("noop"),
+				Rules:                   matchEverythingRules,
+				NamespaceSelector:       &metav1.LabelSelector{},
+				AdmissionReviewVersions: []string{"v1beta1"},
+			}},
+			ExpectAllow: true,
+		},
 	}
 }
 

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/webhook_server.go

@@ -138,6 +138,13 @@ func webhookHandler(w http.ResponseWriter, r *http.Request) {
 				},
 			},
 		})
+	case "/noop":
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(&v1beta1.AdmissionReview{
+			Response: &v1beta1.AdmissionResponse{
+				Allowed: true,
+			},
+		})
 	default:
 		http.NotFound(w, r)
 	}

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

@@ -160,7 +160,7 @@ func (ps *Plugins) NewFromPlugins(pluginNames []string, configProvider ConfigPro
 	if len(validationPlugins) != 0 {
 		klog.Infof("Loaded %d validating admission controller(s) successfully in the following order: %s.", len(validationPlugins), strings.Join(validationPlugins, ","))
 	}
-	return chainAdmissionHandler(handlers), nil
+	return newReinvocationHandler(chainAdmissionHandler(handlers)), nil
 }
 
 // InitPlugin creates an instance of the named interface.

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

@@ -0,0 +1,62 @@
+/*
+Copyright 2019 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 admission
+
+// newReinvocationHandler creates a handler that wraps the provided admission chain and reinvokes it
+// if needed according to re-invocation policy of the webhooks.
+func newReinvocationHandler(admissionChain Interface) Interface {
+	return &reinvoker{admissionChain}
+}
+
+type reinvoker struct {
+	admissionChain Interface
+}
+
+// Admit performs an admission control check using the wrapped admission chain, reinvoking the
+// admission chain if needed according to the reinvocation policy.  Plugins are expected to check
+// the admission attributes' reinvocation context against their reinvocation policy to decide if
+// they should re-run, and to update the reinvocation context if they perform any mutations.
+func (r *reinvoker) Admit(a Attributes, o ObjectInterfaces) error {
+	if mutator, ok := r.admissionChain.(MutationInterface); ok {
+		err := mutator.Admit(a, o)
+		if err != nil {
+			return err
+		}
+		s := a.GetReinvocationContext()
+		if s.ShouldReinvoke() {
+			s.SetIsReinvoke()
+			// Calling admit a second time will reinvoke all in-tree plugins
+			// as well as any webhook plugins that need to be reinvoked based on the
+			// reinvocation policy.
+			return mutator.Admit(a, o)
+		}
+	}
+	return nil
+}
+
+// Validate performs an admission control check using the wrapped admission chain, and returns immediately on first error.
+func (r *reinvoker) Validate(a Attributes, o ObjectInterfaces) error {
+	if validator, ok := r.admissionChain.(ValidationInterface); ok {
+		return validator.Validate(a, o)
+	}
+	return nil
+}
+
+// Handles will return true if any of the admission chain handlers handle the given operation.
+func (r *reinvoker) Handles(operation Operation) bool {
+	return r.admissionChain.Handles(operation)
+}

test/integration/apiserver/admissionwebhook/BUILD

@@ -6,6 +6,7 @@ go_test(
         "admission_test.go",
         "broken_webhook_test.go",
         "main_test.go",
+        "reinvocation_test.go",
     ],
     rundir = ".",
     tags = [
@@ -21,6 +22,7 @@ go_test(
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/api/extensions/v1beta1:go_default_library",
         "//staging/src/k8s.io/api/policy/v1beta1:go_default_library",
+        "//staging/src/k8s.io/api/scheduling/v1:go_default_library",
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

test/integration/apiserver/admissionwebhook/reinvocation_test.go

@@ -0,0 +1,389 @@
+/*
+Copyright 2019 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 admissionwebhook
+
+import (
+	"crypto/tls"
+	"crypto/x509"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"reflect"
+	"strings"
+	"sync"
+	"testing"
+
+	"k8s.io/api/admission/v1beta1"
+	admissionv1beta1 "k8s.io/api/admissionregistration/v1beta1"
+	registrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
+	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
+	schedulingv1 "k8s.io/api/scheduling/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	clientset "k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
+	kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
+	"k8s.io/kubernetes/test/integration/framework"
+)
+
+const (
+	testReinvocationClientUsername = "webhook-reinvocation-integration-client"
+)
+
+// TestWebhookReinvocationPolicy ensures that the admission webhook reinvocation policy is applied correctly.
+func TestWebhookReinvocationPolicy(t *testing.T) {
+	reinvokeNever := registrationv1beta1.NeverReinvocationPolicy
+	reinvokeIfNeeded := registrationv1beta1.IfNeededReinvocationPolicy
+
+	type testWebhook struct {
+		path   string
+		policy *registrationv1beta1.ReinvocationPolicyType
+	}
+
+	testCases := []struct {
+		name                 string
+		initialPriorityClass string
+		webhooks             []testWebhook
+		expectLabels         map[string]string
+		expectInvocations    map[string]int
+		expectError          bool
+		errorContains        string
+	}{
+		{ // in-tree (mutation), webhook (no mutation), no reinvocation required
+			name:                 "no reinvocation for in-tree only mutation",
+			initialPriorityClass: "low-priority", // trigger initial in-tree mutation
+			webhooks: []testWebhook{
+				{path: "/noop", policy: &reinvokeIfNeeded},
+			},
+			expectInvocations: map[string]int{"/noop": 1},
+		},
+		{ // in-tree (mutation), webhook (mutation), reinvoke in-tree (no-mutation), no webhook reinvocation required
+			name:                 "no webhook reinvocation for webhook when no in-tree reinvocation mutations",
+			initialPriorityClass: "low-priority", // trigger initial in-tree mutation
+			webhooks: []testWebhook{
+				{path: "/addlabel", policy: &reinvokeIfNeeded},
+			},
+			expectInvocations: map[string]int{"/addlabel": 1},
+		},
+		{ // in-tree (mutation), webhook (mutation), reinvoke in-tree (mutation), webhook (no-mutation), both reinvoked
+			name:                 "webhook is reinvoked after in-tree reinvocation",
+			initialPriorityClass: "low-priority", // trigger initial in-tree mutation
+			webhooks: []testWebhook{
+				// Priority plugin is ordered to run before mutating webhooks
+				{path: "/setpriority", policy: &reinvokeIfNeeded}, // trigger in-tree reinvoke mutation
+			},
+			expectInvocations: map[string]int{"/setpriority": 2},
+		},
+		{ // in-tree (mutation), webhook A (mutation), webhook B (mutation), reinvoke in-tree (no-mutation), reinvoke webhook A (no-mutation), no reinvocation of webhook B required
+			name:                 "no reinvocation of webhook B when in-tree or prior webhook mutations",
+			initialPriorityClass: "low-priority", // trigger initial in-tree mutation
+			webhooks: []testWebhook{
+				{path: "/addlabel", policy: &reinvokeIfNeeded},
+				{path: "/conditionaladdlabel", policy: &reinvokeIfNeeded},
+			},
+			expectLabels:      map[string]string{"x": "true", "a": "true", "b": "true"},
+			expectInvocations: map[string]int{"/addlabel": 2, "/conditionaladdlabel": 1},
+		},
+		{ // in-tree (mutation), webhook A (mutation), webhook B (mutation), reinvoke in-tree (no-mutation), reinvoke webhook A (mutation), reinvoke webhook B (mutation), both webhooks reinvoked
+			name:                 "all webhooks reinvoked when any webhook reinvocation causes mutation",
+			initialPriorityClass: "low-priority", // trigger initial in-tree mutation
+			webhooks: []testWebhook{
+				{path: "/settrue", policy: &reinvokeIfNeeded},
+				{path: "/setfalse", policy: &reinvokeIfNeeded},
+			},
+			expectLabels:      map[string]string{"x": "true", "fight": "false"},
+			expectInvocations: map[string]int{"/settrue": 2, "/setfalse": 2},
+		},
+		{
+			name: "invalid priority class set by webhook should result in error from in-tree priority plugin",
+			webhooks: []testWebhook{
+				// Priority plugin is ordered to run before mutating webhooks
+				{path: "/setinvalidpriority", policy: &reinvokeIfNeeded},
+			},
+			expectError:       true,
+			errorContains:     "no PriorityClass with name invalid was found",
+			expectInvocations: map[string]int{"/setinvalidpriority": 1},
+		},
+		{
+			name: "'reinvoke never' policy respected",
+			webhooks: []testWebhook{
+				{path: "/conditionaladdlabel", policy: &reinvokeNever},
+				{path: "/addlabel", policy: &reinvokeNever},
+			},
+			expectLabels:      map[string]string{"x": "true", "a": "true"},
+			expectInvocations: map[string]int{"/conditionaladdlabel": 1, "/addlabel": 1},
+		},
+		{
+			name: "'reinvoke never' (by default) policy respected",
+			webhooks: []testWebhook{
+				{path: "/conditionaladdlabel", policy: nil},
+				{path: "/addlabel", policy: nil},
+			},
+			expectLabels:      map[string]string{"x": "true", "a": "true"},
+			expectInvocations: map[string]int{"/conditionaladdlabel": 1, "/addlabel": 1},
+		},
+	}
+
+	roots := x509.NewCertPool()
+	if !roots.AppendCertsFromPEM(localhostCert) {
+		t.Fatal("Failed to append Cert from PEM")
+	}
+	cert, err := tls.X509KeyPair(localhostCert, localhostKey)
+	if err != nil {
+		t.Fatalf("Failed to build cert with error: %+v", err)
+	}
+
+	recorder := &invocationRecorder{counts: map[string]int{}}
+	webhookServer := httptest.NewUnstartedServer(newReinvokeWebhookHandler(recorder))
+	webhookServer.TLS = &tls.Config{
+
+		RootCAs:      roots,
+		Certificates: []tls.Certificate{cert},
+	}
+	webhookServer.StartTLS()
+	defer webhookServer.Close()
+
+	s := kubeapiservertesting.StartTestServerOrDie(t, kubeapiservertesting.NewDefaultTestServerOptions(), []string{
+		"--disable-admission-plugins=ServiceAccount",
+	}, framework.SharedEtcd())
+	defer s.TearDownFn()
+
+	// Configure a client with a distinct user name so that it is easy to distinguish requests
+	// made by the client from requests made by controllers. We use this to filter out requests
+	// before recording them to ensure we don't accidentally mistake requests from controllers
+	// as requests made by the client.
+	clientConfig := rest.CopyConfig(s.ClientConfig)
+	clientConfig.Impersonate.UserName = testReinvocationClientUsername
+	clientConfig.Impersonate.Groups = []string{"system:masters", "system:authenticated"}
+	client, err := clientset.NewForConfig(clientConfig)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	for priorityClass, priority := range map[string]int{"low-priority": 1, "high-priority": 10} {
+		_, err = client.SchedulingV1().PriorityClasses().Create(&schedulingv1.PriorityClass{ObjectMeta: metav1.ObjectMeta{Name: priorityClass}, Value: int32(priority)})
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	for i, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			recorder.Reset()
+			ns := fmt.Sprintf("reinvoke-%d", i)
+			_, err = client.CoreV1().Namespaces().Create(&v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			for i, webhook := range tt.webhooks {
+				defer registerWebhook(t, client, fmt.Sprintf("admission.integration.test%d", i), webhookServer.URL+webhook.path, webhook.policy)()
+			}
+
+			pod := &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: ns,
+					Name:      "labeled",
+					Labels:    map[string]string{"x": "true"},
+				},
+				Spec: corev1.PodSpec{
+					Containers: []v1.Container{{
+						Name:  "fake-name",
+						Image: "fakeimage",
+					}},
+				},
+			}
+			if tt.initialPriorityClass != "" {
+				pod.Spec.PriorityClassName = tt.initialPriorityClass
+			}
+			obj, err := client.CoreV1().Pods(ns).Create(pod)
+
+			if tt.expectError {
+				if err == nil {
+					t.Fatalf("expected error but got none")
+				}
+				if tt.errorContains != "" {
+					if !strings.Contains(err.Error(), tt.errorContains) {
+						t.Errorf("expected an error saying %q, but got: %v", tt.errorContains, err)
+					}
+				}
+				return
+			}
+
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if tt.expectLabels != nil {
+				labels := obj.GetLabels()
+				if !reflect.DeepEqual(tt.expectLabels, labels) {
+					t.Errorf("expected labels '%v', but got '%v'", tt.expectLabels, labels)
+				}
+			}
+
+			if tt.expectInvocations != nil {
+				for k, v := range tt.expectInvocations {
+					if recorder.GetCount(k) != v {
+						t.Errorf("expected %d invocations of %s, but got %d", v, k, recorder.GetCount(k))
+					}
+				}
+			}
+		})
+	}
+}
+
+func registerWebhook(t *testing.T, client clientset.Interface, name, endpoint string, reinvocationPolicy *registrationv1beta1.ReinvocationPolicyType) func() {
+	fail := admissionv1beta1.Fail
+	hook, err := client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Create(&admissionv1beta1.MutatingWebhookConfiguration{
+		ObjectMeta: metav1.ObjectMeta{Name: name},
+		Webhooks: []admissionv1beta1.MutatingWebhook{{
+			Name: name,
+			ClientConfig: admissionv1beta1.WebhookClientConfig{
+				URL:      &endpoint,
+				CABundle: localhostCert,
+			},
+			Rules: []admissionv1beta1.RuleWithOperations{{
+				Operations: []admissionv1beta1.OperationType{admissionv1beta1.OperationAll},
+				Rule:       admissionv1beta1.Rule{APIGroups: []string{"*"}, APIVersions: []string{"*"}, Resources: []string{"*/*"}},
+			}},
+			FailurePolicy:           &fail,
+			ReinvocationPolicy:      reinvocationPolicy,
+			AdmissionReviewVersions: []string{"v1beta1"},
+		}},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tearDown := func() {
+		err := client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations().Delete(hook.GetName(), &metav1.DeleteOptions{})
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+	return tearDown
+}
+
+type invocationRecorder struct {
+	mu     sync.Mutex
+	counts map[string]int
+}
+
+func (i *invocationRecorder) Reset() {
+	i.mu.Lock()
+	defer i.mu.Unlock()
+	i.counts = map[string]int{}
+}
+
+func (i *invocationRecorder) GetCount(path string) int {
+	i.mu.Lock()
+	defer i.mu.Unlock()
+	return i.counts[path]
+}
+
+func (i *invocationRecorder) IncrementCount(path string) {
+	i.mu.Lock()
+	defer i.mu.Unlock()
+	i.counts[path]++
+}
+
+func newReinvokeWebhookHandler(recorder *invocationRecorder) http.Handler {
+	patch := func(w http.ResponseWriter, patch string) {
+		w.Header().Set("Content-Type", "application/json")
+		pt := v1beta1.PatchTypeJSONPatch
+		json.NewEncoder(w).Encode(&v1beta1.AdmissionReview{
+			Response: &v1beta1.AdmissionResponse{
+				Allowed:   true,
+				PatchType: &pt,
+				Patch:     []byte(patch),
+			},
+		})
+	}
+	allow := func(w http.ResponseWriter) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(&v1beta1.AdmissionReview{
+			Response: &v1beta1.AdmissionResponse{
+				Allowed: true,
+			},
+		})
+	}
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		defer r.Body.Close()
+		data, err := ioutil.ReadAll(r.Body)
+		if err != nil {
+			http.Error(w, err.Error(), 400)
+		}
+		review := v1beta1.AdmissionReview{}
+		if err := json.Unmarshal(data, &review); err != nil {
+			http.Error(w, err.Error(), 400)
+		}
+		if review.Request.UserInfo.Username != testReinvocationClientUsername {
+			// skip requests not originating from this integration test's client
+			allow(w)
+			return
+		}
+
+		if len(review.Request.Object.Raw) == 0 {
+			http.Error(w, err.Error(), 400)
+		}
+		pod := &corev1.Pod{}
+		if err := json.Unmarshal(review.Request.Object.Raw, pod); err != nil {
+			http.Error(w, err.Error(), 400)
+		}
+
+		recorder.IncrementCount(r.URL.Path)
+
+		switch r.URL.Path {
+		case "/noop":
+			allow(w)
+		case "/settrue":
+			patch(w, `[{"op": "replace", "path": "/metadata/labels/fight", "value": "true"}]`)
+		case "/setfalse":
+			patch(w, `[{"op": "replace", "path": "/metadata/labels/fight", "value": "false"}]`)
+		case "/addlabel":
+			labels := pod.GetLabels()
+			if a, ok := labels["a"]; !ok || a != "true" {
+				patch(w, `[{"op": "add", "path": "/metadata/labels/a", "value": "true"}]`)
+				return
+			}
+			allow(w)
+		case "/conditionaladdlabel": // if 'a' is set, set 'b' to true
+			labels := pod.GetLabels()
+			if _, ok := labels["a"]; ok {
+				patch(w, `[{"op": "add", "path": "/metadata/labels/b", "value": "true"}]`)
+				return
+			}
+			allow(w)
+		case "/setpriority": // sets /spec/priorityClassName to high-priority if it is not already set
+			if pod.Spec.PriorityClassName != "high-priority" {
+				if pod.Spec.Priority != nil {
+					patch(w, `[{"op": "add", "path": "/spec/priorityClassName", "value": "high-priority"},{"op": "remove", "path": "/spec/priority"}]`)
+				} else {
+					patch(w, `[{"op": "add", "path": "/spec/priorityClassName", "value": "high-priority"}]`)
+				}
+				return
+			}
+			allow(w)
+		case "/setinvalidpriority":
+			patch(w, `[{"op": "add", "path": "/spec/priorityClassName", "value": "invalid"}]`)
+		default:
+			http.NotFound(w, r)
+		}
+	})
+}