fix: isolate test HTTP clients (#25038)

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

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)
+}