From b1f6a3c2ff84a7021753b29b212b0e0e713012b4 Mon Sep 17 00:00:00 2001
From: Jay <jay@cockroachlabs.com>
Date: Mon, 26 Jun 2023 22:59:53 +0000
Subject: [PATCH] ccl/sqlproxyccl: fix possible flake in TestProxyProtocol

Fixes #105585.

This commit updates the TestProxyProtocol test to only test the case where
RequireProxyProtocol=true. There's no point testing the case where the
RequireProxyProtocol field is false since every other tests do not use the
proxy protocol (and that case is implicitly covered by them).

It's unclear what is causing this test flake (and it is extremely rare, i.e.
1 legit failure out of 1000 runs [1]). It may be due to some sort of race
within the tests, but given that the case is covered by all other tests, this
commit opts to remove the test entirely.

[1] https://teamcity.cockroachdb.com/test/-1121006080109385641?currentProjectId=Cockroach_Ci_TestsGcpLinuxX8664BigVm&expandTestHistoryChartSection=true

Release note: None

Release justification: Fixes a test flake.

Epic: none
---
 pkg/ccl/sqlproxyccl/proxy_handler_test.go | 73 +++++++----------------
 1 file changed, 23 insertions(+), 50 deletions(-)

diff --git a/pkg/ccl/sqlproxyccl/proxy_handler_test.go b/pkg/ccl/sqlproxyccl/proxy_handler_test.go
index e68447f32f4a..4ee04b3b2a79 100644
--- a/pkg/ccl/sqlproxyccl/proxy_handler_test.go
+++ b/pkg/ccl/sqlproxyccl/proxy_handler_test.go
@@ -212,61 +212,34 @@ func TestProxyProtocol(t *testing.T) {
 		}
 	}
 
-	t.Run("allow=true", func(t *testing.T) {
-		s, sqlAddr, httpAddr := withProxyProtocol(true)
+	s, sqlAddr, httpAddr := withProxyProtocol(true)
 
-		defer testutils.TestingHook(&validateFn, func(h *proxyproto.Header) error {
-			if h.SourceAddr.String() != "10.20.30.40:4242" {
-				return errors.Newf("got source addr %s, expected 10.20.30.40:4242", h.SourceAddr)
-			}
-			return nil
-		})()
-
-		// Test SQL. Only request with PROXY should go through.
-		url := fmt.Sprintf("postgres://bob:builder@%s/tenant-cluster-42.defaultdb?sslmode=require", sqlAddr)
-		te.TestConnectWithPGConfig(
-			ctx, t, url,
-			func(c *pgx.ConnConfig) {
-				c.DialFunc = proxyDialer
-			},
-			func(conn *pgx.Conn) {
-				require.Equal(t, int64(1), s.metrics.CurConnCount.Value())
-				require.NoError(t, runTestQuery(ctx, conn))
-			},
-		)
-		_ = te.TestConnectErr(ctx, t, url, codeClientReadFailed, "tls error")
-
-		// Test HTTP. Should support with or without PROXY.
-		client := http.Client{Timeout: timeout}
-		makeHttpReq(t, &client, httpAddr, true)
-		proxyClient := http.Client{Transport: &http.Transport{DialContext: proxyDialer}}
-		makeHttpReq(t, &proxyClient, httpAddr, true)
-	})
-
-	t.Run("allow=false", func(t *testing.T) {
-		s, sqlAddr, httpAddr := withProxyProtocol(false)
+	defer testutils.TestingHook(&validateFn, func(h *proxyproto.Header) error {
+		if h.SourceAddr.String() != "10.20.30.40:4242" {
+			return errors.Newf("got source addr %s, expected 10.20.30.40:4242", h.SourceAddr)
+		}
+		return nil
+	})()
 
-		// Test SQL. Only request without PROXY should go through.
-		url := fmt.Sprintf("postgres://bob:builder@%s/tenant-cluster-42.defaultdb?sslmode=require", sqlAddr)
-		te.TestConnect(ctx, t, url, func(conn *pgx.Conn) {
+	// Test SQL. Only request with PROXY should go through.
+	url := fmt.Sprintf("postgres://bob:builder@%s/tenant-cluster-42.defaultdb?sslmode=require", sqlAddr)
+	te.TestConnectWithPGConfig(
+		ctx, t, url,
+		func(c *pgx.ConnConfig) {
+			c.DialFunc = proxyDialer
+		},
+		func(conn *pgx.Conn) {
 			require.Equal(t, int64(1), s.metrics.CurConnCount.Value())
 			require.NoError(t, runTestQuery(ctx, conn))
-		})
-		_ = te.TestConnectErrWithPGConfig(
-			ctx, t, url,
-			func(c *pgx.ConnConfig) {
-				c.DialFunc = proxyDialer
-			},
-			codeClientReadFailed,
-			"tls error",
-		)
+		},
+	)
+	_ = te.TestConnectErr(ctx, t, url, codeClientReadFailed, "tls error")
 
-		// Test HTTP.
-		client := http.Client{Timeout: timeout}
-		makeHttpReq(t, &client, httpAddr, true)
-		proxyClient := http.Client{Transport: &http.Transport{DialContext: proxyDialer}}
-		makeHttpReq(t, &proxyClient, httpAddr, false)
-	})
+	// Test HTTP. Should support with or without PROXY.
+	client := http.Client{Timeout: timeout}
+	makeHttpReq(t, &client, httpAddr, true)
+	proxyClient := http.Client{Transport: &http.Transport{DialContext: proxyDialer}}
+	makeHttpReq(t, &proxyClient, httpAddr, true)
 }
 
 func TestPrivateEndpointsACL(t *testing.T) {