Add per-route forward-health-checks option#246
Conversation
Add a new per-route boolean option `forward-health-checks` to URLMapper, parsed by all providers (docker, file, consul-catalog, static). When enabled, reproxy will forward /ping and /health requests to the backend instead of handling them with built-in responses. Made-with: Cursor
…hecks Modify pingHandler and healthMiddleware to check if the matched route has ForwardHealthChecks enabled. If so, the request is forwarded to the backend instead of being intercepted by reproxy's built-in responses. Made-with: Cursor
|
Is there anything I can do to prevent this PR from being considered junk? Maybe I should file an issue? |
Hmm, why? I have seen it, noticed 15 updated files, and decided to postpone the review until I have time for this. |
umputun
left a comment
There was a problem hiding this comment.
the branch is a bit behind master (missing #245 and #247), would be good to rebase for a clean diff.
main issue I see — inconsistent forward-health-checks value parsing across providers:
- docker (
docker.go:176-178): key presence alone enables it, soreproxy.forward-health-checks=falsewould still enable forwarding - static (
static.go:40): any non-empty string enables it, same problem with"false"or"no" - consul-catalog (
consulcatalog.go:195): properly checks for"true","yes","1" - file (
file.go:88): YAML boolean, works correctly
docker should follow the existing keep-host pattern which inspects the value. static should also handle "false"/"no" correctly.
minor: shouldForwardHealthChecks in health.go calls Match() on every /ping and /health request, then matchHandler calls it again later for the same request. not a big deal given health check frequency, just noting it.
Summary
Some HTTP services behind reproxy expose their own
/pingor/healthendpoints with application-specific responses that clients depend on. For example, audiobookshelf's/pingmust return{"success": true}as a JSON object — its clients use this to verify the server is reachable and functioning correctly. Currently, reproxy's built-inpingHandlerandhealthMiddlewareintercept allGET /pingandGET /healthrequests globally (responding with"pong"and a health JSON summary respectively) before route matching even runs, so these requests never reach the backend. There is no way to disable this per route or per domain.This PR adds a new per-route option
forward-health-checks(supported across all providers: Docker labels, file config, Consul Catalog tags, and static rules) that forwards/pingand/healthrequests to the backend instead of reproxy handling them. The option is off by default, so existing behavior is fully preserved.Docker example
File provider example
Test plan
go test -race -timeout=60s -count 1 ./...)forward-health-checkslabelforward-health-checkstagforward-health-checksYAML field/pingand/healthforwarded when flag is set, built-in response preserved when not