* reverseproxy: isolate active health-check state per distinct check config
Multiple reverse_proxy handlers configured with different active health
checks (health_uri, health_headers, ...) against the same upstream dial
address currently share a single Host in the global pool, so one
handler's failing probes mark the address unhealthy for every other
handler. Key the pool by dial address plus a stable fingerprint of the
active health-check config, so distinct checks get independent health
state.
The fingerprint is strictly internal to pool identity: the Prometheus
upstreams_healthy label and the /reverse_proxy/upstreams admin endpoint
continue to report the plain dial address, unchanged.
Dynamic upstreams are intentionally out of scope here: they resolve
through a separate per-lookup path (dynamicHosts) and collapsing there
has different lifetime semantics; noted for a follow-up.
Fixes#7870
* reverseproxy: use strings.Cut in hostKeyAddress
Satisfies the modernize linter; behaviour is unchanged, since Cut returns
the whole string when the separator is absent.
* reverseproxy: expose the health-check fingerprint as a public discriminator
Health state is now kept per (dial address, active health check config),
but both user-visible surfaces still reported address alone:
- caddy_reverse_proxy_upstreams_healthy was labeled only by upstream, so
every handler sharing an address wrote the same series concurrently and
the reported value was whichever updater ran last. The metric gains a
health_check label carrying the config fingerprint ("" when no active
checks), so each health target owns its series; aggregate across checks
with sum/min by (upstream).
- /reverse_proxy/upstreams reported one entry per pool key but with only
the plain address, so consumers indexing by address silently discarded
all but one entry. Entries now carry health_check (omitted when empty),
and the endpoint documents that (address, health_check) is the entry's
identity — one entry per health target, deliberately not aggregated,
since any aggregation here would be lossy and undocumented.
Tests: two handlers on one address with different checks must produce two
metric series reflecting their own state (fails if the fingerprint is
dropped from the label), and two admin entries distinguished by non-empty
fingerprints.
* reverseproxy: narrow the fix to per-Upstream active health counters
Move the consecutive active pass/fail counters from Host onto Upstream,
alongside the active unhealthy state that already lives there, instead of
re-keying the global host pool.
Host is keyed by dial address alone, but an active health check is
configured per handler, so two handlers dialing the same address with
different health_uri or health_headers share those counters and can push
each other over their own thresholds. Upstream is already per-handler and
already carries the active unhealthy flag, so the counters belong next to
it and the pool keeps its plain dial-address keys.
This drops the host key fingerprint and its exposure in the metric label
and the admin upstreams endpoint; the metric series identity is left for
separate consideration.
---------
Co-authored-by: SillyZir <269283839+SillyZir@users.noreply.github.com>
Co-authored-by: Zen Dodd <mail@steadytao.com>
* admin: Redact sensitive request headers in API logs
* Fix govulncheck and typed atomic lint failures
* Sync Go module metadata after dependency downgrade
Currently if we extract the DialInfo from a Request Context during an active health check, then the Upstream in the DialInfo is nil.
This PR attempts to set the Upstream to a sensible value, based on wether or not the Upstream has been overriden in the active health check's config.
* Add health_upstream
Signed-off-by: Dylan Schultz <9121234+dylanschultzie@users.noreply.github.com>
* Add health_upstream to caddyfile parsing
* Add Active Upstream case for health checks
* Update ignore health port comment
Signed-off-by: Dylan Schultz <9121234+dylanschultzie@users.noreply.github.com>
* Update Upstream json doc
Signed-off-by: Dylan Schultz <9121234+dylanschultzie@users.noreply.github.com>
* Update modules/caddyhttp/reverseproxy/healthchecks.go
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
* Use error rather than log for health_port override
Signed-off-by: Dylan Schultz <9121234+dylanschultzie@users.noreply.github.com>
* Add comment about port being ignore if using upstream
Signed-off-by: Dylan Schultz <9121234+dylanschultzie@users.noreply.github.com>
---------
Signed-off-by: Dylan Schultz <9121234+dylanschultzie@users.noreply.github.com>
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
* reverseproxy: active health check allows configurable health_passes and health_fails
* Need to reset counters after recovery
* rename methods to be more clear that these are coming from active health checks
* do not export methods
* reverseproxy: Begin refactor to enable dynamic upstreams
Streamed here: https://www.youtube.com/watch?v=hj7yzXb11jU
* Implement SRV and A/AAA upstream sources
Also get upstreams at every retry loop iteration instead of just once
before the loop. See #4442.
* Minor tweaks from review
* Limit size of upstreams caches
* Add doc notes deprecating LookupSRV
* Provision dynamic upstreams
Still WIP, preparing to preserve health checker functionality
* Rejigger health checks
Move active health check results into handler-specific Upstreams.
Improve documentation regarding health checks and upstreams.
* Deprecation notice
* Add Caddyfile support, use `caddy.Duration`
* Interface guards
* Implement custom resolvers, add resolvers to http transport Caddyfile
* SRV: fix Caddyfile `name` inline arg, remove proto condition
* Use pointer receiver
* Add debug logs
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
* reverseproxy: Fix dial placeholders, SRV, active health checks
Supercedes #3776
Partially reverts or updates #3756, #3693, and #3695
* reverseproxy: add integration tests
Co-authored-by: Mohammed Al Sahaf <msaa1990@gmail.com>
* reverseproxy: construct active health-check transport from scratch (Fixes#3691)
* reverseproxy: do upstream health-check on the correct alternative port
* reverseproxy: add integration test for health-check on alternative port
* reverseproxy: put back the custom transport for health-check http client
* reverseproxy: cleanup health-check integration test
* reverseproxy: fix health-check of unix socket upstreams
* reverseproxy: skip unix socket tests on Windows
* tabs > spaces
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
* make the linter (and @francislavoie) happy
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
* One more lint fix
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Either Dial or LookupSRV will be set, but if we rely on Dial always
being set, we could run into bugs.
Note: Health checks don't support SRV upstreams.
Previously, all matchers in a route would be evaluated before any
handlers were executed, and a composite route of the matching routes
would be created. This made rewrites especially tricky, since the only
way to defer later matchers' evaluation was to wrap them in a subroute,
or to invoke a "rehandle" which often caused bugs.
Instead, this new sequential design evaluates each route's matchers then
its handlers in lock-step; matcher-handlers-matcher-handlers...
If the first matching route consists of a rewrite, then the second route
will be evaluated against the rewritten request, rather than the original
one, and so on.
This should do away with any need for rehandling.
I've also taken this opportunity to avoid adding new values to the
request context in the handler chain, as this creates a copy of the
Request struct, which may possibly lead to bugs like it has in the past
(see PR #1542, PR #1481, and maybe issue #2463). We now add all the
expected context values in the top-level handler at the server, then
any new values can be added to the variable table via the VarsCtxKey
context key, or just the GetVar/SetVar functions. In particular, we are
using this facility to convey dial information in the reverse proxy.
Had to be careful in one place as the middleware compilation logic has
changed, and moved a bit. We no longer compile a middleware chain per-
request; instead, we can compile it at provision-time, and defer only the
evaluation of matchers to request-time, which should slightly improve
performance. Doing this, however, we take advantage of multiple function
closures, and we also changed the use of HandlerFunc (function pointer)
to Handler (interface)... this led to a situation where, if we aren't
careful, allows one request routed a certain way to permanently change
the "next" handler for all/most other requests! We avoid this by making
a copy of the interface value (which is a lightweight pointer copy) and
using exclusively that within our wrapped handlers. This way, the
original stack frame is preserved in a "read-only" fashion. The comments
in the code describe this phenomenon.
This may very well be a breaking change for some configurations, however
I do not expect it to impact many people. I will make it clear in the
release notes that this change has occurred.
* fix OOM issue caught by fuzzing
* use ParsedAddress as the struct name for the result of ParseNetworkAddress
* simplify code using the ParsedAddress type
* minor cleanups
This PR enables the use of placeholders in an upstream's Dial address.
A Dial address must represent precisely one socket after replacements.
See also #998 and #1639.
SillyZir
WeidiDeng
Zen Dodd
Matt Holt
Francis Lavoie
Jesper Brix Rosenkilde
schultzie
Sam Ottenhoff
Corin Langosch
Mohammed Al Sahaf
Success Go