reverseproxy: extract DNS SRV+A discovery into a reusable package (RFC)#7790
Open
tannevaled wants to merge 2 commits into
Open
reverseproxy: extract DNS SRV+A discovery into a reusable package (RFC)#7790tannevaled wants to merge 2 commits into
tannevaled wants to merge 2 commits into
Conversation
|
|
tannevaled
force-pushed
the
feat/dedup-dynamic-upstreams
branch
from
9eb4ac3 to
b93be96
Compare
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.
tannevaled
changed the title
tannevaled
force-pushed
the
feat/dedup-dynamic-upstreams
branch
from
b93be96 to
5d83b60
Compare
…und, 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).
Member
|
I don't think it should be a top-level package/directory, it has nothing to do with Caddy as a runtime specifically. We typically put things in I would say this would end up becoming a package in So I'm thinking we can probably just put it in |
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
3 participants
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.
RFC — please treat as a proposal for discussion
Materializes the de-duplication idea from caddyserver/caddy-l4#429, where a few caddy-l4 PRs copied the dynamic-upstreams DNS logic from this repo. Rather than keep a second copy in caddy-l4, this extracts the reusable part into core so both can share it.
What
dynamicupstreamswith the SRV and A/AAAA resolution + caching/refresh/grace logic, returning neutralTarget{Host, Port, Priority, Weight}values instead of*reverseproxy.Upstream.reverseproxy.SRVUpstreamsandAUpstreamsnow delegate to it and build their*Upstreams from the returned targets;SRVUpstreams.ResetCachedelegates too. Behavior is unchanged (same caching, grace period, filtered-records handling,dial_networkprefix, IP-version handling, debug logs).reverseproxy/upstreams.goshrinks substantially; the logic now lives once, in a package third-party proxies can import.Why
So proxies outside this repo (e.g. layer4 TCP/UDP proxies) can reuse the same DNS discovery + caching instead of copying it.
Notes
dynamicupstreams) is a first guess — happy to move/rename to fit core's conventions.Tests
dynamicupstreamshas its own tests (SRV + A resolve/cache, error without grace, grace serves stale, reset). Existingreverseproxytests pass;gofmt/go vet/golangci-lintclean; no go.mod change.Does this direction work for you?