kubernetes / website

Kubernetes website and documentation repo:
https://kubernetes.io
Creative Commons Attribution 4.0 International
4.51k stars 14.46k forks source link

Document the best practices when implementing mutating webhook #47465

Open SergeyKanzhelev opened 3 months ago

SergeyKanzhelev commented 3 months ago

This is a Feature Request

What would you like to be added

This is a follow up from the https://github.com/kubernetes/website/pull/46825#issue-2354156381

  1. Document the best practices writing tools and mutating web hooks that will preserve unknown fields: Add tutorial about running Pods with sidecar containers #46825 (comment)

Specifically:

Why is this needed

KEPs like Sidecar Containers add new fields. We hit many issues when mutating webhook unintentionally removing the fields, they are not aware of. This highlights the issue that many mutating webhooks are implemented incorrectly and would benefit from following the best practices.

/assign @jpbetz

/sig api-machinery

natalisucks commented 2 months ago

/triage accepted

SergeyKanzhelev commented 2 months ago

@jpbetz @shannonxtreme any chance you can help with this some time soon? We are getting reports about the sidecar feature incompatibility from time to time and hope to get the document to point to.

jpbetz commented 2 months ago

I'll try to get some attention on this.

sftim commented 1 month ago

/priority important-longterm

due to relevance around sidecar containers turning into init containers /wg api-expression /language en

sftim commented 1 month ago

@shannonxtreme or @jpbetz if you'd like to pair on this (or pair up async), I can try to make time

shannonxtreme commented 1 month ago

Yeah I can pair up. What's the scope here? It sounds like a best practices page, and maybe should include a working example of a "correct" implementation?

sftim commented 1 month ago

Not sure about scope. We could do a blog article that links to a section within https://kubernetes.io/docs/tutorials/configuration/pod-sidecar-containers/ perhaps?

I'll message you via Slack

SergeyKanzhelev commented 1 month ago

Not sure about scope.

Ideally we need something that we can share whenever we see this issue. If some webhook stripped out the field we can say "please upgrade to k8s API 1.28+ so this field will be known and change the design of your webhook to follow the best practices to avoid problems like this going forward".

Scope-wise, maybe @jpbetz may also help to share some good practices.

shannonxtreme commented 1 month ago

/assign

shannonxtreme commented 1 month ago

I've written a quick and dirty skeleton for the structure https://docs.google.com/document/d/1pg80Qn3Uz2SupCHjuQ_CRhnh9m3q9a3KU_PGV3ecVFE/edit?usp=sharing

@jpbetz @sftim can you two take a look and add your thoughts/suggestions? I'll check with the API machinery SIG in slack too

jpbetz commented 1 month ago

This is great. I'll dropped in some comments.

AverageMarcus commented 1 month ago

I've added a few small notes but looks really good so far! I can see this being very useful when complete!

shannonxtreme commented 1 month ago

Alright, I've implemented a lot of the suggestions. I think that this is a good enough state for an initial draft - we can add more after publication as needed. @AverageMarcus @jpbetz @cici37 @SergeyKanzhelev @sftim can you please look at the doc one more time and change the LGTM dropdown for your name to LGTM if it's ok? I'll draft the content after that.

SergeyKanzhelev commented 1 month ago

lgtm from the content structure perspective. The doc promising to be very helpful!