Skip to content

backend: externalproxy: Fix glob panic and pre-compile patterns#5678

Merged
illume merged 4 commits into
kubernetes-sigs:mainfrom
ayushmaan-16:fix/externalproxy-glob-panic-final
May 19, 2026
Merged

backend: externalproxy: Fix glob panic and pre-compile patterns#5678
illume merged 4 commits into
kubernetes-sigs:mainfrom
ayushmaan-16:fix/externalproxy-glob-panic-final

Conversation

@ayushmaan-16
Copy link
Copy Markdown
Contributor

@ayushmaan-16 ayushmaan-16 commented May 15, 2026 •
edited
Loading

Description

This change fixes a bug where an invalid /externalproxy URL glob pattern could crash the Headlamp backend. It also improves request handling by compiling allowed proxy URL patterns once and reusing the compiled patterns during external proxy validation.

Changes

  • Centralized proxy URL glob compilation: Added a shared helper for compiling configured proxy URL patterns so startup, request handling, and tests use the same path.
  • Safer external proxy validation: /externalproxy now validates requests through a helper that can compile configured ProxyURLs when needed, preventing configs with only ProxyURLs populated from silently denying all proxy requests.
  • Startup validation: Invalid proxy URL glob patterns are detected during config creation and logged with a clear error before startup exits.
  • Improved performance: Valid proxy URL patterns are precompiled and stored for reuse instead of being reprocessed on every request.
  • Updated tests: Added coverage for invalid glob pattern compilation and for configs that provide ProxyURLs without manually precompiled patterns.

@k8s-ci-robot k8s-ci-robot requested review from ashu8912 and yolossn May 15, 2026 19:40
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 15, 2026
@illume illume requested a review from Copilot May 16, 2026 10:03
Copilot AI reviewed May 16, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the backend external proxy configuration path to precompile allowed glob patterns during startup and reuse them during request handling.

Changes:

  • Adds compiled proxy URL glob storage to HeadlampConfig.
  • Compiles proxy URL patterns in createHeadlampConfig.
  • Updates external proxy tests to provide precompiled patterns.

CI/check status and commit history were not available in this review context; please confirm both are clean.

Reviewed changes

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

File Description
backend/cmd/server.go Builds proxy URL slices and precompiles glob patterns during config creation.
backend/cmd/headlamp.go Uses precompiled glob patterns when validating external proxy requests.
backend/cmd/headlamp_test.go Updates external proxy tests to populate compiled proxy URL patterns.

Comment thread backend/cmd/headlamp.go
Comment thread backend/cmd/server.go Outdated
illume
illume requested changes May 16, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this.

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

@illume illume requested a review from Copilot May 17, 2026 06:38
Copilot AI reviewed May 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread backend/cmd/headlamp.go
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 17, 2026
@ayushmaan-16 ayushmaan-16 requested a review from illume May 17, 2026 07:03
@illume illume requested a review from Copilot May 17, 2026 10:45
Copilot AI reviewed May 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread backend/cmd/headlamp.go Outdated
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 17, 2026
illume
illume requested changes May 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution.

This PR has git conflicts — could you resolve them?

How to resolve conflicts

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

There are some open Copilot review comments — could you take a look at them? Please mark each one as resolved once you've addressed it.

@ayushmaan-16
backend: externalproxy: Fix glob panic and pre-compile patterns
63287b9 
Refactor the externalproxy handler to pre-compile glob patterns at server
startup instead of during each request. This change:
- Prevents server panics caused by glob.MustCompile with invalid patterns.
- Handles compilation errors gracefully at startup by logging and exiting.
- Improves performance by eliminating redundant per-request compilation.
- Updates unit tests to support pre-compiled glob patterns.

Signed-off-by: ayxsh_shxrma <ayushmaan.sharma911@gmail.com>
@ayushmaan-16
backend: externalproxy: Fix glob panic and pre-compile patterns
88fc759 
Refactor the externalproxy handler to pre-compile glob patterns at server
startup instead of during each request. This change:
- Prevents server panics caused by glob.MustCompile with invalid patterns.
- Handles compilation errors gracefully at startup by logging and exiting.
- Improves performance by eliminating redundant per-request compilation.
- Updates unit tests to support pre-compiled glob patterns.
- Fixes linter errors related to formatting and whitespace.

Signed-off-by: ayxsh_shxrma <ayushmaan.sharma911@gmail.com>
@ayushmaan-16
backend: externalproxy: Centralize glob compilation
e53b740
@ayushmaan-16
backend: externalproxy: Guard compiled glob cache
5323d90
@ayushmaan-16 ayushmaan-16 force-pushed the fix/externalproxy-glob-panic-final branch from 7c84da0 to 5323d90 Compare May 18, 2026 12:21
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 18, 2026
@illume illume requested a review from Copilot May 18, 2026 15:58
Copilot AI reviewed May 18, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

illume
illume approved these changes May 19, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 thanks!

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented May 19, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ayushmaan-16, illume

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 19, 2026
@illume illume merged commit f7200f6 into kubernetes-sigs:main May 19, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@illume illume illume approved these changes

@ashu8912 ashu8912 Awaiting requested review from ashu8912

@yolossn yolossn Awaiting requested review from yolossn

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ayushmaan-16 @k8s-ci-robot @illume