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

Support Admission WebHooks that use Services #37

Open
Tracked by #39
davidfestal opened this issue Mar 8, 2023 · 5 comments
Open
Tracked by #39

Support Admission WebHooks that use Services #37

davidfestal opened this issue Mar 8, 2023 · 5 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.

Comments

@davidfestal
Copy link
Member

davidfestal commented Mar 8, 2023

The need

A quite big proportion of controllers that create Kubernetes standard workload objects (Deployments, ...) from higher-level or domain-specific CRDs define webhooks. And in most cases these are service-based webhooks.
These should be supported in a typical TMC environment if we want such operators to be usable, or at least testable under TMC.

First POC

A branch already already provides a hacky and non-mergeable implementation of service-based webhooks: kcp-dev/kcp#2910, which directs the webhooks to services/proxy sub-resources (themselves tunneled to the SyncTarget physical cluster.

While this works feature-wise and allows playing with controllers that require webhooks, this is a no-go and cannot be merged, since insecure: services/proxy uses insecure TLS connections, so that the CAs of the webhook configurations are ignored.

Required changes for a real implementation

A real implementation would require tunneling such requests at the TCP level, and not at the http level.
According to the first exploration, this probably involves the following changes:

  • in the KCP mutation and admission plugins:
    • inject the tunneler (once made available and public, as well as the tunneler.GetDialer function)
    • using a dedicated Endpointsresolver to
    • using a dedicated AuthenticationResolverWrapper that overrides the restConfig.Dialer to set it to the tunneler Dialer for the right SyncTarget. This would send the tcp traffic involved in the request directly to the other side (including the first TLS handshake)
    • This includes finding a way to pass to these resolvers the clusterName of the webhook configuration (which is also the clusterName of the webhook service), in addition to the namespace, name and port.
  • on the Syncer tunneler side:
    • remove the direct connection of the http server (associated with a NewSingleHostReverseProxy) to the tunneler lister
    • directly use the listener with a simple net server (at the TCP layer) that can:
      • peek the http request from the connection to find out the expected host
      • according to the case (either the physical cluster apiServer itself, as until now, or another host in the case of traffic sent from KCP to a service for webhooks)
        • forward the http request to the http server (apiServer case)
        • or send the raw traffic to the expected host (service case)
      • It might also be that we would open 2 connections from the Syncer to KCP SyncTarget tunnel sub-resource, one to setup a reverse connection dedicated to http traffic (the same as today), and the other one dedicated to tcp traffic routed according to some custom rules.
@davidfestal
Copy link
Member Author

Changes required for a real implementation started in the following branch:
kcp-dev/kcp@main...davidfestal:service-based-webhooks-at-tcp-level

@davidfestal davidfestal removed their assignment Mar 28, 2023
@davidfestal davidfestal added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Mar 28, 2023
@stevekuznetsov
Copy link

We probably want careful guidrails on this sort of thing to help users not DoS themselves in a sharded/multi-region kcp.

@davidfestal
Copy link
Member Author

davidfestal commented Mar 28, 2023

We probably want careful guidrails on this sort of thing to help users not DoS themselves in a sharded/multi-region kcp.

@stevekuznetsov Please provide more details here. help welcomed.

@mjudeikis
Copy link
Contributor

/transfer-issue contrib-tmc

1 similar comment
@mjudeikis
Copy link
Contributor

/transfer-issue contrib-tmc

@kcp-ci-bot kcp-ci-bot transferred this issue from kcp-dev/kcp Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.
Projects
Status: In Progress
Development

No branches or pull requests

3 participants