chore(tests): do not use InsecureSkipVerify for HTTPS tests (#6307)

.golangci.yaml

@@ -171,7 +171,7 @@ issues:
   - path: _test\.go
     linters:
       - gosec
-    text: "TLS InsecureSkipVerify set true|Potential hardcoded credentials"
+    text: "Potential hardcoded credentials"
   # Ignore prealloc in tests
   - path: _test\.go
     linters:

go.mod

@@ -128,7 +128,7 @@ require (
 	github.com/googleapis/gax-go/v2 v2.12.5 // indirect
 	github.com/gorilla/websocket v1.5.1 // indirect
 	github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect
-	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
+	github.com/hashicorp/go-cleanhttp v0.5.2
 	github.com/hashicorp/go-immutable-radix v1.3.1 // indirect
 	github.com/hashicorp/go-memdb v1.3.4 // indirect
 	github.com/hashicorp/go-retryablehttp v0.7.7

test/e2e/all_in_one_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/go-cleanhttp"
 	"github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/kong"
 	"github.com/kong/kubernetes-testing-framework/pkg/environments"
 	"github.com/stretchr/testify/require"
@@ -334,20 +335,21 @@ func ensureAllProxyReplicasAreConfigured(ctx context.Context, t *testing.T, env
 	require.NoError(t, err)
 
 	t.Logf("ensuring all %d proxy replicas are configured", len(pods))
+	client := cleanhttp.DefaultClient()
+	tr := cleanhttp.DefaultTransport()
+	// Anything related to TLS can be ignored, because only availability is being tested here.
+	// Testing communicating over TLS is done as part of actual E2E test.
+	tr.TLSClientConfig = &tls.Config{
+		InsecureSkipVerify: true, //nolint:gosec
+	}
+	client.Transport = tr
+
 	wg := sync.WaitGroup{}
 	for _, pod := range pods {
 		pod := pod
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
-			client := &http.Client{
-				Timeout: time.Second * 30,
-				Transport: &http.Transport{
-					TLSClientConfig: &tls.Config{
-						InsecureSkipVerify: true,
-					},
-				},
-			}
 
 			forwardCtx, cancel := context.WithCancel(ctx)
 			defer cancel()

test/e2e/features_test.go

@@ -6,6 +6,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/tls"
+	"crypto/x509"
 	"fmt"
 	"net/http"
 	"strings"
@@ -19,6 +20,7 @@ import (
 	"github.com/kong/kubernetes-testing-framework/pkg/clusters/types/kind"
 	"github.com/kong/kubernetes-testing-framework/pkg/environments"
 	"github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -118,10 +120,13 @@ func TestWebhookUpdate(t *testing.T) {
 	t.Log("deploying kong components")
 	ManifestDeploy{Path: dblessPath}.Run(ctx, t, env)
 
-	const firstCertificateCommonName = "first.example"
+	certPool := x509.NewCertPool()
+	const firstCertificateHostName = "first.example"
 	firstCertificateCrt, firstCertificateKey := certificate.MustGenerateSelfSignedCertPEMFormat(
-		certificate.WithCommonName(firstCertificateCommonName),
+		certificate.WithCommonName(firstCertificateHostName),
+		certificate.WithDNSNames(firstCertificateHostName),
 	)
+	require.True(t, certPool.AppendCertsFromPEM(firstCertificateCrt))
 	firstCertificate := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "admission-cert",
@@ -133,10 +138,12 @@ func TestWebhookUpdate(t *testing.T) {
 		},
 	}
 
-	const secondCertificateCommonName = "second.example"
+	const secondCertificateHostName = "second.example"
 	secondCertificateCrt, secondCertificateKey := certificate.MustGenerateSelfSignedCertPEMFormat(
-		certificate.WithCommonName(secondCertificateCommonName),
+		certificate.WithCommonName(secondCertificateHostName),
+		certificate.WithDNSNames(secondCertificateHostName),
 	)
+	require.True(t, certPool.AppendCertsFromPEM(secondCertificateCrt))
 	secondCertificate := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "admission-cert",
@@ -201,35 +208,26 @@ func TestWebhookUpdate(t *testing.T) {
 		deployment, metav1.UpdateOptions{})
 	require.NoError(t, err)
 
+	checkCertificate := func(hostname string) {
+		require.EventuallyWithT(t, func(c *assert.CollectT) {
+			_, err := tls.Dial("tcp", admissionAddress+":443", &tls.Config{
+				MinVersion: tls.VersionTLS12,
+				RootCAs:    certPool,
+				ServerName: hostname,
+			})
+			assert.NoError(c, err)
+		}, 1*time.Minute, time.Second)
+	}
+
 	t.Log("checking initial certificate")
-	require.Eventually(t, func() bool {
-		conn, err := tls.Dial("tcp", admissionAddress+":443",
-			&tls.Config{MinVersion: tls.VersionTLS12, InsecureSkipVerify: true})
-		if err != nil {
-			t.Logf("failed to dial %s:443, error %v", admissionAddress, err)
-			return false
-		}
-		certCommonName := conn.ConnectionState().PeerCertificates[0].Subject.CommonName
-		t.Logf("subject common name of certificate: %s", certCommonName)
-		return certCommonName == firstCertificateCommonName
-	}, time.Minute*2, time.Second)
+	checkCertificate(firstCertificateHostName)
 
 	t.Log("changing certificate")
 	_, err = env.Cluster().Client().CoreV1().Secrets(kongNamespace).Update(ctx, secondCertificate, metav1.UpdateOptions{})
 	require.NoError(t, err)
 
 	t.Log("checking second certificate")
-	require.Eventually(t, func() bool {
-		conn, err := tls.Dial("tcp", admissionAddress+":443",
-			&tls.Config{MinVersion: tls.VersionTLS12, InsecureSkipVerify: true})
-		if err != nil {
-			t.Logf("failed to dial %s:443, error %v", admissionAddress, err)
-			return false
-		}
-		certCommonName := conn.ConnectionState().PeerCertificates[0].Subject.CommonName
-		t.Logf("subject common name of certificate: %s", certCommonName)
-		return certCommonName == secondCertificateCommonName
-	}, time.Minute*10, time.Second)
+	checkCertificate(secondCertificateHostName)
 }
 
 // TestDeployAllInOneDBLESSGateway tests the Gateway feature flag and the admission controller with no user-provided

test/e2e/konnect_test.go

@@ -7,12 +7,12 @@ import (
 	"context"
 	"crypto/tls"
 	"fmt"
-	"net/http"
 	"os/exec"
 	"sync"
 	"testing"
 	"time"
 
+	"github.com/hashicorp/go-cleanhttp"
 	environment "github.com/kong/kubernetes-testing-framework/pkg/environments"
 	"github.com/samber/lo"
 	"github.com/stretchr/testify/assert"
@@ -299,14 +299,14 @@ func requireAllProxyReplicasIDsConsistentWithKonnect(
 	nodeAPIClient := createKonnectNodeClient(t, rg, cert, key)
 
 	getNodeIDFromAdminAPI := func(proxyPod corev1.Pod) string {
-		client := &http.Client{
-			Timeout: time.Second * 30,
-			Transport: &http.Transport{
-				TLSClientConfig: &tls.Config{
-					InsecureSkipVerify: true,
-				},
-			},
+		client := cleanhttp.DefaultClient()
+		tr := cleanhttp.DefaultTransport()
+		// Anything related to TLS can be ignored, because only availability is being tested here.
+		// Testing communicating over TLS is done as part of actual E2E test.
+		tr.TLSClientConfig = &tls.Config{
+			InsecureSkipVerify: true, //nolint:gosec
 		}
+		client.Transport = tr
 
 		forwardCtx, cancel := context.WithCancel(ctx)
 		defer cancel()

test/helpers/ports_test.go

@@ -8,6 +8,7 @@ import (
 	"strconv"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -46,10 +47,11 @@ func TestGetFreePort(t *testing.T) {
 					}
 					s := httptest.Server{
 						Listener: l,
-						Config: &http.Server{ //nolint:gosec
+						Config: &http.Server{
 							Addr: fmt.Sprintf("localhost:%d", p),
 							Handler: http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
 							}),
+							ReadHeaderTimeout: 10 * time.Second,
 						},
 					}
 					s.Start()

test/integration/consumer_group_test.go

@@ -149,7 +149,7 @@ func TestConsumerGroup(t *testing.T) {
 			req := helpers.MustHTTPRequest(t, http.MethodGet, proxyHTTPURL.Host, path, map[string]string{
 				"apikey": consumer.Name,
 			})
-			resp, err := helpers.DefaultHTTPClientWithProxy(proxyHTTPURL).Do(req)
+			resp, err := helpers.DefaultHTTPClient().Do(req)
 			if err != nil {
 				t.Logf("WARNING: consumer %q failed to make a request: %v", consumer.Name, err)
 				return false
@@ -194,7 +194,7 @@ func TestConsumerGroup(t *testing.T) {
 		req := helpers.MustHTTPRequest(t, http.MethodGet, proxyHTTPURL.Host, multiPath, map[string]string{
 			"apikey": four.Name,
 		})
-		resp, err := helpers.DefaultHTTPClientWithProxy(proxyHTTPURL).Do(req)
+		resp, err := helpers.DefaultHTTPClient().Do(req)
 		if !assert.NoError(c, err) {
 			return
 		}
@@ -211,7 +211,7 @@ func TestConsumerGroup(t *testing.T) {
 		clear := helpers.MustHTTPRequest(t, http.MethodGet, proxyHTTPURL.Host, path, map[string]string{
 			"apikey": four.Name,
 		})
-		clearResp, err := helpers.DefaultHTTPClientWithProxy(proxyHTTPURL).Do(clear)
+		clearResp, err := helpers.DefaultHTTPClient(helpers.WithResolveHostTo(proxyHTTPURL.Host)).Do(clear)
 		if !assert.NoError(c, err) {
 			return
 		}
@@ -228,7 +228,7 @@ func TestConsumerGroup(t *testing.T) {
 		empty := helpers.MustHTTPRequest(t, http.MethodGet, proxyHTTPURL.Host, multiPath, map[string]string{
 			"apikey": "test-consumer-3",
 		})
-		emptyResp, err := helpers.DefaultHTTPClientWithProxy(proxyHTTPURL).Do(empty)
+		emptyResp, err := helpers.DefaultHTTPClient(helpers.WithResolveHostTo(proxyHTTPURL.Host)).Do(empty)
 		if !assert.NoError(c, err) {
 			return
 		}

test/integration/consumer_test.go

@@ -145,7 +145,7 @@ func TestConsumerCredential(t *testing.T) {
 	assert.Eventually(t, func() bool {
 		req := helpers.MustHTTPRequest(t, http.MethodGet, proxyHTTPURL.Host, "/test_consumer_credential", nil)
 		req.SetBasicAuth("test_consumer_credential", "test_consumer_credential")
-		resp, err := helpers.DefaultHTTPClientWithProxy(proxyHTTPURL).Do(req)
+		resp, err := helpers.DefaultHTTPClient(helpers.WithResolveHostTo(proxyHTTPURL.Host)).Do(req)
 		if err != nil {
 			return false
 		}

test/integration/gateway_test.go

@@ -427,9 +427,9 @@ func TestGatewayFilters(t *testing.T) {
 	require.Eventually(t, callback, ingressWait, waitTick)
 
 	t.Log("waiting for routes from HTTPRoute to become operational")
-	helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "test_gateway_filters", http.StatusOK, "<title>httpbin.org</title>", emptyHeaderSet, ingressWait, waitTick)
+	helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "test_gateway_filters", nil, http.StatusOK, "<title>httpbin.org</title>", emptyHeaderSet, ingressWait, waitTick)
 	t.Log("waiting for routes from HTTPRoute in other namespace to become operational")
-	helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "other_test_gateway_filters", http.StatusOK, "<title>httpbin.org</title>", emptyHeaderSet, ingressWait, waitTick)
+	helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "other_test_gateway_filters", nil, http.StatusOK, "<title>httpbin.org</title>", emptyHeaderSet, ingressWait, waitTick)
 
 	t.Log("changing to the same namespace filter")
 	require.Eventually(t, func() bool {
@@ -452,9 +452,9 @@ func TestGatewayFilters(t *testing.T) {
 	}, ingressWait, waitTick)
 
 	t.Log("confirming other namespace route becomes inaccessible")
-	helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "other_test_gateway_filters", http.StatusNotFound, "no Route matched", emptyHeaderSet, ingressWait, waitTick)
+	helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "other_test_gateway_filters", nil, http.StatusNotFound, "no Route matched", emptyHeaderSet, ingressWait, waitTick)
 	t.Log("confirming same namespace route still operational")
-	helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "test_gateway_filters", http.StatusOK, "<title>httpbin.org</title>", emptyHeaderSet, ingressWait, waitTick)
+	helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "test_gateway_filters", nil, http.StatusOK, "<title>httpbin.org</title>", emptyHeaderSet, ingressWait, waitTick)
 
 	t.Log("changing to a selector filter")
 	require.Eventually(t, func() bool {
@@ -483,7 +483,7 @@ func TestGatewayFilters(t *testing.T) {
 	}, ingressWait, waitTick)
 
 	t.Log("confirming wrong selector namespace route becomes inaccessible")
-	helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "test_gateway_filters", http.StatusNotFound, "no Route matched", emptyHeaderSet, ingressWait, waitTick)
+	helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "test_gateway_filters", nil, http.StatusNotFound, "no Route matched", emptyHeaderSet, ingressWait, waitTick)
 	t.Log("confirming right selector namespace route becomes operational")
-	helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "other_test_gateway_filters", http.StatusOK, "<title>httpbin.org</title>", emptyHeaderSet, ingressWait, waitTick)
+	helpers.EventuallyGETPath(t, proxyHTTPURL, proxyHTTPURL.Host, "other_test_gateway_filters", nil, http.StatusOK, "<title>httpbin.org</title>", emptyHeaderSet, ingressWait, waitTick)
 }