Controllers for *WebhookConfiguration

[mamachanko]

To facilitate TLS for webhooks it is common to rely on cert-manager APIs. It's easy to issue a certificate, mount it into one's controller and let cert-manager inject the CA bundle into the *WebhookConfiguration. However, often one doesn't want to depend on cert-manager APIs. Maybe the controller might get deployed to environments where cert-manager is not available or it's simply limit its dependencies.

As a solution, I can either write and maintain my own webhook controller (possibly with reconciler.io/runtime), or I use knative.dev/pkg’s. For example, github.com.vmware-tanzu/servicebinding use knative.dev/pkg for its webhooks. However, since this is a common, cross-cutting concern it would be nice if I could rely on a solution that supported and proven.

Knative’s webhook controllers aren’t straightforward to integrate with a kubebuilder-style controller manager. And knative.dev/pkg would be yet another dependency. Furthermore, iirc knative.dev/pkg has no stable/versioned public API.

Maybe such webhook controllers could be offered by reconciler.io/runtime itself or a sibling, say reconciler.io/webhooks. Furthermore, offering controller for webhooks (to eliminate a dependency of cert-manager) would present an additional adoption path for reconciler.io/runtime.

Like knative.dev/pkg webhook controllers the solution must play well with leader election. It should support defaulting and validating webhooks. Annoyingly the solution would have to absorb all the PKI problems which cert-manager solves like renewal etc.

Different levels of fidelity are conceivable with everything possible in between:

• Do the bare minimum to facilitate TLS; update a TLS Secret and *WebhookConfiguration.webhooks.clientConfig.caBundle
• Also keep rules in-sync; updates TLS Secret and *WebhookConfiguration.webhooks.{clientConfig.caBundle, rules}
• Manage all involved resources' complete lifecycle; create-update TLS Secret and *WebhookConfiguration

[scothis]

I see the value in avoiding a hard dependency on cert-manager.

A major downside to this approach is that the controller must have permission to update AdmissionControllers. This essentially would allow a rogue controller to start intercepting requests for any resource in the cluster and either scrape it's content (in the case of secrets), or modify the content to be anything it desires.

This is ok for controllers that are trusted, but not all controllers are. Ironically, the Service Binding reference implementation uses this approach to dynamically intercept updates of resources and inject values into their spec.

[mamachanko]

A major downside to this approach is that the controller must have permission to update AdmissionControllers.

"permission to update AdmissionControllers" as in RBAC for ValidatingWebhookConfiguration and DefaultingWebhookConfiguration?

This is ok for controllers that are trusted, but not all controllers are.

By which heuristic does one trust a controller or not? "Downloaded from the internet" probably no. "Provided by my vendor" maybe yes.

[scothis]

A major downside to this approach is that the controller must have permission to update AdmissionControllers.

"permission to update AdmissionControllers" as in RBAC for ValidatingWebhookConfiguration and DefaultingWebhookConfiguration?

It's MutatingWebhookConfiguration, but yea. If you can update the spec of any MWC you can update it to incept any api server request and mutate any resource on the cluster. It's in effect cluster-admin levels of access.

This is ok for controllers that are trusted, but not all controllers are.

By which heuristic does one trust a controller or not? "Downloaded from the internet" probably no. "Provided by my vendor" maybe yes.

It's going to be different for every user. Some care a lot, and inspect the rbac roles before installing anything on cluster. Others don't care at all and yolo it. Back when TGIK was going, Joe would always inspect both the resources that existed in an install, and the RBAC permissions granted.

[mamachanko]

I am happy to have learned about the possibility of exploiting the required RBAC.

Enterprise Sarcasm requires to point out that few users are ever given the choice though. Depends on the definition of "user" naturally.

However, whether an implementor wants to use risky RBAC or not is not a decision runtime should make for them. If it were to provide the tools it should l, of course, point out the risks involved. That being said, that's a reason not to consider the feature, don't you think?

[scothis]

It's a good feature to exist, even if it's not appropriate in many cases. It's also worth considering if this functionality belongs upstream in controller-runtime as it really has no interaction with existing reconciler runtime components/interfaces. Also fine to incubate here and then try to push upstream if there is interest.

[mamachanko]

The point of reconciler.io/runtime is to avoid sigs.k8s.io/controller-runtime in the first place 😆

Incubating using reconciler.io/runtime's reconcilers should make it much easier.

[scothis]

The point of reconciler.io/runtime is to avoid sigs.k8s.io/controller-runtime in the first place 😆

The goal was never to compete with, or avoid controller-runtime, but to pickup where it left users to "draw the rest of the owl" with a more opinionated approach.

Any project using our runtime is also a first class controller-runtime project. Just look at their main.go.

[mamachanko]

💯 to what you said. I should've been more specific and not say "avoid controller-runtime" but "avoid the complexities of implementing the Reconciler interface yourself and let reconciler.io/runtime help you focus on what matters".