Skip to content

Commit

Permalink
addressed some comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mbohlool committed Jan 30, 2019
1 parent cf3fbb9 commit 917dd5d
Showing 1 changed file with 12 additions and 7 deletions.
19 changes: 12 additions & 7 deletions keps/sig-api-machinery/00xx-admission-webhooks-to-ga.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ Based on the user feedback, these are proposed changes to current feature to gra

* Object Selector: Add a general selector for objects that a webhook can target.
* Scope: Define the scope of the targeted objects, e.g. cluster vs. namespaced.
* http for local host: relax https constrain for localhost.
* timeout configuration: Add a configuration field to set the timeout of the webhook call.
* http for localhost: relax https constraint for localhost.
* timeout configuration: Add a configuration field to shorten the timeout of the webhook call.
* port configuration: Add a configuration field to change the service port from the default 443.
* AdmissionReview version selection: Add a API field to set the AdmissionReview versions a webhook can understand.
* Pass existing objects to delete and delete collection admission webhooks
* re-run mutating plugins if any webhook changed object
* re-run mutating plugins if any webhook changed object to fix the plugin ordering problem
* pass OperationOption (such as CreateOption/DeleteOption) to the webhook

### Non-Goals
Expand All @@ -79,8 +79,11 @@ This section describes each of the goals in the [Goals](#goals) section in more

### Object selector

Currently Admission Webhook supports namespace selectors, but that may not be enough for some cases that admission webhook need to be skipped on some objects. For example if the Admission Webhook is running inside cluster and its rules includes wildcards which match required API objects for its own execution, it won't work. An object selector would be useful exclude those objects. Also in case of an optional webhooks, an object selector gives the end user ability to include or exclude an object without having access to admission webhook configuration which is normally restricted to cluster admins.
Note that namespace objects must match both object selector (if specified) and namespace selector to be sent to the Webhook.
Currently Admission Webhook supports namespace selectors, but that may not be enough for some cases that admission webhook need to be skipped on some objects. For example if the Admission Webhook is running inside cluster and its rules includes wildcards which match required API objects for its own execution, it won't work. An object selector would be useful exclude those objects.

Also in case of an optional webhooks, an object selector gives the end user ability to include or exclude an object without having access to admission webhook configuration which is normally restricted to cluster admins.

Note that namespaced objects must match both the object selector (if specified) and namespace selector to be sent to the Webhook.

The object selector applies to an Object's labels. For create and update, the object is the new object. For delete, it is existing object (passed as `oldObject` in the `admissionRequest`). For sub-resources, if the subresource accepts the whole object and apply the changes to object's labels (e.g. `pods/status`) the labels on the new object will be used otherwise (e.g. `pods/proxy`) the labels on the existing parent object will be used. If subresource does not have a parent object, the Webhook call may fail or skipped based on the failure policy.

Expand Down Expand Up @@ -132,11 +135,13 @@ type Rule struct {

Admission Webhook requires https for all connections. This is not necessary when the webhook in running on the same machine as apiserver (like a side-car or static pod). By relaxing this restriction, it would be possible to run side-cars or static pods on the same machine without cert management. The API change for this feature is documentation and relaxing validation.

Note that there must be caution around any validation relaxing in regard to rolling-back the cluster. It must be documented to the end user not to use the feature until they are not going to roll-back the cluster to a version that does not have this feature. If they do, they need to delete or update these webhook configuration as the old cluster does not allow running insecure webhooks as sidecar. To delete these configurations in an old API server, one must first fix the http validation problem otherwise finalizers would not allow deletion the object.
Note that there must be caution around any validation relaxing in regard to rolling-back the cluster. It must be documented to the end user not to use the feature until they are not going to roll-back the cluster to a version that does not have this feature.

If users wants to rollback safely to a version of API Server that does not support this feature, they need to delete or update these webhook configuration as the old cluster does not allow running insecure webhooks as sidecar. To delete these configurations in an old API server, one must first fix the http validation problem otherwise finalizers would not allow deletion the object.

### timeout configuration

Admission Webhook has a default timeout of 30 seconds today but long running webhooks would result in a poor performance. By adding a timeout field to the configuration, the webhook author can limit the running time of the webhook that either result in failing the API call earlier or ignore the webhook call based on the failure policy. This feature, however, would not let the timeout to be extended more than 30 seconds and the timeout would be defaulted to a shorter value (e.g. 10 seconds) for v1 API while stays 30 second for v1beta API to keep backward compatibility.
Admission Webhook has a default timeout of 30 seconds today, but long running webhooks would result in a poor performance. By adding a timeout field to the configuration, the webhook author can limit the running time of the webhook that either result in failing the API call earlier or ignore the webhook call based on the failure policy. This feature, however, would not let the timeout to be extended more than 30 seconds and the timeout would be defaulted to a shorter value (e.g. 10 seconds) for v1 API while stays 30 second for v1beta API to keep backward compatibility.

```golang
type Webhook struct {
Expand Down

0 comments on commit 917dd5d

Please sign in to comment.