Skip to content

Commit

Permalink
Add mutating webhook re-invocation details
Browse files Browse the repository at this point in the history
  • Loading branch information
liggitt committed May 16, 2019
1 parent 39b8b39 commit 4a47764
Showing 1 changed file with 76 additions and 16 deletions.
92 changes: 76 additions & 16 deletions keps/sig-api-machinery/00xx-admission-webhooks-to-ga.md
Original file line number Diff line number Diff line change
Expand Up @@ -242,22 +242,65 @@ type AdmissionRequest struct {

### Mutating Plugin ordering

Current ordering of the plugins (including webhooks) is fixed but may not work for all cases
(e.g. [this issue](https://github.com/kubernetes/kubernetes/issues/64333)). A mutating webhook
can add a completely new structure to the object (e.g. a `Container`) and other mutating plugins
may have opinion on those new structures (e.g. setting the image pull policy on all containers).
Although this problem can go deeper than two level (if the second mutating webhook added a new
structure based on the new `Container`), with current set of standard mutating plugins and use-cases,
it look like a rerun of mutating plugins is a sufficient fix. The proposal is to rerun all mutating
plugins (including webhooks) if there is any change by mutating webhooks. The assumption for this
process is all mutating plugins are idempotent and this must be clearly documented to users.
A single ordering of mutating admissions plugins (including webhooks) does not work for all cases
(see https://issue.k8s.io/64333 as an example). A mutating webhook can add a new sub-structure
to the object (like adding a `container` to a pod), and other mutating plugins which have already
run may have opinions on those new structures (like setting an `imagePullPolicy` on all containers).

Mutations from in-tree plugins will not trigger this process as they are well-ordered but if
there is any mutation by webhooks, all of the plugins including in-tree ones will be run again.
To allow mutating admission plugins to observe changes made by other plugins, regardless of order,
admission will add the ability to re-run mutating plugins (including webhooks) a single time if
there is any change made by a mutating webhook on the first pass.

This feature would be would be opt in and defaulted to false for `v1beta1`.
1. Initial mutating admission pass
2. If any webhook plugins returned patches modifying the object, do a second mutating admission pass
1. Run all in-tree mutating admission plugins
2. Run all applicable webhook mutating admission plugins that indicate they want to be reinvoked

The API representation and behavior for this feature is still under design and will be updated/approved here prior to implementation.
Mutating plugins that are reinvoked must be idempotent, able to successfully process an object they have already
admitted and potentially modified. This will be clearly documented in admission webhook documentation,
examples, and test guidelines. Mutating admission webhooks *should* already support this (since any
change they can make in an object could already exist in the user-provided object), and any webhooks
that do not are broken for some set of user input.

Note that idempotence (whether a webhook can successfully operate on its own output) is distinct from
the `sideEffects` indicator, which describes whether a webhook can safely be called in `dryRun` mode.

The reinvocation behavior will be opt-in, so `reinvocationPolicy` defaults to `"Never"`.

```golang
type Webhook struct {
...
// 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"`
}

type ReinvocationPolicyType string

var (
NeverReinvocationPolicy ReinvocationPolicyType = "Never"
IfNeededReinvocationPolicy ReinvocationPolicyType = "IfNeeded"
)
```

Although this problem can go deeper than two levels (for example, if the second mutating webhook added
a new structure based on the new `container`), a single re-run is sufficient for the current set of
in-tree plugins and webhook use-cases. If future use cases require additional or more targeted reinvocations,
the `AdmissionReview` response can be enhanced to provide information to target those reinvocations.

### Passing {Operation}Option to Webhook

Expand Down Expand Up @@ -652,6 +695,24 @@ type Webhook struct {
// +optional
FailurePolicy *FailurePolicyType `json:"failurePolicy,omitempty" protobuf:"bytes,4,opt,name=failurePolicy,casttype=FailurePolicyType"`

// 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"`

// NamespaceSelector decides whether to run the webhook on an object based
// on whether the namespace for that object matches the selector. If the
// object itself is a namespace, the matching is performed on
Expand Down Expand Up @@ -859,8 +920,6 @@ Also there need to be sufficient tests for any of these new features and all exi
There are still open questions that need to be addressed and updated in this KEP before graduation:
* ConnectOptions is sent as the main object to the webhooks today (and it is mutable). Should we change that and send parent object as the main object?
* Update with design and test details for "convert to webhook-requested version"
* Update with design and test details for "mutating plugin ordering"
## Post-GA tasks
Expand All @@ -876,4 +935,5 @@ These are related Post-GA tasks:
* First version of the KEP being merged: Jan 29th, 2019
* The set of features for GA approved, initial set of features marked implementable: Feb 4th, 2019
* Implementation start for all approved changes: Feb 4th, 2019
* Target Kubernetes version for GA: TBD
* Added details for auto-conversion: Apr 25th, 2019
* Added details for mutating admission re-running: May 6th, 2019

0 comments on commit 4a47764

Please sign in to comment.