backend: Extract handleExternalProxy from createHeadlampHandler #4298

hxrshxz · 2025-12-20T17:08:13Z

Summary

This PR refactors createHeadlampHandler by extracting the external proxy handler into a separate method, reducing function length and improving code maintainability.

Related Issue

Fixes #3273 (partial)

Changes

Extracted /externalproxy handler into handleExternalProxy method (~108 lines)
Added oauthRequestMap and oauthMu fields to HeadlampConfig struct
Updated OIDC handlers to use struct fields instead of local variables
Reduced createHeadlampHandler from ~530 to ~420 lines

Steps to Test

Run the backend linter: make backend-lint
Run the backend tests: make backend-test
Verify the external proxy functionality works as expected

Screenshots (if applicable)

N/A - Backend refactoring only

Notes for the Reviewer

N/A

k8s-ci-robot · 2025-12-20T17:08:21Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hxrshxz
Once this PR has been reviewed and has the lgtm label, please assign sniok 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 refactors the createHeadlampHandler function by extracting the external proxy handler logic into a separate method called handleExternalProxy. The refactoring moves OAuth state management from local variables to struct fields and reduces the main handler function length by approximately 110 lines.

Key Changes:

Extracted /externalproxy handler logic (~108 lines) into new handleExternalProxy method
Added oauthRequestMap and oauthMu fields to HeadlampConfig struct for OAuth state management
Updated OIDC handlers to use struct-level fields instead of local variables

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

backend/cmd/headlamp.go

Copilot · 2025-12-20T17:15:00Z

backend/cmd/headlamp.go

+	defer resp.Body.Close()
+


Duplicate defer statement for resp.Body.Close(). The response body is already deferred for closure at line 895. This second defer at line 929 is redundant and should be removed.

Suggested change

defer resp.Body.Close()

backend/cmd/headlamp.go

skoeva

could you adjust the commit message to match the PR title? take a look here for more info

illume

Seems there’s some tests failing.

To run them locally you can do:

make backend-test

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

backend/cmd/headlamp.go

illume

Thanks for this.

If you want to continue this:

Please see the open review comments?
Probably handleExternalProxy should be moved into a separate module?

hxrshxz · 2026-02-02T20:27:36Z

hey @illume! moved the external proxy handler to a seperate pkg/externalproxy module as you suggested. also fixed all the copilot review comments while i was at it (variable shadowing, removed redundant timeout, added graceful shutdown for oauth cleanup, etc). all tests passing now 👍

illume · 2026-02-02T20:58:15Z

The github tests weren't running for some reason. Closing the PR and opening it again got them running again.

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-02T20:58:51Z

backend/pkg/externalproxy/handler_test.go

+			// Use the parsed URL to avoid unused warning
+			_ = parsedURL.String()


This code appears to be working around a linter warning by artificially using parsedURL. The variable is parsed but never meaningfully used in the test logic. Consider removing both the parsing and this workaround since the test validates URL allowance through the HTTP response behavior, not by directly checking the parsed URL.

Copilot · 2026-02-02T20:58:52Z

backend/cmd/headlamp.go

+	// Initialize OAuth request map if not already initialized
+	if config.oauthRequestMap == nil {
+		config.oauthRequestMap = make(map[string]*OauthConfig)
+	}

-			return
-		}
+	// Initialize OAuth cleanup done channel if not already initialized
+	if config.oauthCleanupDone == nil {
+		config.oauthCleanupDone = make(chan struct{})
+	}


The initialization logic for oauthRequestMap and oauthCleanupDone should be moved to a proper initialization function or constructor for HeadlampConfig rather than being placed inside the handler creation. This prevents the cleanup goroutine from being started multiple times if createHeadlampHandler is called more than once.

Copilot · 2026-02-02T20:58:52Z

backend/cmd/headlamp.go

+	// Start background goroutine to clean up expired OAuth entries (prevents unbounded memory growth)
+	go func() {


Starting this goroutine unconditionally in createHeadlampHandler means multiple goroutines could be created if the function is called multiple times. This creates a goroutine leak since there's no mechanism to stop previously started cleanup goroutines. The cleanup goroutine should be started once during config initialization, not during handler creation.

illume · 2026-02-02T21:01:00Z

Thanks for this! I will come back to try it out and look in more detail.

Could you please consider changing the git commit message to this?

backend: externalproxy: Extract handleExternalProxy to seperate pkg/externalproxy module

…xternalproxy module

Copilot AI review requested due to automatic review settings December 20, 2025 17:08

k8s-ci-robot added the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Dec 20, 2025

k8s-ci-robot requested a review from illume December 20, 2025 17:08

k8s-ci-robot requested a review from yolossn December 20, 2025 17:08

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 20, 2025

Copilot started reviewing on behalf of hxrshxz December 20, 2025 17:08 View session

hxrshxz force-pushed the refactor/create-headlamp-handler branch from e8e0102 to df27629 Compare December 20, 2025 17:09

k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Dec 20, 2025

Copilot AI reviewed Dec 20, 2025

View reviewed changes

hxrshxz force-pushed the refactor/create-headlamp-handler branch from df27629 to af01932 Compare December 20, 2025 17:23

skoeva reviewed Dec 23, 2025

View reviewed changes

hxrshxz force-pushed the refactor/create-headlamp-handler branch from af01932 to 57afc45 Compare December 23, 2025 10:00

hxrshxz requested a review from skoeva December 23, 2025 10:00

illume requested changes Dec 31, 2025

View reviewed changes

hxrshxz marked this pull request as draft December 31, 2025 12:22

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 31, 2025

hxrshxz force-pushed the refactor/create-headlamp-handler branch 4 times, most recently from f1f256e to 08668a2 Compare December 31, 2025 12:29

hxrshxz marked this pull request as ready for review December 31, 2025 12:30

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 31, 2025

k8s-ci-robot requested review from sniok and vyncent-t December 31, 2025 12:30

hxrshxz force-pushed the refactor/create-headlamp-handler branch 4 times, most recently from 2e0b8a9 to d222047 Compare December 31, 2025 13:25

hxrshxz requested a review from illume December 31, 2025 14:06

hxrshxz force-pushed the refactor/create-headlamp-handler branch from d222047 to e03d74e Compare January 3, 2026 17:44

skoeva requested a review from Copilot January 7, 2026 13:27

Copilot started reviewing on behalf of skoeva January 7, 2026 13:28 View session

Copilot AI reviewed Jan 7, 2026

View reviewed changes

illume requested changes Feb 1, 2026

View reviewed changes

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 2, 2026

hxrshxz force-pushed the refactor/create-headlamp-handler branch 2 times, most recently from 5034e03 to ec56aa6 Compare February 2, 2026 20:33

illume closed this Feb 2, 2026

illume reopened this Feb 2, 2026

illume requested a review from Copilot February 2, 2026 20:57

Copilot AI reviewed Feb 2, 2026

View reviewed changes

backend: externalproxy: Extract handleExternalProxy to seperate pkg/e…

f11b440

…xternalproxy module

hxrshxz force-pushed the refactor/create-headlamp-handler branch from ec56aa6 to f11b440 Compare February 2, 2026 21:14

		// Use the parsed URL to avoid unused warning
		_ = parsedURL.String()

		// Start background goroutine to clean up expired OAuth entries (prevents unbounded memory growth)
		go func() {

backend: Extract handleExternalProxy from createHeadlampHandler #4298

Are you sure you want to change the base?

backend: Extract handleExternalProxy from createHeadlampHandler #4298

Conversation

hxrshxz commented Dec 20, 2025

Summary

Related Issue

Changes

Steps to Test

Screenshots (if applicable)

Notes for the Reviewer

Uh oh!

k8s-ci-robot commented Dec 20, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

skoeva left a comment

Choose a reason for hiding this comment

Uh oh!

illume left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

illume left a comment

Choose a reason for hiding this comment

Uh oh!

hxrshxz commented Feb 2, 2026

Uh oh!

illume commented Feb 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

illume commented Feb 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants