backend: Reuse a configured HTTP client in /externalproxy

[VijayabaskarR-06]

Summary

The /externalproxy handler was newing up an http.Client{} on every incoming request, with no timeout and no custom transport. That meant a slow or hanging upstream would block the request goroutine forever, slowly leaking goroutines and file descriptors, and the default transport's pool limits applied per-host so under load the same TCP connections were getting torn down and rebuilt instead of reused.

This PR replaces that per-request client with a single shared package-level http.Client backed by a tuned http.Transport. The transport sets a ResponseHeaderTimeout so stalling upstreams get cut off, dial / TLS / continue timeouts so handshake failures don't hang either, and a larger idle-connection pool so Keep-Alive can actually reuse sockets.

I deliberately avoided setting a Client.Timeout because that includes body read time and would break legitimate long streaming responses (logs, anything tailed). The transport-level timeouts protect against the slow-headers case without that side effect, which is the same approach httputil.ReverseProxy takes.

Related Issue

Fixes #5512

Changes

• backend/cmd/headlamp.go: added a package-level externalProxyClient with a configured http.Transport (ResponseHeaderTimeout 30s, DialContext timeout 30s, MaxIdleConnsPerHost 10, etc.) and switched the /externalproxy handler to use it.
• backend/cmd/headlamp_test.go: added two tests. TestExternalProxyResponseHeaderTimeout drives a slow upstream that sleeps 2s and confirms the proxy returns 502 in well under a second. TestExternalProxyReusesConnections fires five sequential requests through the proxy and asserts the upstream sees roughly one new TCP connection (proves Keep-Alive is being used).

Steps to Test

1. cd backend then make backend and make backend-lint, both should be clean (0 issues from golangci-lint).
2. go test ./..., the full backend suite passes (14 packages).
3. go test -run TestExternalProxyResponseHeaderTimeout -v ./cmd/..., should pass and show the timeout firing before the upstream's own sleep returns.
4. go test -run TestExternalProxyReusesConnections -v ./cmd/..., should pass and confirm the upstream connection counter stays low.

Screenshots (if applicable)

N/A, backend-only change.

Notes for the Reviewer

• The shared client is exposed at package scope on purpose so tests can swap timeouts on it (the timeout test temporarily lowers ResponseHeaderTimeout to 200ms so the suite stays fast). Restored in defer.
• I did not touch the gzip handling block or the response writing block, those are unrelated to this issue.
• Pool sizes (MaxIdleConns 100, MaxIdleConnsPerHost 10) are conservative and just bigger than the net/http defaults of 100 and 2, happy to tune if you'd prefer different numbers.
• No frontend, type, or i18n impact.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: VijayabaskarR-06
Once this PR has been reviewed and has the lgtm label, please assign illume for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Copilot]

Pull request overview

This PR improves the backend /externalproxy handler by reusing a single shared http.Client backed by a tuned http.Transport, addressing potential goroutine/FD leaks from slow or hanging upstreams and improving connection reuse under load.

Changes:

• Introduces a package-level externalProxyClient with transport-level timeouts and a larger per-host idle connection pool.
• Updates the /externalproxy handler to use the shared client instead of allocating a new http.Client per request.
• Adds regression tests for response header timeout behavior and upstream connection reuse.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
backend/cmd/headlamp.go	Adds a shared, configured HTTP client and switches /externalproxy to use it.
backend/cmd/headlamp_test.go	Adds tests validating response-header timeout behavior and keep-alive connection reuse.

[illume]

Thanks for these changes.

It looks like this PR has git conflicts. Can you please fix them?

How to resolve conflicts

Rebase or merge the latest main into your branch, resolve the conflicts, and push the updated branch.

Can you please address the open review comments? Once you've resolved each one, please mark it as resolved.

[illume]

Thanks for this PR.

A few of the commits don't quite follow the project guidelines. We use Linux kernel style for git commits — have a look at the contributing guide and previous commits with git log.

Commits that need attention

• backend: Address externalproxy review comments — Only one file changed inside backend/; add a sub-area so it's clear what was touched (e.g. backend: ComponentName: description).

Commit guidelines

• Use atomic commits focused on a single change.
• Use the title format <area>: <Description of changes> — description must start with a capital letter.
• Keep the title under 72 characters (soft requirement).
• Explain the intention and why the change is needed.
• Make commit titles meaningful and describe what changed.
• Do not add code that a later commit rewrites; squash or reorder commits instead.
• Do not include Fixes #NN in commit messages.

Good examples:

• frontend: HomeButton: Fix so it navigates to home
• backend: config: Add enable-dynamic-clusters flag

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

backend/cmd/headlamp.go:639

• externalProxyClient adds a ResponseHeaderTimeout, but the handler still reads the entire response body via io.ReadAll (and the upstream request is created with context.Background()). If the upstream sends headers and then stalls (or the downstream client disconnects), this goroutine can still block indefinitely and keep the connection/file descriptor open. Consider creating proxyReq with r.Context() and adding a per-request deadline/cancellation strategy for the body read (or stream the body to w instead of io.ReadAll) so slow/hung bodies can’t leak goroutines.

		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)

[illume]

Thanks for working on this.

Would you mind addressing the open Copilot review comments? Please mark each comment as resolved after addressing it.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: VijayabaskarR-06 / name: VijayabaskarR-06 (55cd6a0)

[illume]

Thanks for these changes.

Can you please have a look at the git commits to see if they meet the contribution guidelines? We use a Linux kernel style of git commits. See the contributing guide for general context, and please see previous git commits with git log for examples.

Commits that need attention

• backend: Address externalproxy review comments — Only one file changed inside backend/; add a sub-area so it's clear what was touched (e.g. backend: ComponentName: description).
• Potential fix for pull request finding — Missing area: description prefix — e.g. frontend: HomeButton: Fix so it navigates to home or backend: config: Add enable-dynamic-clusters flag.

Commit guidelines

• Use atomic commits focused on a single change.
• Use the title format <area>: <Description of changes> — description must start with a capital letter.
• Keep the title under 72 characters (soft requirement).
• Explain the intention and why the change is needed.
• Make commit titles meaningful and describe what changed.
• Do not add code that a later commit rewrites; squash or reorder commits instead.
• Do not include Fixes #NN in commit messages.

Good examples:

• frontend: HomeButton: Fix so it navigates to home
• backend: config: Add enable-dynamic-clusters flag

It looks like the Contributor License Agreement (CLA) has not been signed. Kubernetes requires all contributors to sign the CLA before a PR can be merged. Could you please take a look at the EasyCLA bot comment and sign it?

About the CLA

The Kubernetes project uses the Linux Foundation EasyCLA system. Signing takes only a minute through the link in the EasyCLA bot comment. Individual contributors sign electronically; corporate contributors may need their employer to sign a Corporate CLA.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

backend/cmd/headlamp_test.go:597

• This test issues multiple HTTP requests without a context timeout. Adding a short per-request timeout will prevent the test suite from hanging indefinitely if the handler blocks or the test server fails to respond.

		req, err := http.NewRequestWithContext(context.Background(), "GET", srv.URL+"/externalproxy", nil)
		if err != nil {
			t.Fatal(err)
		}

		req.Header.Set("proxy-to", upstream.URL)

		resp, err := http.DefaultClient.Do(req)
		if err != nil {
			t.Fatal(err)
		}

[skoeva]

hi! could you address the failing CI test? you can run make backend-lint-fix locally

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[illume]

Thanks for this PR.

The open review comments from Copilot still need attention — can you have a look? Once addressed, please mark them as resolved.

[VijayabaskarR-06]

Addressed the Copilot comments the handler now forwards the Location header so redirect 30x responses stay usable client side, and TestExternalProxyDoesNotFollowRedirects asserts it. Also rebased onto latest main. go test ./. and golangci lint both pass. Threads resolved. /cc @illume

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[illume]

Thanks for these changes.

can you please rebase against main to remove the merge main commit?

Why this matters

Merge commits from main make the PR history harder to review. Please rebase your branch on top of the latest main instead, then update the PR with the rebased commits.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[k8s-ci-robot]

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.