Repository navigation
test: cover public and operational HTTP endpoints - #65
Merged
ktrubilo9 merged 1 commit intoOct 13, 2026
Merged
Conversation
Extract the public and operational muxes into Proxy.publicHandler() and Proxy.operationalHandler() so they can be exercised without opening fixed ports, and add tests covering both. Public port: proxied traffic reaches the backend, /health returns a minimal JSON payload that exposes no backend details, /metrics and /metrics/prometheus return 404, and unsupported methods on /health get 405 with an Allow header instead of falling through to the catch-all proxy route. Operational port: /health reports per-backend detail and 503 when no backend is healthy, /metrics returns the JSON payload, /metrics/prometheus returns Prometheus text format, and unsupported methods get 405.
ktrubilo9
reviewed
Oct 13, 2026
|
|
||
| mux := http.NewServeMux() | ||
| mux.Handle("/", finalHandler) | ||
| mux.Handle("/health", readOnlyHandler(handler.PublicHealthHandler(p.Runtime), http.MethodGet, http.MethodHead)) |
Owner
There was a problem hiding this comment.
Nice catch here, keeping /health registered for every methods prevents unsupported requests from falling through to the catch-all proxy route and being forwarded to a backend.
ktrubilo9
reviewed
Oct 13, 2026
|
|
||
| // The reserved /health path must never fall through to the catch-all proxy | ||
| // route, otherwise an unsupported method would be forwarded to a backend. | ||
| if paths := backend.receivedPaths(); len(paths) != 0 { |
Owner
There was a problem hiding this comment.
I like that this test verifies the request never reached the backend instead of checking only the 405 response. This protects the actual behavior we care about and makes the test much more valuable
ktrubilo9
approved these changes
Oct 13, 2026
ktrubilo9
left a comment
ktrubilo9
left a comment
Owner
There was a problem hiding this comment.
Thanks for the contribution, the implementation is clear, focused on the issue and adds meaningful coverage for both HTTP surfaces. I also appreciate the clear PR description and explanation of the routing edge case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #64
Summary
Adds tests for the public and operational HTTP endpoints, which previously had
no direct coverage.
To make the handlers testable without binding fixed ports (
8080,9091), thetwo muxes that
Proxy.Start()built inline are extracted intoProxy.publicHandler()andProxy.operationalHandler().Start()now justwires
http.Server.Handlerto those, so behaviour is unchanged and the testsdrive the exact same handler graph.
internal/proxy/proxy_test.gocovers:Public port (
publicHandler)GET /api/itemsis proxied through the client-IP and rate-limit middlewareto the configured backend, and the backend receives the original path.
GET/HEAD /healthreturns200withapplication/jsonand the minimal{"status":"healthy"}body./healthpayload exposes onlystatus— no backend URLs, and nobackends/active_count/total_countkeys./metricsand/metrics/prometheusreturn404onGET,HEAD,POSTand
DELETE, and never reach a backend.POST/PUT/PATCH/DELETE/OPTIONS/TRACE /healthreturn405withAllow: GET, HEAD, and the reserved path is not forwarded to a backend.Operational port (
operationalHandler)GET/HEAD /healthreturns200withapplication/json, statushealthy,active/total = 1/1, and the backend detail entry./healthreturns503once the single backend is probed unhealthy./metricsreturns JSON containingsystem,backends,rate_limitandhttp./metrics/prometheusreturnstext/plainPrometheus output includingreverse_proxy_requests_total.405withAllow: GET, HEAD.Implementation notes
readOnlyHandlerwraps a handler so that unsupported methods get405with anAllowheader rather than falling through to a broader mux pattern. The routestays registered for every method — registering only
GET /healthwould letPOST /healthmatch the catch-all"/"pattern and be proxied to a backend.A
recordingBackendtest double asserts on this: every method-rejection testalso checks the backend recorded no requests.
Verification
All run on go1.26.0 (the version in
go.mod):gofmt -l .— cleango vet ./...— cleango test ./...— all packagesokgo test -race ./...— all packagesokgolangci-lint run ./...(v2.13, the version CI pins) —0 issues