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

[liggitt]

follow up to #736 (comment) with proposed design for "conversion to requested-version" capability (#736 (comment))

/sig api-machinery
/assign @lavalamp @deads2k @mbohlool

[deads2k]

separate and just merge teh line breaks?

[liggitt]

separate and just merge teh line breaks?

done - #803

[liggitt]

limited this to the proposed "convert to webhook-requested version" behavior, for discussion during 1.14.

Moved requested updates for items planned for 1.14 to #806 and marks those as implementable.

[caesarxuchao]

/sub

[mbohlool]

/lgtm

[caesarxuchao]

RE. the discussion in the SIG meeting regarding whether to use ResourceID to decide if the apiserver should attempt a conversion. Jordan answered that instead, we should leave it to the apiserver to build an internal map based on if the apiserver knows how to convert two resources. The reason Jordan gave was that even if the ResourceID showed two resource are referring to the same set of objects, as long as the apiserver didn't know how to convert, there was little the apiserver could do.

I think that's actually what we want to capture. The apiserver should fire an alert in this case, because it means that attackers can walk around the webhook to modify the protected resource by visiting the unconvertible resource, defeating the purpose of this KEP.

Also, note that even if two resource endpoints are convertible, as long as they refer to different objects in etcd, a webhook probably doesn't want to intercept requests to both endpoints. If that's the case, then to build the internal map, the knowledge of if two kinds are convertible isn't enough, the apiserver needs to do the same analysis it does when populating the ResourceID, i.e., analyse the etcd path of the resource.

[liggitt]

because it means that attackers can walk around the webhook to modify the protected resource by visiting the unconvertible resource

The webhook should only receive resources that are stored in the same location as the resources they registered for. If they are stored in the same location, they must be convertible.

the knowledge of if two kinds are convertible isn't enough, the apiserver needs to do the same analysis it does when populating the ResourceID, i.e., analyse the etcd path of the resource.

I agree with that. The question is how we build the map of colocated group/resource/versions.

[caesarxuchao]

I think the apiserver will build such a mapping once. The webhook controller will use the mapping directly. The same mapping will be used to populate the ResourceID.

[liggitt]

I've started looking into building the mapping, but I don't think we need to hold the design bits on that part of the implementation

ping @lavalamp @deads2k

[lavalamp]

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

[liggitt]

changed to MatchPolicy: Exact | Equivalent

[lavalamp]

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.)

[liggitt]

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

[liggitt]

addressed comments

[lavalamp]

/lgtm
/approve

matchPolicy: equivalent is much more understandable, thanks.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lavalamp, liggitt, mbohlool

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• keps/sig-api-machinery/OWNERS [lavalamp]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment