search

Kong / kubernetes-ingress-controller

:gorilla: Kong for Kubernetes: The official Ingress Controller for Kubernetes.
https://docs.konghq.com/kubernetes-ingress-controller/
Apache License 2.0
2.16k stars 592 forks source link

Run more tests with admission webhook (validation) enabled #4680

Open programmer04 opened 9 months ago

programmer04 commented 9 months ago

Is there an existing issue for this?

Problem Statement

Currently, the admission webhook is tested in the integration suite in isolation in files:

But tests that apply actual configurations (e.g. routing traffic based on Ingress configuration, etc.) and test their correctness don't have webhook enabled. Every valid configuration should not be blocked by webhook too. On the other hand, due to the nature of K8s, the admission webhook may be not configured/removed thus KIC should cope with invalid configurations too (the webhook should be able to reject as many as possible). It has to be tested.

Furthermore, validation webhook needs to be configured in a cluster for particular objects with validatingwebhookconfigurations.admissionregistration.k8s K8s object that is defined in custom rarely used script in repo and Helm chart. It can be easily overlooked, e.g. it happened for HTTPRoute and Ingress, fixed in the below PRs

Proposed Solution

Treat admission webhook enabled as the default configuration for KIC, and run as much as possible test with it enabled.

Additional information

It's been discovered during the work on

that implemented (and covered with tests) features used to be rejected by admission webhook as not implemented yet. Another complementary issue has been created too

Acceptance Criteria

pmalek commented 8 months ago

I believe we could actually investigate the possibility of running webhook tests with envtest (and perhaps Kong Gateway started via testcontainers).

With the introduction of Gateway Discovery tests like admission webhook integration tests might become more and more complicated to setup and debug.

czeslavo commented 8 months ago

I believe we could actually investigate the possibility of running webhook tests with envtest (and perhaps Kong Gateway started via testcontainers).

With the introduction of Gateway Discovery tests like admission webhook integration tests might become more and more complicated to setup and debug.

+1 I had an attempt to adapt our admission server boilerplate to controller-runtime so it's run as Manager's runnable, not separately as it is now. It would allow running it in envtests more easily. Leaving the WIP PR for posterity: https://github.com/Kong/kubernetes-ingress-controller/pull/4763

pmalek commented 8 months ago

I had an attempt to adapt our admission server boilerplate to controller-runtime so it's run as Manager's runnable, not separately as it is now.

What would be the difference in tests? Convenience how it's started?

czeslavo commented 8 months ago

What would be the difference in tests? Convenience how it's started?

Yes, I believe it should allow using envtest webhook install options with less friction. Here's an example of such a test in controller-runtime repo: https://github.com/alenkacz/controller-runtime/blob/528cd19ee0de5d4732234566f756ef75f8c5ce77/pkg/envtest/webhook_test.go

pangruoran commented 8 months ago

Currently, we use the ensureAdmissionRegistration function to registe the Admission webhook.

https://github.com/Kong/kubernetes-ingress-controller/blob/9a38f984baed428847e0e7f277ff6eb2a5bcfbb7/test/integration/webhook_test.go#L603

And call it, when we want to test with the Admission webhook.

I think we can have several options:

pangruoran commented 8 months ago

Another option:

We can deploy the resources we need through Helm, which can also simplify many steps.

I will temporarily leave this issue, I think #4973 will be a good choice.