Should we refactor the webhook implementation by using kubebuilder webhook scaffolding?

[cvvz]

Is your feature request related to a problem? Please describe.

Currently, the webhook is implemented by knative library. I suggest to refactor by using kubebuilder webhook scaffolding, which is the common practice for building operator. And in this way, we can make the webhook code consistent with controller code and avoid using a third-party libraries.

I'm not familiar with knative and maybe I didn't get the advantage of using knative to build webhook ranther than kubebuilder. I'm open to discuss this further.

Describe the solution you'd like

Describe alternatives you've considered

Additional context

[Fei-Guo]

We use knative webhook scaffolding for managing cert autorotation. It is the default webhook implementation for Karpenter. It looks slightly different from kubebuilder scaffolding but I found it is convenient and reliable.

[rambohe-ch]

@cvvz @Fei-Guo I have developed webhooks based on controller-runtime instead of kubebuilder and knative in grit, and solved the certificate rotation.

https://github.com/kaito-project/grit/blob/8bf3b5467b4b7b0e03ff4c099318b08e5768caac/pkg/gritmanager/webhooks/webhooks.go#L17

Btw: I think this implementation have obtained the benefits from OpenYurt and Karpenter, and prevent their shortages.

[cvvz]

@rambohe-ch I reviewed your implementation of the cert rotation part and didn’t find any issues. You used a separate reconciler to check whether the certificate is expired, and also used reconcile.Result{RequeueAfter: timeUntilNextCheck} to periodically trigger the cert reconcile process — that’s a very thorough approach.

Btw, if it were me, I might consider directly using something like cert-controller or cert-manager — I’m too lazy and don’t want to implement the reconciler myself. But since you’ve already implemented a good one by yourself, I don’t think there’s any need to change it.

As for the webhook implementation itself, I think if you're familiar with controller-runtime, it should be fine to not rely on the Kubebuilder scaffolding to auto-generate the webhook code, although IMO the code-generation is helpful, especially when we want to change the api and generate the manifest.

Using Kubebuilder/controller-runtime to implement the webhook can help simplify the code. For example, in the current Kaito webhook, it uses the client which is a global variable copied from mgr.GetClient() . By using controller-runtime, we can just use mgr.GetClient() as client.

[rambohe-ch]

@cvvz Thanks for your detailed response.

1. As a OSS project, It's not a good idea to rely on other components like cert-manager to generate or manage certificate rotation, and we should ensure the project can be worked independently.

2. I think kubebuilder is too complicated for an open source project which only need to manage controllers and webhooks. In Kubernetes ecosystem, It seems that most of OSS projects use controller-runtime to manage controllers and webhooks, even kubebuilder itself.

3. controller-gen tool is also used in grit project to generate manifest when api or rbac settings change.

[rambohe-ch]

add other reasons:

4. beside grit-manager component, other components(like girt-agent, grirt-runtime, etc.) exist in grit project, so we want to avoid kubebuilder’s opinionated structure. in the other word, we want to get low level control and flexibility.

[cvvz]

Yes, we should certainly use controller-runtime. Kubebuilder can be used to conveniently manage APIs and generate template code and manifests. Under the hood, it relies on controller-runtime. For complex projects, the best practice is to use controller-runtime to implement the controller manager.