WIP: Replace webhook validations with CEL validation

[erikgb]

I want to see if it's possible to remove the trust-manager validating webhook, as it will eliminate the requirement to pre-install cert-manager. But it doesn't seem possible, at least not with the current Bundle API, to fully replace the webhook validations with CEL api-server validation. We might get a bit further after redesigning the target structure ref. #242, but the label selectors require a webhook to be validated AFAIK.

I personally think the CEL validations are more compact and fairly easy to read, at least the common union validations. The additional formats validation CEL expression is probably a bit over the line, so I am open to reverting it. But I wanted to see how far I could get with CEL, and I even got assistance from Jordan Liggit himself! 🦄

Jordan suggested optional fields to reduce the complexity, but I decided to avoid them for now - since that will require Kubernetes version >= 1.29 to work. We can simplify later on.

Please let me know what you think!

/cc @juliocamarero @SgtCoDFish @inteon

[juliocamarero]

LGTM. I wasn't aware of the CEL validations, but they seem very expressive and easy to use, I like this approach 👍

[cert-manager-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: juliocamarero
Once this PR has been reviewed and has the lgtm label, please ask for approval from erikgb. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[SgtCoDFish]

Seeing the amount of code this removes is super cool 🚀 I've got a few questions to help me understand better!

[SgtCoDFish]

question: Did you run these tests against main, before switching to CEL validation? I think that would be valuable if we've not done it yet!

[erikgb]

Good point! I think some of them actually might fail, and that is because the webhook validation code is complex and error-prone. Let me pull the tests into a new PR to see.

[erikgb]

I just tried this, and it is not working - for "technical reasons". 😢 Our integration tests are poorly configured IMO, and improving this will require some changes that at least @inteon would reject. 😉 The main issue in this case, is that the validating webhook configuration is only available from the Helm chart. And to bootstrap an integration test with webhook enabled, we need the webhook configuration in a format that is machine-readable.

If we adopted more of the "best practices" established by kubebuilder, this would be an improvement - at least to me.

Relates to cert-manager/makefile-modules#209.

[SgtCoDFish]

question: Is a +kubebuilder comment the only way of specifying these validation rules? (I'm unfamiliar generally with this CEL validation stuff)

This CEL code is super important to our UX but it's really difficult to read, understand and test it when it's embedded in a comment like this - seems like a sharp edge.

This isn't a blocker or anything, but it seems a shame if this is the only way today!

[erikgb]

No, it should be possible to hack the expression directly into the CRDs, but that is not something I would do. 😆 The expressions are mainly complex because the API is suboptimal. After #486, I think the expressions could become really simple.

Thanks for reviewing @SgtCoDFish (and @juliocamarero)! But I think the best option here is to put this PR back into draft-mode and wait for a conclusion on #486.