From 4a6756a3e8b15e042fe162cfcd84784e5009193a Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Mon, 11 May 2026 11:03:38 +0200 Subject: [PATCH] fix: isolate test HTTP clients (#25038) --- cli/agent_test.go | 2 +- cli/root_test.go | 2 +- cli/templateedit_test.go | 8 +-- coderd/coderd_test.go | 2 +- coderd/coderdtest/coderdtest.go | 51 +++++++++++++++-- coderd/coderdtest/httpclient_test.go | 86 ++++++++++++++++++++++++++++ coderd/workspaces_scoped_test.go | 6 +- enterprise/cli/boundary_test.go | 8 +-- 8 files changed, 148 insertions(+), 17 deletions(-) create mode 100644 coderd/coderdtest/httpclient_test.go diff --git a/cli/agent_test.go b/cli/agent_test.go index fb073ff571..89976a03d1 100644 --- a/cli/agent_test.go +++ b/cli/agent_test.go @@ -111,7 +111,7 @@ func TestWorkspaceAgent(t *testing.T) { t.Cleanup(func() { _ = provisionerCloser.Close() }) - client := codersdk.New(serverURL) + client := codersdk.New(serverURL, codersdk.WithHTTPClient(coderdtest.NewIsolatedHTTPClient(serverURL))) t.Cleanup(func() { cancelFunc() _ = provisionerCloser.Close() diff --git a/cli/root_test.go b/cli/root_test.go index 3aab248dec..aac161eb6a 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -217,7 +217,7 @@ func TestDERPHeaders(t *testing.T) { t.Cleanup(func() { _ = provisionerCloser.Close() }) - client := codersdk.New(serverURL) + client := codersdk.New(serverURL, codersdk.WithHTTPClient(coderdtest.NewIsolatedHTTPClient(serverURL))) t.Cleanup(func() { cancelFunc() _ = provisionerCloser.Close() diff --git a/cli/templateedit_test.go b/cli/templateedit_test.go index bc9a53758d..cf5eb57a3d 100644 --- a/cli/templateedit_test.go +++ b/cli/templateedit_test.go @@ -384,7 +384,7 @@ func TestTemplateEdit(t *testing.T) { // Create a new client that uses the proxy server. proxyURL, err := url.Parse(proxy.URL) require.NoError(t, err) - proxyClient := codersdk.New(proxyURL) + proxyClient := codersdk.New(proxyURL, codersdk.WithHTTPClient(coderdtest.NewIsolatedHTTPClient(proxyURL))) proxyClient.SetSessionToken(templateAdmin.SessionToken()) t.Cleanup(proxyClient.HTTPClient.CloseIdleConnections) @@ -515,7 +515,7 @@ func TestTemplateEdit(t *testing.T) { // Create a new client that uses the proxy server. proxyURL, err := url.Parse(proxy.URL) require.NoError(t, err) - proxyClient := codersdk.New(proxyURL) + proxyClient := codersdk.New(proxyURL, codersdk.WithHTTPClient(coderdtest.NewIsolatedHTTPClient(proxyURL))) proxyClient.SetSessionToken(templateAdmin.SessionToken()) t.Cleanup(proxyClient.HTTPClient.CloseIdleConnections) @@ -659,7 +659,7 @@ func TestTemplateEdit(t *testing.T) { // Create a new client that uses the proxy server. proxyURL, err := url.Parse(proxy.URL) require.NoError(t, err) - proxyClient := codersdk.New(proxyURL) + proxyClient := codersdk.New(proxyURL, codersdk.WithHTTPClient(coderdtest.NewIsolatedHTTPClient(proxyURL))) proxyClient.SetSessionToken(templateAdmin.SessionToken()) t.Cleanup(proxyClient.HTTPClient.CloseIdleConnections) @@ -771,7 +771,7 @@ func TestTemplateEdit(t *testing.T) { // Create a new client that uses the proxy server. proxyURL, err := url.Parse(proxy.URL) require.NoError(t, err) - proxyClient := codersdk.New(proxyURL) + proxyClient := codersdk.New(proxyURL, codersdk.WithHTTPClient(coderdtest.NewIsolatedHTTPClient(proxyURL))) proxyClient.SetSessionToken(templateAdmin.SessionToken()) t.Cleanup(proxyClient.HTTPClient.CloseIdleConnections) diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index 0ffbe695b4..813c596fcb 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -184,7 +184,7 @@ func TestDERPForceWebSockets(t *testing.T) { _ = provisionerCloser.Close() }) - client := codersdk.New(serverURL) + client := codersdk.New(serverURL, codersdk.WithHTTPClient(coderdtest.NewIsolatedHTTPClient(serverURL))) t.Cleanup(func() { client.HTTPClient.CloseIdleConnections() }) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index cbbee1b1c2..bb517dbdc3 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -669,7 +669,7 @@ func NewWithAPI(t testing.TB, options *Options) (*codersdk.Client, io.Closer, *c if options.IncludeProvisionerDaemon { provisionerCloser = NewTaggedProvisionerDaemon(t, coderAPI, defaultTestDaemonName, options.ProvisionerDaemonTags, coderd.MemoryProvisionerWithVersionOverride(options.ProvisionerDaemonVersion)) } - client := codersdk.New(serverURL) + client := codersdk.New(serverURL, codersdk.WithHTTPClient(NewIsolatedHTTPClient(serverURL))) t.Cleanup(func() { cancelFunc() _ = provisionerCloser.Close() @@ -679,6 +679,46 @@ func NewWithAPI(t testing.TB, options *Options) (*codersdk.Client, io.Closer, *c return client, provisionerCloser, coderAPI } +// NewIsolatedHTTPClient returns a test client with its own transport. +// Closing idle connections at test cleanup must not close http.DefaultTransport +// while another parallel test is using it. +func NewIsolatedHTTPClient(serverURL *url.URL) *http.Client { + transport := &http.Transport{Proxy: http.ProxyFromEnvironment} + if defaultTransport, ok := http.DefaultTransport.(*http.Transport); ok { + transport = defaultTransport.Clone() + } + if serverURL == nil || serverURL.Scheme != "https" { + transport.TLSClientConfig = nil + return &http.Client{Transport: transport} + } + if transport.TLSClientConfig == nil { + transport.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12} + } + if transport.TLSClientConfig.MinVersion == 0 { + transport.TLSClientConfig.MinVersion = tls.VersionTLS12 + } + //nolint:gosec // The coderdtest server uses test-only TLS certificates. + transport.TLSClientConfig.InsecureSkipVerify = true + return &http.Client{Transport: transport} +} + +// newHTTPClientWithTransportFrom returns a fresh client that shares the base +// transport without sharing mutable per-client state like CheckRedirect. +func newHTTPClientWithTransportFrom(base *http.Client) *http.Client { + if base == nil { + return NewIsolatedHTTPClient(nil) + } + if base.Transport == nil { + client := NewIsolatedHTTPClient(nil) + client.Timeout = base.Timeout + return client + } + return &http.Client{ + Transport: base.Transport, + Timeout: base.Timeout, + } +} + // ProvisionerdCloser wraps a provisioner daemon as an io.Closer that can be called multiple times type ProvisionerdCloser struct { mu sync.Mutex @@ -937,10 +977,11 @@ func createAnotherUserRetry(t testing.TB, client *codersdk.Client, organizationI require.NoError(t, err) } - other := codersdk.New(client.URL, codersdk.WithSessionToken(sessionToken)) - t.Cleanup(func() { - other.HTTPClient.CloseIdleConnections() - }) + other := codersdk.New( + client.URL, + codersdk.WithSessionToken(sessionToken), + codersdk.WithHTTPClient(newHTTPClientWithTransportFrom(client.HTTPClient)), + ) if len(roles) > 0 { // Find the roles for the org vs the site wide roles diff --git a/coderd/coderdtest/httpclient_test.go b/coderd/coderdtest/httpclient_test.go new file mode 100644 index 0000000000..600c1c1582 --- /dev/null +++ b/coderd/coderdtest/httpclient_test.go @@ -0,0 +1,86 @@ +package coderdtest_test + +import ( + "crypto/tls" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/testutil" +) + +func TestNewIsolatedHTTPClient(t *testing.T) { + t.Parallel() + + client := coderdtest.NewIsolatedHTTPClient(testutil.MustURL(t, "http://example.com")) + require.NotNil(t, client.Transport) + require.NotSame(t, http.DefaultTransport, client.Transport) + + transport, ok := client.Transport.(*http.Transport) + require.True(t, ok) + require.Nil(t, transport.TLSClientConfig) +} + +func TestNewIsolatedHTTPSClient(t *testing.T) { + t.Parallel() + + client := coderdtest.NewIsolatedHTTPClient(testutil.MustURL(t, "https://example.com")) + require.NotSame(t, http.DefaultTransport, client.Transport) + + transport, ok := client.Transport.(*http.Transport) + require.True(t, ok) + require.NotNil(t, transport.TLSClientConfig) + require.True(t, transport.TLSClientConfig.InsecureSkipVerify) + require.Equal(t, uint16(tls.VersionTLS12), transport.TLSClientConfig.MinVersion) +} + +func TestNewIsolatedHTTPClientNilURL(t *testing.T) { + t.Parallel() + + client := coderdtest.NewIsolatedHTTPClient(nil) + require.NotNil(t, client.Transport) + require.NotSame(t, http.DefaultTransport, client.Transport) + + transport, ok := client.Transport.(*http.Transport) + require.True(t, ok) + require.Nil(t, transport.TLSClientConfig) +} + +func TestCreateAnotherUserHTTPClient(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, nil) + first := coderdtest.CreateFirstUser(t, client) + client.HTTPClient.CheckRedirect = func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + } + + other, _ := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + + require.NotSame(t, client.HTTPClient, other.HTTPClient) + require.Same(t, client.HTTPClient.Transport, other.HTTPClient.Transport) + require.Nil(t, other.HTTPClient.CheckRedirect) +} + +func TestCreateAnotherUserHTTPClientDefaultTransport(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, nil) + first := coderdtest.CreateFirstUser(t, client) + base := codersdk.New( + client.URL, + codersdk.WithSessionToken(client.SessionToken()), + codersdk.WithHTTPClient(&http.Client{Timeout: time.Second}), + ) + + other, _ := coderdtest.CreateAnotherUser(t, base, first.OrganizationID) + + require.NotSame(t, base.HTTPClient, other.HTTPClient) + require.NotNil(t, other.HTTPClient.Transport) + require.NotSame(t, http.DefaultTransport, other.HTTPClient.Transport) + require.Equal(t, base.HTTPClient.Timeout, other.HTTPClient.Timeout) +} diff --git a/coderd/workspaces_scoped_test.go b/coderd/workspaces_scoped_test.go index 016487306d..0e9f34dc00 100644 --- a/coderd/workspaces_scoped_test.go +++ b/coderd/workspaces_scoped_test.go @@ -63,7 +63,11 @@ func TestCompositeWorkspaceScopes(t *testing.T) { }) require.NoError(t, err, "creating scoped token") - scoped := codersdk.New(adminClient.URL, codersdk.WithSessionToken(resp.Key)) + scoped := codersdk.New( + adminClient.URL, + codersdk.WithSessionToken(resp.Key), + codersdk.WithHTTPClient(coderdtest.NewIsolatedHTTPClient(adminClient.URL)), + ) t.Cleanup(func() { scoped.HTTPClient.CloseIdleConnections() }) return scoped } diff --git a/enterprise/cli/boundary_test.go b/enterprise/cli/boundary_test.go index 25cb9074c7..2457f4ca63 100644 --- a/enterprise/cli/boundary_test.go +++ b/enterprise/cli/boundary_test.go @@ -118,7 +118,7 @@ func TestBoundaryLicenseVerification(t *testing.T) { proxyURL, err := url.Parse(proxy.URL) require.NoError(t, err) - proxyClient := codersdk.New(proxyURL) + proxyClient := codersdk.New(proxyURL, codersdk.WithHTTPClient(coderdtest.NewIsolatedHTTPClient(proxyURL))) proxyClient.SetSessionToken(client.SessionToken()) t.Cleanup(proxyClient.HTTPClient.CloseIdleConnections) @@ -182,7 +182,7 @@ func TestBoundaryLicenseVerification(t *testing.T) { proxyURL, err := url.Parse(proxy.URL) require.NoError(t, err) - proxyClient := codersdk.New(proxyURL) + proxyClient := codersdk.New(proxyURL, codersdk.WithHTTPClient(coderdtest.NewIsolatedHTTPClient(proxyURL))) proxyClient.SetSessionToken(client.SessionToken()) t.Cleanup(proxyClient.HTTPClient.CloseIdleConnections) @@ -219,7 +219,7 @@ func TestBoundaryLicenseVerification(t *testing.T) { proxyURL, err := url.Parse(proxy.URL) require.NoError(t, err) - proxyClient := codersdk.New(proxyURL) + proxyClient := codersdk.New(proxyURL, codersdk.WithHTTPClient(coderdtest.NewIsolatedHTTPClient(proxyURL))) proxyClient.SetSessionToken(client.SessionToken()) t.Cleanup(proxyClient.HTTPClient.CloseIdleConnections) @@ -286,7 +286,7 @@ func TestBoundaryChildProcessSkipsCheck(t *testing.T) { proxyURL, err := url.Parse(proxy.URL) require.NoError(t, err) - proxyClient := codersdk.New(proxyURL) + proxyClient := codersdk.New(proxyURL, codersdk.WithHTTPClient(coderdtest.NewIsolatedHTTPClient(proxyURL))) proxyClient.SetSessionToken(client.SessionToken()) t.Cleanup(proxyClient.HTTPClient.CloseIdleConnections)