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

Replace golang validation rules for gardener Custom Resources with CEL expressions in the CRD #11266

Open
voelzmo opened this issue Jan 30, 2025 · 4 comments
Labels
area/quality Output qualification (tests, checks, scans, automation in general, etc.) related kind/enhancement Enhancement, improvement, extension

Comments

@voelzmo
Copy link
Member

voelzmo commented Jan 30, 2025

How to categorize this issue?

/area quality
/kind enhancement

What would you like to be added:
In k8s 1.29, CEL validation rules were graduated to GA. This allows for specifying validations for your custom resources directly in the CRD definition. It is also supported by kubebuilder annotations.

See an example of how VPA switched to this here: https://github.com/kubernetes/autoscaler/pull/7690/files

Useful for creating custom CEL validations and looking at some examples: https://playcel.undistro.io/

@gardener-prow gardener-prow bot added area/quality Output qualification (tests, checks, scans, automation in general, etc.) related kind/enhancement Enhancement, improvement, extension labels Jan 30, 2025
@timuthy
Copy link
Member

timuthy commented Jan 31, 2025

While I understand that using CEL has its benefits, I'm not sure why this is now generally requested for the gardener/gardener repo while we have well tested validation logic already in place for many many components.

Maybe you can share more insights if you see any particular areas in this repo that benefit from CEL, so we could potentially find a structured way what to convert into CELs.
I think it will take a major investment to do this transition generally and don't see a compelling argument at the moment.

@voelzmo
Copy link
Member Author

voelzmo commented Feb 3, 2025

My thinking was that all the things we currently put into webhooks (see e.g. https://github.com/gardener/gardener/blob/d5d3c638c9092efcb68b030da49edc7d953f61f6/docs/concepts/admission-controller.md) would no longer need an additional back-and-forth between the apiserver and the admission-controller.

For all the other stuff, which we can nicely implement as AdmissionPlugins in GAPI (e.g. https://github.com/gardener/gardener/blob/d5d3c638c9092efcb68b030da49edc7d953f61f6/docs/concepts/apiserver-admission-plugins.md) , you're probably right and there is no big benefit – it already runs inside the apiserver without additional hops.

@timuthy
Copy link
Member

timuthy commented Feb 3, 2025

There might be a few places where validation happens on the input object only and we could replace a validation logic with CEL, e.g., CRDDeletionProtection.

The majority of validations however is rather complex, as it contains object decoding (example) or cross validation (example). CEL can't help here to my understanding.
My impression is that we wouldn't be able to replace a significant amount, but will rather have a mixture between OpenAPI/CEL schema validation and validating webhooks.

@voelzmo
Copy link
Member Author

voelzmo commented Feb 3, 2025

That's a fair assessment, I guess you know the current state of validations in g/g much better than I do.

I was probably a bit influenced by the repetitive work I had to do in https://github.com/gardener/gardener/pull/11215/files#diff-51d6dcfca3b6142beb72780e52d0a554bff707dc2aeebe35df9edc6b05aa297f and https://github.com/gardener/gardener/pull/11215/files#diff-7a31395cc11cbafe095cb36d5140c6653b82e4e21fbc8becefdd204965976a9c and was wondering if we really need people to do this for each and every new field they're introducing, or if we could take a simpler approach.

Feel free to close this if you think this isn't a useful thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/quality Output qualification (tests, checks, scans, automation in general, etc.) related kind/enhancement Enhancement, improvement, extension
Projects
None yet
Development

No branches or pull requests

2 participants