Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update webhook admission proposal with "convert to webhook-requested version" #802

Merged
merged 2 commits into from
Apr 25, 2019
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 122 additions & 1 deletion keps/sig-api-machinery/00xx-admission-webhooks-to-ga.md
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,107 @@ Since the apiserver can convert between all of the versions by which a resource
is made available, this situation can be improved by having the apiserver
convert resources to the group/versions a webhook registered for.

The API representation and behavior for this feature is still under design and will be updated/approved here prior to implementation.
A `conversionPolicy` field will be added to the webhook configuration object,
liggitt marked this conversation as resolved.
Show resolved Hide resolved
allowing a webhook to choose whether the apiserver should ignore these requests
or convert them to a kind corresponding to a resource the webhook registered for
and send them to the webhook. For safety, this field defaults to Convert.

```golang
// Webhook describes an admission webhook and the resources and operations it applies to.
type Webhook struct {
...
// ConversionPolicy describes whether the webhook wants to be called when a request is made
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As described here, isn't it an "InterceptionPolicy"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to MatchPolicy: Exact | Equivalent

// to a resource that does not match "rules", but which modifies the same data
// as a resource that does match "rules".
// Allowed values are Ignore or Convert.
// "Ignore" means the request should be ignored.
// "Convert" means the request should be converted to a kind corresponding to a resource matched by "rules", and sent to the webhook.
// Defaults to Convert.
// +optional
ConversionPolicy *ConversionPolicyType `json:"conversionPolicy,omitempty"`
```
The apiserver will do the following:
1. For each resource, compute the set of other resources that access or affect the same data, and the kind of the expected object. For example:
* `apps,v1,deployments` (`apiVersion: apps/v1, kind: Deployment`) is also available via:
* `apps,v1beta2,deployments` (`apiVersion: apps/v1beta2, kind: Deployment`)
* `apps,v1beta1,deployments` (`apiVersion: apps/v1beta1, kind: Deployment`)
* `extensions,v1beta1,deployments` (`apiVersion: extensions/v1beta1, kind: Deployment`)
* `apps,v1,deployments/scale` (`apiVersion: autoscaling/v1, kind: Scale`) is also available via:
* `apps,v1beta2,deployments/scale` (`apiVersion: apps/v1beta2, kind: Scale`)
* `apps,v1beta1,deployments/scale` (`apiVersion: apps/v1beta1, kind: Scale`)
* `extensions,v1beta1,deployments/scale` (`apiVersion: extensions/v1beta1, kind: Scale`)
2. When evaluating whether to dispatch an incoming request to a webhook with
`conversionPolicy: Convert`, check the request's resource *and* all equivalent
resources against the ones the webhook had registered for. If needed, convert
the incoming object to one the webhook indicated it understood.
The `AdmissionRequest` sent to a webhook includes the fully-qualified
kind (group/version/kind) and resource (group/version/resource):
```golang
type AdmissionRequest struct {
...
// Kind is the type of object being manipulated. For example: Pod
Kind metav1.GroupVersionKind `json:"kind" protobuf:"bytes,2,opt,name=kind"`
// Resource is the name of the resource being requested. This is not the kind. For example: pods
Resource metav1.GroupVersionResource `json:"resource" protobuf:"bytes,3,opt,name=resource"`
// SubResource is the name of the subresource being requested. This is a different resource, scoped to the parent
// resource, but it may have a different kind. For instance, /pods has the resource "pods" and the kind "Pod", while
// /pods/foo/status has the resource "pods", the sub resource "status", and the kind "Pod" (because status operates on
// pods). The binding resource for a pod though may be /pods/foo/binding, which has resource "pods", subresource
// "binding", and kind "Binding".
// +optional
SubResource string `json:"subResource,omitempty" protobuf:"bytes,4,opt,name=subResource"`
```
Prior to this conversion feature, the resource and kind of the request made to the
API server, and the resource and kind sent in the AdmissionRequest were identical.
When a conversion occurs and the object we send to the webhook is a different kind
than was sent to the API server, or the resource the webhook registered for is different
than the request made to the API server, we have three options for communicating that to the webhook:
1. Do not expose that fact to the webhook:
* Set AdmissionRequest `kind` to the converted kind
* Set AdmissionRequest `resource` to the registered-for resource
2. Expose that fact to the webhook using the existing fields:
* Set AdmissionRequest `kind` to the API request's kind (not matching the object in the AdmissionRequest)
* Set AdmissionRequest `resource` to the API request's resource (not matching the registered-for resource)
3. Expose that fact to the webhook using new AdmissionRequest fields:
liggitt marked this conversation as resolved.
Show resolved Hide resolved
* Set AdmissionRequest `kind` to the converted kind
* Set AdmissionRequest `requestKind` to the API request's kind
* Set AdmissionRequest `resource` to the registered-for resource
* Set AdmissionRequest `requestResource` to the API request's resource
Option 1 loses information the webhook could use (for example, to enforce different validation or defaulting rules for newer APIs).
Option 2 risks breaking webhook logic by sending it resources it did not register for, and kinds it did not expect.
Option 3 is preferred, and is the safest option that preserves information for use by the webhook.
To support this, three fields will be added to AdmissionRequest, and populated with the original request's kind, resource, and subResource:
```golang
type AdmissionRequest struct {
...
// RequestKind is the type of object being manipulated in the original API request. For example: Pod
liggitt marked this conversation as resolved.
Show resolved Hide resolved
// If this differs from the value in "kind", conversion was performed.
// +optional
liggitt marked this conversation as resolved.
Show resolved Hide resolved
RequestKind *metav1.GroupVersionKind `json:"requestKind,omitempty"`
// RequestResource is the name of the resource being requested in the original API request. This is not the kind. For example: ""/v1/pods
// If this differs from the value in "resource", conversion was performed.
// +optional
RequestResource *metav1.GroupVersionResource `json:"requestResource,omitempty"`
// RequestSubResource is the name of the subresource being requested in the original API request. This is a different resource, scoped to the parent
// resource, but it may have a different kind. For instance, /pods has the resource "pods" and the kind "Pod", while
// /pods/foo/status has the resource "pods", the sub resource "status", and the kind "Pod" (because status operates on
// pods). The binding resource for a pod though may be /pods/foo/binding, which has resource "pods", subresource
// "binding", and kind "Binding".
// If this differs from the value in "subResource", conversion was performed.
// +optional
RequestSubResource string `json:"requestSubResource,omitempty"`
```
## V1 API
Expand Down Expand Up @@ -410,6 +510,17 @@ type Rule struct {
Scope ScopeType `json:"scope,omitempty" protobuf:"bytes,3,opt,name=scope"`
}

type ConversionPolicyType string

const (
// ConversionIgnore means that requests that do not match a webhook's rules but could be
// converted to a resource the webhook registered for, should be ignored.
ConversionIgnore ConversionPolicyType = "Ignore"
// ConversionConvert means that requests that do not match a webhook's rules but could be
// converted to a resource the webhook registered for, should be converted and sent to the webhook.
ConversionConvert ConversionPolicyType = "Convert"
)

type FailurePolicyType string

const (
Expand Down Expand Up @@ -516,6 +627,16 @@ type Webhook struct {
// on admission requests for ValidatingWebhookConfiguration and MutatingWebhookConfiguration objects.
Rules []RuleWithOperations `json:"rules,omitempty" protobuf:"bytes,3,rep,name=rules"`

// ConversionPolicy describes whether the webhook wants to be called when a request is made
liggitt marked this conversation as resolved.
Show resolved Hide resolved
// to a resource that does not match "rules", but which modifies the same data
// as a resource that does match "rules".
// Allowed values are Ignore or Convert.
// "Ignore" means the request should be ignored.
// "Convert" means the request should be converted to a kind corresponding to a resource matched by "rules", and sent to the webhook.
// Defaults to Convert.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a solid use case for ever setting it to ignore? It seems super backwards compatible, do we even need this option? (I didn't reread the whole kep, maybe the reasoning exists and I missed it.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use per-version defaulting and admission logic in rest handlers today, this gives equivalent power to out-of-tree admission

// +optional
ConversionPolicy *ConversionPolicyType `json:"conversionPolicy,omitempty"`

// FailurePolicy defines how unrecognized errors from the admission endpoint are handled -
// allowed values are Ignore or Fail. Defaults to Ignore.
// +optional
Expand Down