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

Move CRD validation that's possible to be done in CEL, from webhook to CRD Validation Expressions #5062

Closed
1 task done
pmalek opened this issue Nov 2, 2023 · 4 comments · Fixed by #5119, #5137 or #5142
Closed
1 task done
Labels
area/CRD Changes in existing CRDs or introduction of new ones area/debt
Milestone

Comments

@pmalek
Copy link
Member

pmalek commented Nov 2, 2023

Problem statement

KIC seems to not only have started using https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#validation-rules which are available in Kubernetes 1.25+.

This issue tracks the migration of webhook validation to CRD Validation Expressions where possible.

Acceptance criteria

  • Webhook validations that are doable in CEL are migrated to CEL
@pangruoran
Copy link
Contributor

I want to handle the error "plugin name cannot be empty". Thanks

@randmonkey
Copy link
Contributor

Could we really remove the the checks in admission webhooks when moved the checks to CELs? The feature gate CustomResourceValidationExpressions is enabled by default in k8s 1.25 while there are still usage of 1.21. So if we remove the checks in admission webhooks, the checks will not take effect in k8s 1.24 and below.

@pmalek
Copy link
Member Author

pmalek commented Nov 16, 2023

@randmonkey As per https://docs.konghq.com/kubernetes-ingress-controller/latest/reference/version-compatibility/#general we're support k8s 1.25+ with KIC v3 so we should be good in this regard.

@pmalek pmalek closed this as completed Nov 16, 2023
@pmalek
Copy link
Member Author

pmalek commented Nov 16, 2023

I've gone through the code in internal/admission/ and I haven't found any further opportunities to replace validations there to CRD validation expressions.

If you happen to find any please, feel free to re-open this.


1 lesson from reading that code though, is that we have Gateway API related code in admission webhook which most likely should land in controller's code (in order to reconcile those objects into appropriate status conditions).

Started team discussion on this: https://kongstrong.slack.com/archives/C011RQPHDC7/p1700161029335029 which will probably result in a net new issue to remove those validations from admission webhook.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment