Files
caddy/modules
tannevaledandFrancis Lavoie f2c20fb810 reverseproxy: deduplicate DNS SRV+A discovery into modules/internal/network (#7790)
* reverseproxy: extract DNS SRV+A discovery into a reusable package

Move the SRV and A/AAAA resolution + caching logic out of reverseproxy
into a new, transport-neutral package (dynamicupstreams) that returns
neutral targets instead of *reverseproxy.Upstream. reverseproxy's
SRVUpstreams and AUpstreams now delegate to it and build their own
upstreams from the targets; behavior is unchanged.

This lets other proxies (e.g. third-party layer4 proxies) reuse the same
DNS discovery + caching instead of copying it, reducing duplication and
maintenance burden. reverseproxy/upstreams.go shrinks substantially.

RFC for the de-duplication discussed in caddyserver/caddy-l4#429.

* test(dynamicupstreams): cover ResetAllSRV, filtered records, cache bound, concurrent refresh

Brings the package to 96.8% statement coverage. Adds:
- TestResetAllSRV: full-cache drop
- TestSRVFilteredRecords: LookupSRV partial-error semantics (usable
  records returned, error downgraded to a warning)
- TestSRVCacheBound / TestACacheBound: eviction keeps the cache bounded
  at maxCacheEntries
- TestSRVConcurrentRefreshDeduplicates / TestAConcurrentRefreshDeduplicates:
  two goroutines missing the read-lock fast path trigger only one lookup
  (verified under -race)

The grace-period and filtered-records logs now use a level-enabled logger
so the log-write branches execute. The only uncovered statements are the
two double-checked-locking re-check returns, which are reachable solely in
a narrow race window (the same untested idiom as reverseproxy's SRV/A).

* reverseproxy: move DNS SRV+A discovery into modules/internal/network

Per review feedback, the shared discovery code does not belong in a
top-level package: it is internal helper code, not part of Caddy's
runtime surface. Move it from the top-level dynamicupstreams package into
the existing modules/internal/network package and update reverse_proxy's
upstreams.go to consume it from there.

Being under modules/internal means third-party modules (e.g. layer4)
cannot import it yet; they keep their own copy until layer4 is part of
Caddy's standard distribution, at which point they can switch over.

* test(dynamicupstreams): correct the concurrent-refresh test's claim

The two concurrent-refresh tests documented themselves as covering the
write-lock re-check branch ("the second re-checks under the write lock"),
but they do not: while the first goroutine holds the write lock the second
cannot acquire the read lock either, so by the time it acquires srvMu the
cache is already populated and it is served by the read-lock fast path. The
50ms sleep was presented as letting the second goroutine "queue on the write
lock" but proved nothing.

Make the comments and failure messages honest: these tests assert
deduplication (two concurrent callers for one uncached key share a single
lookup) and, under -race, race-safety on the shared cache -- not a specific
locking branch. Ordering is enforced by the channels, so the sleep is removed.

No production code change.

* reverseproxy: preserve DNS discovery logging and grace behavior

Keep refresh and discovery logs in the DNS lookup path, retaining the original SRV field types. Preserve error suppression when an SRV lookup fails with grace enabled before any result has been cached.

Add regression tests for refresh-only logging and cold-cache grace handling.

AI-assisted-by: OpenAI Codex (GPT-6)

---------

Co-authored-by: Francis Lavoie <lavofr@gmail.com>
2026-10-12 10:37:55 +00:00
..