Move webhooks out of the API definitions

[michaelasp]

Is your feature request related to a problem?

We import the metallb API in our controllers to create the various CRDs. Controller-runtime has made a breaking change making the API of 0.15 webhooks incompatible with 0.14 leading us to be unable to import metallb as a result without conflicts until it is bumped. We can always create our own folder and import the APIs solely without the webhooks, but if the API changes we are vulnerable to losing track of it.

An example of the issues we run into at the moment.

# go.universe.tf/metallb/api/v1beta2
vendor/go.universe.tf/metallb/api/v1beta2/bgppeer_webhook.go:39:27: cannot use &BGPPeer{} (value of type *BGPPeer) as admission.Validator value in variable declaration: *BGPPeer does not implement admission.Validator (wrong type for method ValidateCreate)
                have ValidateCreate() error
                want ValidateCreate() (admission.Warnings, error)

Describe the solution you'd like

What we've done with a lot of our packages is pull out the webhooks and have them in their own package so that version change doesn't matter, however that requires a refactor from the webhook as a method on the API type since that cannot be pulled out of the package. Instead we could use a generic handle function.

For the time being, an upgrade to 0.15 with the corresponding interface changes, such as validations being switched from returning an err to (admission.Warnings, error) should suffice.

Additional context

I can work on this, feel free to assign to me and I can send a pull request in later this week or next week.

I've read and agree with the following

• I've checked all open and closed issues and my request is not there.
• I've checked all open and closed pull requests and my request is not there.

[michaelasp]

Context on controller runtimes end here kubernetes-sigs/controller-runtime#2596

[oribon]

Hi, in v0.13.12 we bumped to 0.16.x

metallb/go.mod

Line 26 in 4f7e299

sigs.k8s.io/controller-runtime v0.16.3

and the webhooks implement the new interface. not sure I understand, is there anything missing?

[michaelasp]

Ah I was looking at an older version, this should suffice! I'm still wondering if we should move webhooks out of the API definitions for similar reasoning. It's been a major pain point for us importing any API definitions when importing APIs that we have to rely on matching controller runtime versions. Wdyt?

[fedepaol]

Ah I was looking at an older version, this should suffice! I'm still wondering if we should move webhooks out of the API definitions for similar reasoning. It's been a major pain point for us importing any API definitions when importing APIs that we have to rely on matching controller runtime versions. Wdyt?

That makes sense to me. But in order to avoid the need for importing, the api would need to stay in a separate go module, which I am not against.

[michaelasp]

Got it, I can do that. However, we've noticed that just by pulling out the webhooks from the API package was enough, since go only pulls in the relevant code. It would be more explicit if we declared it to be it's own submodule however. I don't mind either way, I'm also leaning towards making it a module.

[fedepaol]

Got it, I can do that. However, we've noticed that just by pulling out the webhooks from the API package was enough, since go only pulls in the relevant code. It would be more explicit if we declared it to be it's own submodule however. I don't mind either way, I'm also leaning towards making it a module.

Ah even better! I'd keep things simple then and stay with one single module if possible.

[michaelasp]

Created a PR which does this, completely decoupling webhooks from the API types. Let me know what you think @fedepaol @oribon :)