backend: Reuse a configured HTTP client in /externalproxy

backend/cmd/headlamp.go

@@ -158,6 +158,40 @@ const (
 	TokenCacheFileName = "headlamp-token-cache"
 )
 
+// externalProxyClient is the shared HTTP client used by the /externalproxy
+// handler. It is configured with transport-level timeouts so a slow or hung
+// upstream cannot block goroutines and exhaust file descriptors, and with a
+// larger per-host idle-connection pool than the net/http default so
+// connections can be reused under load.
+//
+// Redirects are not followed: an allowed upstream could otherwise return a
+// 30x pointing at a disallowed or internal address, which the client would
+// fetch server-side and bypass the /externalproxy allowlist. Returning
+// http.ErrUseLastResponse hands the 30x response back to the handler, which
+// forwards its status and Location header to the caller so the redirect can
+// still be followed client-side.
+//
+//nolint:gochecknoglobals // shared client reused across all proxy requests
+var externalProxyClient = &http.Client{
+	CheckRedirect: func(_ *http.Request, _ []*http.Request) error {
+		return http.ErrUseLastResponse
+	},
+	Transport: &http.Transport{
+		Proxy: http.ProxyFromEnvironment,
+		DialContext: (&net.Dialer{
+			Timeout:   30 * time.Second,
+			KeepAlive: 30 * time.Second,
+		}).DialContext,
+		ForceAttemptHTTP2:     true,
+		MaxIdleConns:          100,
+		MaxIdleConnsPerHost:   10,
+		IdleConnTimeout:       90 * time.Second,
+		TLSHandshakeTimeout:   10 * time.Second,
+		ExpectContinueTimeout: 1 * time.Second,
+		ResponseHeaderTimeout: 30 * time.Second,
+	},
+}
+
 type clientConfig struct {
 	Clusters                []Cluster `json:"clusters"`
 	IsDynamicClusterEnabled bool      `json:"isDynamicClusterEnabled"`
@@ -705,9 +739,7 @@ func createHeadlampHandler(ctx context.Context, config *HeadlampConfig) http.Han
 		w.Header().Set("Pragma", "no-cache")
 		w.Header().Set("X-Accel-Expires", "0")
 
-		client := http.Client{}
-
-		resp, err := client.Do(proxyReq) //nolint:gosec
+		resp, err := externalProxyClient.Do(proxyReq) //nolint:gosec
 		if err != nil {
 			logger.Log(logger.LevelError, nil, err, "making request")
 			http.Error(w, err.Error(), http.StatusBadGateway)
@@ -747,6 +779,12 @@ func createHeadlampHandler(ctx context.Context, config *HeadlampConfig) http.Han
 			return
 		}
 
+		// Forward Location so the caller can act on redirect (30x) responses,
+		// which are passed through unfollowed rather than chased server-side.
+		if location := resp.Header.Get("Location"); location != "" {
+			w.Header().Set("Location", location)
+		}
+
 		w.WriteHeader(resp.StatusCode)
 
 		// Stream the response with a limit to prevent memory exhaustion and decompression bombs.

backend/cmd/headlamp_test.go

@@ -36,6 +36,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -567,8 +568,22 @@ func TestProxyURLAllowedCompilesConfiguredProxyURLs(t *testing.T) {
 	assert.NotEmpty(t, config.compiledProxyURLs)
 }
 
+func newProxyTestHandler(t *testing.T, upstream string) http.Handler {
+	t.Helper()
+
+	return createHeadlampHandler(context.Background(), &HeadlampConfig{
+		HeadlampConfig: &headlampconfig.HeadlampConfig{
+			HeadlampCFG: &headlampconfig.HeadlampCFG{
+				UseInCluster:    false,
+				ProxyURLs:       []string{upstream},
+				KubeConfigStore: kubeconfig.NewContextStore(),
+			},
+			Cache: cache.New[interface{}](),
+		},
+	})
+}
+
 func TestExternalProxyForwarding(t *testing.T) {
-	// Create a new server for testing that returns a specific status and content type
 	proxyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
 		w.WriteHeader(http.StatusNotFound)
@@ -577,38 +592,186 @@ func TestExternalProxyForwarding(t *testing.T) {
 	}))
 	defer proxyServer.Close()
 
-	proxyURL, err := url.Parse(proxyServer.URL)
+	handler := newProxyTestHandler(t, proxyServer.URL)
+
+	req, err := http.NewRequestWithContext(context.Background(), "GET", "/externalproxy", nil)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	cache := cache.New[interface{}]()
-	kubeConfigStore := kubeconfig.NewContextStore()
+	req.Header.Set("proxy-to", proxyServer.URL)
 
-	handler := createHeadlampHandler(context.Background(), &HeadlampConfig{
-		HeadlampConfig: &headlampconfig.HeadlampConfig{
-			HeadlampCFG: &headlampconfig.HeadlampCFG{
-				UseInCluster:    false,
-				ProxyURLs:       []string{proxyURL.String()},
-				KubeConfigStore: kubeConfigStore,
-			},
-			Cache: cache,
-		},
-	})
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+
+	assert.Equal(t, http.StatusNotFound, rr.Code)
+	assert.Equal(t, "application/json", rr.Header().Get("Content-Type"))
+	assert.Equal(t, `{"error": "not found"}`, rr.Body.String())
+}
+
+func TestExternalProxyDoesNotFollowRedirects(t *testing.T) {
+	var secretHits int32
+
+	// A server that must never be reached server-side: it stands in for a
+	// disallowed or internal address a redirect could try to point at.
+	secret := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		atomic.AddInt32(&secretHits, 1)
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer secret.Close()
+
+	// The allowed upstream redirects to the secret server.
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, secret.URL, http.StatusFound)
+	}))
+	defer upstream.Close()
+
+	handler := newProxyTestHandler(t, upstream.URL)
 
 	req, err := http.NewRequestWithContext(context.Background(), "GET", "/externalproxy", nil)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	req.Header.Set("proxy-to", proxyURL.String())
+	req.Header.Set("proxy-to", upstream.URL)
 
 	rr := httptest.NewRecorder()
 	handler.ServeHTTP(rr, req)
 
-	assert.Equal(t, http.StatusNotFound, rr.Code)
-	assert.Equal(t, "application/json", rr.Header().Get("Content-Type"))
-	assert.Equal(t, `{"error": "not found"}`, rr.Body.String())
+	if rr.Code != http.StatusFound {
+		t.Errorf("status code = %d, want %d (the redirect must be forwarded, not followed)",
+			rr.Code, http.StatusFound)
+	}
+
+	if hits := atomic.LoadInt32(&secretHits); hits != 0 {
+		t.Errorf("redirect target was fetched %d time(s); the allowlist must not be bypassable via redirects",
+			hits)
+	}
+
+	if location := rr.Header().Get("Location"); location != secret.URL {
+		t.Errorf("Location header = %q, want %q (the redirect must stay usable client-side)",
+			location, secret.URL)
+	}
+}
+
+func TestExternalProxyResponseHeaderTimeout(t *testing.T) {
+	originalTransport := externalProxyClient.Transport
+
+	baseTransport, ok := originalTransport.(*http.Transport)
+	if !ok {
+		t.Fatalf("externalProxyClient.Transport = %T, want *http.Transport", originalTransport)
+	}
+
+	clonedTransport := baseTransport.Clone()
+	clonedTransport.ResponseHeaderTimeout = 200 * time.Millisecond
+	externalProxyClient.Transport = clonedTransport
+
+	defer func() {
+		externalProxyClient.CloseIdleConnections()
+		externalProxyClient.Transport = originalTransport
+		externalProxyClient.CloseIdleConnections()
+	}()
+
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(2 * time.Second)
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer upstream.Close()
+
+	handler := newProxyTestHandler(t, upstream.URL)
+
+	srv := httptest.NewServer(handler)
+	defer srv.Close()
+
+	// A per-request deadline keeps the suite fail-fast: if the handler ever
+	// regresses and blocks, the request errors out instead of hanging until
+	// the overall go test timeout.
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	req, err := http.NewRequestWithContext(ctx, "GET", srv.URL+"/externalproxy", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	req.Header.Set("proxy-to", upstream.URL)
+
+	start := time.Now()
+
+	resp, err := http.DefaultClient.Do(req)
+	elapsed := time.Since(start)
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	defer func() { _ = resp.Body.Close() }()
+
+	if resp.StatusCode != http.StatusBadGateway {
+		t.Errorf("status code = %d, want %d (timeout should produce 502)",
+			resp.StatusCode, http.StatusBadGateway)
+	}
+
+	if elapsed > 1*time.Second {
+		t.Errorf("request took %v, expected to fail fast (well under upstream's 2s sleep)", elapsed)
+	}
+}
+
+func TestExternalProxyReusesConnections(t *testing.T) {
+	var connCount int32
+
+	upstream := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+
+		_, _ = w.Write([]byte("OK"))
+	}))
+
+	upstream.Config.ConnState = func(c net.Conn, s http.ConnState) {
+		if s == http.StateNew {
+			atomic.AddInt32(&connCount, 1)
+		}
+	}
+
+	upstream.Start()
+	defer upstream.Close()
+
+	handler := newProxyTestHandler(t, upstream.URL)
+
+	srv := httptest.NewServer(handler)
+	defer srv.Close()
+
+	for i := 0; i < 5; i++ {
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+
+		req, err := http.NewRequestWithContext(ctx, "GET", srv.URL+"/externalproxy", nil)
+		if err != nil {
+			cancel()
+			t.Fatal(err)
+		}
+
+		req.Header.Set("proxy-to", upstream.URL)
+
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			cancel()
+			t.Fatal(err)
+		}
+
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("request %d: status code = %d, want %d", i, resp.StatusCode, http.StatusOK)
+		}
+
+		_, _ = io.Copy(io.Discard, resp.Body)
+		_ = resp.Body.Close()
+
+		cancel()
+	}
+
+	got := atomic.LoadInt32(&connCount)
+	if got > 2 {
+		t.Errorf("upstream saw %d new connections across 5 sequential requests, "+
+			"expected ~1 with Keep-Alive enabled", got)
+	}
 }
 
 func TestExternalProxyTimeout(t *testing.T) {