operator
operator copied to clipboard
istio
Initial implementation of validation webhook
Changes include:
- Setup webhook server and register the validation path with self defined handler.
- Implemented the validation logic towards CR admission request, which is using the validation logic of IstioControlPlaneSpec 3..Create configs for ValidatingWebhookConfiguration, Certificate/Issuer and update the corresponding operator service deployment
Update at 11/12: Thanks for the help of @martonsereg , we update the matching rules of webhook configuration to be plural form of IstioControlPlane, otherwise the admission request would not trigger the validation call.
@richardwxn: The following test failed, say /retest to rerun them all:
| Test name | Commit | Details | Rerun command |
|---|---|---|---|
| e2e_operator | c93af51861ff4ce579ebbd108600fa0a514a88ef | link | /test e2e_operator |
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.
@howardjohn I think the motivation here is that validation webhook can do more than openAPI schema check does(basic type, pattern, range checking). So now our custom validation logic is not doing much more than that, but it is being added more custom logic(I would add more details). Also this webhook is managed by controller-runtime manager, similar with controller, so we don't need to have extra deployment or so for it, making it simpler to maintain.
@richardwxn I am not suggesting the openAPI check can do everything a webhook can do. Of course the webhook can do whatever validation it wants so its much more flexible. It is a huge pain to manage the certs and other issues involved though, so we shouldn't take it lightly to add a webhook.
OpenAPI and webhook can coexist too. In my opinion, we the best option is to start with openAPI schema (which is still useful even if we add a webhook later) and only add a webhook if there are clearly defined benefits that openAPI does not provide
+1 to @howardjohn's suggestion to start with OpenAPI schema first and only add the webhook later if the schema isn't sufficient.
@howardjohn @ayj Yep, that makes sense. I would put this PR on hold for now and move the basic schema checking part to openAPI first.
@richardwxn: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.