Run more tests with admission webhook (validation) enabled

[programmer04]

Is there an existing issue for this?

• I have searched the existing issues

Problem Statement

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

• gateway_webhook_test.go
• httproute_webhook_test.go
• etc.

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

• Add Ingress and GWAPI v1beta1 to admission webhook #4733
• Update admission webhook config, remove Chart.lock, and release kong 2.29.1 charts#886
• chore(ingress) release 0.7.0 charts#887

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

• feat(admission): validate regex supplied in HTTPRoute (stop rejecting) #4608

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

• Automation proposal: prevent referencing closed issues in code #4676

Acceptance Criteria

• All tests that expect to have valid configuration should go through webhook
• Invalid configs should be tested both with webhook enabled (config is rejected and not applied) and disabled (config is applied, KIC handles it)

[pmalek]

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]

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: #4763

[pmalek]

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]

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]

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

kubernetes-ingress-controller/test/integration/webhook_test.go

Line 603 in 9a38f98

func ensureAdmissionRegistration(ctx context.Context, t *testing.T, namespace, configResourceName string, rules []admregv1.RuleWithOperations) {

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

• kubernetes-ingress-controller/test/integration/gateway_webhook_test.go

  Lines 32 to 47 in 9a38f98

  	ensureAdmissionRegistration(
  	ctx,
  	t,
  	ns.Name,
  	configResourceName,
  	[]admregv1.RuleWithOperations{
  	{
  	Rule: admregv1.Rule{
  	APIGroups: []string{"gateway.networking.k8s.io"},
  	APIVersions: []string{"v1beta1"},
  	Resources: []string{"gateways"},
  	},
  	Operations: []admregv1.OperationType{admregv1.Create, admregv1.Update},
  	},
  	},
  	)

• kubernetes-ingress-controller/test/integration/httproute_webhook_test.go

  Lines 216 to 231 in 9a38f98

  	ensureAdmissionRegistration(
  	ctx,
  	t,
  	namespace,
  	webhookName,
  	[]admregv1.RuleWithOperations{
  	{
  	Rule: admregv1.Rule{
  	APIGroups: []string{"gateway.networking.k8s.io"},
  	APIVersions: []string{"v1beta1"},
  	Resources: []string{"httproutes"},
  	},
  	Operations: []admregv1.OperationType{admregv1.Create, admregv1.Update},
  	},
  	},
  	)

I think we can have several options:

• Add it as an addon to KTF.

  • I think there will be relatively few points of modification.

• Explore the method mentioned by @czeslavo for webhook install options

[pangruoran]

Another option:

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

• Switch e2e tests to use Helm chart instead of all-in-one manifests #4973

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