fix(resolver): send the original name to the next resolver for rewritten queries

[0xERR0R]

Follow-up to #2223, which restored one half of what #1884 dropped. This restores the other half.

#1884 removed RewriterResolver and moved rewriting into CustomDNSResolver and ConditionalUpstreamResolver. The old rewriter wired its inner resolver to a NoOpResolver and continued the chain itself, after putting the original request back:

// Revert the request: must be done before calling r.next
request.Req = original

Both new resolvers instead continue the chain from inside their own resolution, while request.Req still holds the rewritten message. So the rewrite target is what leaves the resolver:

customDNS:
  rewrite:
    example.com: printer.lan
  mapping:
    printer.lan: 192.168.178.3

blog.example.com is rewritten to blog.printer.lan, misses the mapping, and blog.printer.lan is what goes upstream — an internal name handed to a public resolver, and an NXDOMAIN for a name the upstream could have answered. The documentation promises the opposite:

If not found and if fallbackUpstream was set to true, the original query "blog.example.com" will be sent upstream.

What changed

Both resolvers revert the request before any exit, and delegate to the next resolver themselves instead of from inside processRequest.

For ConditionalUpstreamResolver that is a move of request.Req = original above the delegation. For CustomDNSResolver, processRequest now reports whether the mapping handled the query at all — the same shape the conditional resolver's processRequest already had — rather than continuing the chain on its own. processCNAME keeps resolving out-of-mapping CNAME targets (#1867) by delegating explicitly.

Three things fall out of that:

• fallbackUpstream now works with filterUnmappedTypes: false. Previously the fallback could only fire for the empty CUSTOM DNS response that filterUnmappedTypes: true produces; with it off, the unmapped-type case breaks to the next resolver and never reached the check at all.
• ConditionalUpstreamResolver no longer leaks the rewritten request on error. When the next resolver failed it returned before the revert, leaving the caller's model.Request pointing at the rewritten message — so query logging, metrics and server.go's SERVFAIL reply all reported the internal name instead of what the client asked for.
• shouldFallbackUpstream loses its answered parameter. Callers now hand it only queries they answered themselves, so it no longer has to be told. That also means an error from CustomDNSResolver is always its own, so the error case is honoured for it too — a CNAME loop or an unsupported RR type in the mapping now falls back instead of returning SERVFAIL. A cancelled context is excluded: it would only fail again on the next resolver, and its own error is the useful one.

Tests

Five specs, each failing on main before the change:

spec	on main
conditional: rewritten name matches no mapping	next resolver saw www.nomatch.example.
conditional: next resolver fails	request.Req left at www.nomatch.example.
customDNS: rewritten domain not in mapping	next resolver saw www.nomatch.example.
customDNS: unmapped type, filterUnmappedTypes: false	next resolver saw www.custom.domain.
customDNS: mapping errors, fallbackUpstream: true	CNAME loop detected surfaced instead of falling back

Two of them also assert the next resolver is called exactly once, pinning the invariant that a query which never hit the mapping is not resolved twice.

The existing cname.example → example.com specs cover the delegation processCNAME now does itself.

Test cleanup

Also in here, since it is the same specs:

• The two fallbackUpstream error specs from fix(resolver): restore fallbackUpstream for rewritten queries #2223 built their dead upstream with Start() then Close(), which hands the ephemeral port back to the OS while the config still points at it. Under ginkgo -p, or if anything else claims that port, the "dead" upstream comes back to life and the spec asserts the wrong branch. Replaced with NewBrokenUDPUpstreamServer(), which keeps the port bound and answers with data the client cannot parse — MockUDPUpstreamServer already had that mode.
• The identical nine-line mock wiring repeated across four specs is now newRecordingResolver.
• Dropped a dead sut = NewCustomDNSResolver(cfg) in a BeforeEach — the outer JustBeforeEach rebuilds sut and re-wires Next afterwards, so it was discarded immediately.

Verification

go test ./... green over 20 packages (e2e excluded locally), -race green, gofmt clean, golangci-lint v2.12.2 — the version pinned in the Makefile — reports 0 issues.

Not in scope

A CNAME whose target has no record of the queried type still yields a CNAME-only answer, so len(Answer) == 0 is false and fallbackUpstream does not fire. That predates #1884 — the old rewriter tested Answer == nil and behaved identically — so it is a limitation of the feature rather than part of this regression.

https://claude.ai/code/session_01NueDwL4oyg972RkyQezMJL

[Copilot]

🟢 Approval recommended

The resolver-chain semantics are corrected with clear, targeted refactoring and are backed by new/expanded tests that assert both downstream qname correctness and single-delegation behavior.

Pull request overview

This PR fixes resolver-chain behavior for rewritten DNS queries by ensuring the original query name (not the rewritten/internal one) is what gets passed to downstream resolvers and used for fallback logic, aligning runtime behavior with the documented fallbackUpstream semantics and preventing leaked rewritten names in errors/metrics/logging.

Changes:

• Restore original request.Req before delegating to the next resolver in both CustomDNSResolver and ConditionalUpstreamResolver, so downstream resolvers see the client’s original name.
• Refactor CustomDNSResolver.processRequest to report whether the mapping handled the query, moving delegation responsibility to Resolve (and explicitly in processCNAME) so request restoration always happens before exiting.
• Strengthen/expand specs and add test helpers (newRecordingResolver, NewBrokenUDPUpstreamServer) to verify downstream is called exactly once and sees the original name under multiple scenarios.

File summaries

File	Description
resolver/rewrite_helper.go	Simplifies shouldFallbackUpstream API and adds context-cancellation exclusions for error-triggered fallback.
resolver/mocks_test.go	Adds reusable test helpers to record delegated qname and to simulate a reliably broken UDP upstream.
resolver/custom_dns_resolver.go	Ensures request reversion happens before any exit and prevents rewritten names from leaking downstream; refactors mapping resolution to return “handled”.
resolver/custom_dns_resolver_test.go	Updates/extends specs to assert downstream sees original qname and is called exactly once across mapping-miss, unmapped-type, and error cases.
resolver/conditional_upstream_resolver.go	Restores original request before delegation/fallback decisions so rewritten names don’t leak to next resolver or callers on error.
resolver/conditional_upstream_resolver_test.go	Adds specs for rewrite+no-mapping and next-resolver failure; uses broken-upstream helper to avoid flaky port reuse.
docs/configuration.md	Clarifies that fallbackUpstream is ignored when no rewrite rules are configured.
config/rewriter.go	Documents that FallbackUpstream only applies when rewrite rules exist.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.08%. Comparing base (77b0fe7) to head (1d29a38).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2232      +/-   ##
==========================================
+ Coverage   88.05%   88.08%   +0.02%     
==========================================
  Files         126      126              
  Lines        9958     9961       +3     
==========================================
+ Hits         8769     8774       +5     
+ Misses        924      923       -1     
+ Partials      265      264       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.