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

Do webhooks have a place in KCP #143

Closed
maleck13 opened this issue Aug 12, 2021 · 13 comments
Closed

Do webhooks have a place in KCP #143

maleck13 opened this issue Aug 12, 2021 · 13 comments
Assignees
Labels
area/kubernetes Issues or changes for Kubernetes as it relates to kcp new-investigation A topic that seems likely to be an area of investigation
Milestone

Comments

@maleck13
Copy link

Wondering if things like the validating and mutating webhooks have a place in KCP? Maybe they are already present?

@imjasonh
Copy link
Contributor

They're not currently implemented, but we will have to support them, to be able to support the ecosystem of controllers that expect/require them.

We're working through how Services work in a multicluster setup now, and as part of that we'll have to make sure the kcp layer can call those services too as webhooks.

@smarterclayton smarterclayton self-assigned this Sep 21, 2021
@ncdc
Copy link
Member

ncdc commented Oct 20, 2021

One option could be to use the url field in the webhook's clientConfig section, instead of service... although that likely wouldn't work out of the box with 99-100% of existing webhook configs out there today that presumably are using service.

@sttts
Copy link
Member

sttts commented Oct 20, 2021

Do webhooks have a place in this when we have an embedded programming language for admission?

@ncdc
Copy link
Member

ncdc commented Oct 20, 2021

Long-term, probably not. But until projects have migrated from validating/mutating webhooks to the embedded language, if they want target running against KCP, KCP will need webhooks.

@maleck13
Copy link
Author

maleck13 commented Oct 21, 2021

Is there any more detail on the embedded language for admission? wondering if it would be something like rego (which can be embedded in go) from open policy agent?

@sttts
Copy link
Member

sttts commented Oct 21, 2021

@imjasonh
Copy link
Contributor

Do webhooks have a place in this when we have an embedded programming language for admission?

I think so, yes. I'm very excited about KEP-2876, but even though CEL is miles better than JSONschema validation, there are things it's just not practical to do.

For example, Tekton has a bunch of code to validate the DAG of Tasks in a Pipeline, which is really not something I'd want to have to maintain (and utterly fail to write tests for!) in CEL.

CEL is going to replace a lot of webhook validation today, but I don't think it can (or should) replace all of it.

@sttts
Copy link
Member

sttts commented Oct 21, 2021

True. Everything that is not self-contained and calls out to other resources or services won't fit into CEL.

@sttts
Copy link
Member

sttts commented Oct 21, 2021

Which leads us to a nice problem: if we have our copy of the webhook, it will be based on different resources than the one in the downstream cluster, leading to different answers potentially.

@ncdc ncdc added area/kubernetes Issues or changes for Kubernetes as it relates to kcp new-investigation A topic that seems likely to be an area of investigation labels Feb 23, 2022
@ncdc ncdc added this to the TBD milestone Feb 23, 2022
@maleck13
Copy link
Author

maleck13 commented Mar 3, 2022

Example use cases: An ingress controller that wants to control the hosts that can be used for ingress. A controller that wants to ensure some quota / subscription is enabled based on a resource it provides.

@ncdc ncdc modified the milestones: TBD, v0.4.0 Apr 21, 2022
@ncdc
Copy link
Member

ncdc commented Apr 21, 2022

We are adding support for validating and mutating webhooks for full external URLs (not service URLs), at least as step 1.

@shawn-hurley
Copy link

Step 1 Implementation is here: #818

@ncdc
Copy link
Member

ncdc commented May 2, 2022

Closing this now that 818 is merged. We can open additional issues if needed

@ncdc ncdc closed this as completed May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubernetes Issues or changes for Kubernetes as it relates to kcp new-investigation A topic that seems likely to be an area of investigation
Projects
None yet
Development

No branches or pull requests

6 participants