TestDeployAllInOneDBLESSGateway does not verify admission webhook works

[czeslavo]

Is there an existing issue for this?

• I have searched the existing issues

Problem Statement

When working on #3378 it was discovered that the TestDeployAllInOneDBLESSGateway test doesn't really verify whether the admission webhook works correctly.

In the test, we were creating a KongConsumer in a loop, waiting for it to be created successfully (err == nil in the Eventually predicate).

	t.Log("creating a consumer to ensure the admission webhook is online")
	consumer := &kongv1.KongConsumer{
		ObjectMeta: metav1.ObjectMeta{
			Name: "nihoniy",
			Annotations: map[string]string{
				annotations.IngressClassKey: ingressClass,
			},
        }
	kongClient, err := clientset.NewForConfig(env.Cluster().Config())
	require.Eventually(t, func() bool {
		_, err = kongClient.ConfigurationV1().KongConsumers(namespace).Create(ctx, consumer, metav1.CreateOptions{})
		return err == nil
        ... 

Instead of verifying that it is created successfully, we should rather check that it's rejected (err != nil).

When I changed the condition to check that the creation eventually fails, I noticed that the webhook doesn't work as expected, giving the following error when the API server calls it:

TLS handshake error from 10.40.0.7:41890: EOF

The aim of this issue is to make sure that the check is enabled back to verify that the webhook is working correctly (possibly by fixing the self-signed certificate generated by deploy-admission-controller.sh script).

Proposed Solution

• Investigate why API server cannot establish TLS connection with the admission webhook

Additional information

After changing the condition to the correct one (err != nil), both Kind and GKE tests started to fail: https://github.com/Kong/kubernetes-ingress-controller/actions/runs/3938189668/jobs/6736610203

Acceptance Criteria

• TestDeployAllInOneDBLESSGateway E2E test verifies that KongConsumer with a duplicated username cannot be created due to being rejected by the admission webhook

[czeslavo]

@rainest could you please take a look at this to verify that I'm not mistaken that the assertion was incorrect? 🙇

[rainest]

Trying to recall why we needed this. I think we can probably get rid of the E2E test bit--looking over the history I don't recall why we would have needed to test webhook behavior when the certificate is missing.

IIRC the test is confirming that the webhook is working via consumer creation because if consumer creation fails, we can assume the webhook isn't working (the API server rejects the resource if it can't get an answer from the webhook), so successful creation is a confirmation by side-effect (consumer accepted implies the webhook is up; we can't create the consumer if it isn't, the API server would have failed the request due to webhook unavailability).

The controller probably should fail to start if it it wants to run a webhook without a cert, and integration tests the expected webhook behavior independently.

Not testing controller behavior where it expects a webhook, but doesn't have one, is something we can probably live without.

If the self-signed cert from the hack script is making GKE testing unhappy, IMO just axe it. It's just old E2E test cruft we don't really need, getting the GKE environment to be happy is more valuable than weird edge case tests.

[czeslavo]

@rainest Yeah, that makes sense. You've convinced me to close it 👍 #3403