Skip to content

Commit

Permalink
ccl/sqlproxyccl: fix possible flake in TestProxyProtocol
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jaylim-crl committed Jun 26, 2023
1 parent 2279c89 commit b1f6a3c
Showing 1 changed file with 23 additions and 50 deletions.
73 changes: 23 additions & 50 deletions pkg/ccl/sqlproxyccl/proxy_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit b1f6a3c

Please sign in to comment.