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

Allow manifest based registration of Admission webhooks. #1872

Closed
vivekbagade opened this issue Jun 24, 2020 · 10 comments
Closed

Allow manifest based registration of Admission webhooks. #1872

vivekbagade opened this issue Jun 24, 2020 · 10 comments
Labels
sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. tracked/no Denotes an enhancement issue is NOT actively being tracked by the Release Team

Comments

@vivekbagade
Copy link
Contributor

vivekbagade commented Jun 24, 2020

Enhancement Description

  • One-line enhancement description (can be used as a release note):
    Manifest based webhook configuration allows registering admission webhooks
    during kube-apiserver start up allowing for no delays in policy enforcement
    between policy addition and kube-apiserver startup.
  • Kubernetes Enhancement Proposal: https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/1872-manifest-based-admission-webhooks
  • Primary contact (assignee): vivekbagade@
  • Responsible SIGs: sig-api-machinery
  • Enhancement target (which target equals to which milestone):
    • Alpha release target (1.20)
    • Beta release target (1.21)
    • Stable release target (TBD)
@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jun 24, 2020
@vivekbagade
Copy link
Contributor Author

vivekbagade commented Jun 29, 2020

/sig api-machinery

@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 29, 2020
@kikisdeliveryservice
Copy link
Member

kikisdeliveryservice commented Sep 12, 2020

Hi @vivekbagade

Enhancements Lead here. Can you please confirm that you will be working towards alpha in 1.20?

Also could you please update the link in the description to link directly to the merged kep: https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/1872-manifest-based-admission-webhooks

Thanks!
Kirsten

@kikisdeliveryservice
Copy link
Member

kikisdeliveryservice commented Sep 27, 2020

Hi @vivekbagade

Any updates on whether this will be included in 1.20?

Enhancements Freeze is October 6th and by that time we require:

The KEP must be merged in an implementable state
The KEP must have test plans
The KEP must have graduation criteria
The KEP must have an issue in the milestone

Thanks
Kirsten

@kikisdeliveryservice kikisdeliveryservice added the tracked/no Denotes an enhancement issue is NOT actively being tracked by the Release Team label Oct 7, 2020
@vivekbagade
Copy link
Contributor Author

vivekbagade commented Nov 25, 2020

@deads2k @liggitt

@lavalamp had expressed concerns on manifest based webhooks in #2074 (comment)

I strongly feel that this mechanism as proposed helps administrators solve the problems mentioned in the KEP. Only current solution to which is having compiled in admission controllers. That said, the concerns Daniel mentions are apt to discuss.

We need to make a call on this. I intend to target 1.21 with a complete implementation for this.

@lavalamp
Copy link
Member

lavalamp commented Jan 25, 2021

(Current status: Vivek is going to explore bootstrapping objects from a file (and loading them into the cluster) in a future KEP.)

@lavalamp
Copy link
Member

lavalamp commented Mar 5, 2021

@vivekbagade are you coming back to this? People are interested in the replacement :)

@tallclair
Copy link
Member

tallclair commented May 11, 2021

I'd like to add another use case for this: staged rollout of admission changes to HA clusters. In other words, I'd like to be able to update the admission configuration for 1 api-server instance at a time to catch errors without bringing down the whole cluster. We could of course address this with an enhancement to the regular (dynamic) webhooks, but I think the static file approach would address it by default.

@lavalamp
Copy link
Member

lavalamp commented May 12, 2021

I'd like to be able to update the admission configuration for 1 api-server instance at a time to catch errors without bringing down the whole cluster

Regular admission webhooks have this problem. I would definitely not want to solve it for only certain webhooks. Additionally, breaking "only" 1/Nth of the cluster traffic is actually a bad solution to this problem:

  • reentrant calls (webhooks) mean a request may effectively make more than one request, giving more chances to hit the broken apiserver
  • In most clusters, some apiserver ends up with the majority of traffic, due to a few different reasons. This means a policy change on one apiserver is either mostly not tested or mostly affects the whole cluster.

Anyway, the replacements to this approach at the moment are:

  • #101815
  • #101762

@tallclair
Copy link
Member

tallclair commented May 14, 2021

@lavalamp I agree that solving this in a general way makes sense. I filed kubernetes/kubernetes#102019 to capture the feature request.

@SaranBalaji90
Copy link

SaranBalaji90 commented Jul 13, 2021

This issue can be closed based on #2822

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. tracked/no Denotes an enhancement issue is NOT actively being tracked by the Release Team
Projects
None yet
Development

No branches or pull requests

6 participants