* plugin/rewrite: apply rcode rewrites to record-less responses
An rcode rewrite rewrites the message-level RCODE, but the reverter only ran
response rules from inside the per-record loops in WriteMsg. When a response
carries no answer, authority or additional records - for example a bare
SERVFAIL that a downstream plugin returns to a non-EDNS client - none of the
loops iterate, so the rcode rewrite was silently skipped and the client
received the original RCODE.
Apply message-level response rules once when the response has no records, using
a small marker interface that mirrors the existing requestExtraRevertRule
pattern. This fixes the plugin's documented SERVFAIL-to-NOERROR use case for
responses without records.
Signed-off-by: Sueun Cho <sueun.dev@gmail.com>
* plugin/rewrite: apply fallback rcode rewrites for continue
Signed-off-by: Sueun Cho <sueun.dev@gmail.com>
---------
Signed-off-by: Sueun Cho <sueun.dev@gmail.com>
* plugin/cache: add prefer_positive stale policy
Add an opt-in serve_stale_policy that prefers an eligible success-cache
answer over denial-cache entries while serve_stale is enabled. Preserve the
existing ncache-first behavior when the policy is absent.
Also classify SOA-backed CNAME NODATA responses in the cache so incomplete
answers cannot be selected as positive stale responses.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 25da81ab-92dd-4663-b480-efd6262090c6
Signed-off-by: Nitin Nizhawan <nnizhawan@microsoft.com>
* plugin/cache: retain last-known-good positive answers
Keep an answering success-cache item reachable when a later NOERROR or
referral response overwrites the visible cache key without answering the
question. This lets prefer_positive survive empty responses, referrals, and
additional-only data while leaving policy-off lookup behavior unchanged.
Return the exact accepted verify refresh item instead of re-reading an
ambiguous cache key, avoiding expired TTL wraparound for uncacheable replies.
Add regression coverage for non-answer refreshes, NODATA, SERVFAIL, NOTIMP,
stale-window expiry, and bounded verify reply shaping.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 25da81ab-92dd-4663-b480-efd6262090c6
Signed-off-by: Nitin Nizhawan <nnizhawan@microsoft.com>
* plugin/cache: validate preferred stale answers
Reject truncated, DNSSEC-expired, mismatched-class, unrelated ANY, and ambiguous CNAME refreshes before replacing a stale last-known-good answer. Precompute answer eligibility when cache items are created so prefer_positive hits avoid repeated CNAME walks.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 25da81ab-92dd-4663-b480-efd6262090c6
Signed-off-by: Nitin Nizhawan <nnizhawan@microsoft.com>
---------
Signed-off-by: Nitin Nizhawan <nnizhawan@microsoft.com>
Co-authored-by: Nitin Nizhawan <nnizhawan@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 25da81ab-92dd-4663-b480-efd6262090c6
* plugin/kubernetes: copy Labels in Pod.DeepCopyObject
Pod.DeepCopyObject built the copy field by field and left Labels out, so
every copy came back unlabelled. Every other object in this package copies
all of its fields; Pod was the only one missing any.
Labels feeds the kubernetes/client-label/<key> metadata, so anything that
reached a pod through a deep copy would see no labels at all rather than an
error. Nothing does today - DefaultProcessor stores the converted object
straight into the indexer without copying it, which is why this has not
surfaced - so this is a latent bug rather than a live one.
Clone the map instead of assigning it, so the copy does not alias the
original. maps.Clone returns nil for a nil map, so an unlabelled pod stays
unlabelled and a round trip does not turn a nil map into an empty one.
The test covers every runtime.Object in the package rather than just Pod,
and refuses to pass if a fixture leaves a field at its zero value. Adding a
field to any of these types therefore fails the test until the fixture sets
it, which is what makes the round-trip assertion cover the new field too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
* plugin/kubernetes/object: deep copy the Service and ServiceImport ports
The deepcopy tests claimed a package-wide guarantee that a copy shares nothing
with its original, but only checked it for Pod and a hand-written Endpoints
value. Both api.ServicePort and mcs.ServicePort hold an AppProtocol *string, so
the elementwise copy(s1.Ports, s.Ports) in Service.DeepCopyObject and
ServiceImport.DeepCopyObject left the copy pointing at the original's string.
Mutating it through either side was visible from the other, and the tests still
passed.
Copy the ports with the generated DeepCopyInto so the copy owns everything it
can reach, and make the guarantee real: TestDeepCopyObjectIsDeep now runs over
every case in deepCopyCases, mutates every value reachable through a pointer,
slice, or map, and requires the copy to still equal a pristine fixture. A type
that gains a reference-bearing field is covered as soon as assertAllFieldsSet
forces the fixture to populate it, rather than needing a new hand-written case.
Failure messages render as JSON, because %+v prints an aliased pointer field as
an address and hides the value that actually differs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
---------
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Use net.JoinHostPort rather than string concatenation to support
both ipv4, ipv6, and hostname service endpoints for the trace
plugin. Previously, ipv6 bind addresses in the coredns configuration
would fail to be parsed, as the ipv6 address was not surrounded in
brackets.
Signed-off-by: Michael Wolf <mwolf@cloudflare.com>
Closes#8409
Co-authored-by: Michael Wolf <mwolf@cloudflare.com>
* test: add benchmark cases for Request IP/Port and parseRequest
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
* test(kubernetes): add BenchmarkServices and BenchmarkServicesHeadless
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
* perf(hosts): pre-convert Origins to plugin.Zones in hosts setup
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
* style: fix gofmt trailing line formatting
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
---------
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
externalLookup, which resolves the chase for ordinary CNAME, wildcard
CNAME, and DNAME answers, returned a nil additional section. So a chased
SRV/MX/SVCB/HTTPS answer with an in-bailiwick target was missing the
target's A/AAAA glue, unlike the direct path.
Run additionalProcessing at externalLookup's return points so all three
callers add the glue, and add ordinary-CNAME, wildcard-CNAME, and DNAME
regression cases.
Fixes#6628
Signed-off-by: Saleh <root@lr0.org>
Default to two connect attempts per configured upstream so fast failures cannot spin until the request deadline. Track whether max_connect_attempts was explicitly configured so zero still opts into the legacy unbounded behavior.
Fixes#7723
Signed-off-by: houyuwushang <liuluoqianqiu@outlook.com>
* plugin/kubernetes: skip zone serial bump on DNS neutral pod updates
In pods verified mode every pod update event bumped the zone modified
timestamp, even when the pod IP did not change. Pod records only depend
on the pod IP, so routine status churn (conditions, container statuses,
labels) caused spurious SOA serial changes and needless zone transfer
activity, even though pod records are not part of transfers at all.
Only bump the modified timestamp when the pod IP changes, mirroring how
service and endpoint updates are already filtered. Also update the pods
verified documentation to describe the actual overhead: modest memory
for a stripped down pod object, plus watch load on the API server.
Ref #8043
Signed-off-by: Karan V <karanvknarayanan@gmail.com>
* plugin/kubernetes: keep pods verified cost description neutral
Avoid characterizing the memory overhead as modest until benchmark
data quantifies it. State only what the code does: the watch requires
additional memory in CoreDNS and adds load to the API server.
Signed-off-by: Karan V <karanvknarayanan@gmail.com>
---------
Signed-off-by: Karan V <karanvknarayanan@gmail.com>
* test: add benchmark cases for Request IP/Port and parseRequest
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
* test(kubernetes): add BenchmarkServices and BenchmarkServicesHeadless
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
* perf(forward): fast-path string comparison in isAllowedDomain
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
---------
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
RFC 4592 permits a wildcard source of synthesis to exist as an empty non-terminal. Track wildcard names proven by descendant records and return NODATA when such a source is selected.
Fixes#4256
Signed-off-by: houyuwushang <liuluoqianqiu@outlook.com>
Adds two regression tests for #8234 that the existing suite does not cover:
- an OPT-less upstream reply that carries a record, so the per-record
response rules run while the request OPT is reused by ScrubWriter;
- a request that already carries the option, where "set ... revert" must
put the client's original value back rather than just drop the option.
Both fail against the tree before #8235 and pass on current master.
Signed-off-by: maximilize <3752128+maximilize@users.noreply.github.com>
This PR fixes multi-primary AXFR zone contamination. It
use a fresh candidate zone for each primary so records from failed transfers cannot leak into later.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
* plugin/cache: do not cache SOA-less NODATA responses
An upstream may return NOERROR with a non-empty answer that still does not
resolve the question and without an SOA record to bound a negative TTL: a
CNAME chain that does not terminate in a record of the queried type at the
chain's terminal name (an incomplete recursion result from a forwarder).
Because the answer section is non-empty, response.Typify classifies it as
NoError (positive), so the cache plugin stores it keyed on <qname,qtype> and
replays the non-answer to clients until the TTL expires.
Per RFC 2308 section 5, negative responses without an SOA record SHOULD NOT be
cached. Following RFC 1034 section 3.6.2 and RFC 2308 sections 1 and 2.2, the
effective owner name is the target at the end of the CNAME chain, and the
response is NODATA unless it carries the queried type at that terminal name;
this holds for every query type, not just A/AAAA. Skip caching such a response
(mirroring the existing NameError && !hasSOA guard) and let the next query be
resolved upstream again.
An empty answer section is deliberately left cacheable: it is indistinguishable
from a legitimate NOERROR positive response that carries its data outside the
answer section (for example the whoami plugin).
Refs coredns#6958, coredns#5077, coredns#4987.
Signed-off-by: Nitin Nizhawan <nitin.nizhawan@gmail.com>
* plugin/cache: fail closed on malformed CNAME chains in isNODATA
canonicalName now returns a validity flag and rejects chains that are not a
single unambiguous path to a terminal name: an owner with more than one
distinct CNAME target (RFC 2181 section 10.1) and a revisited owner / CNAME
loop (RFC 1034 section 3.6.2). isNODATA treats an invalid chain as a
non-answer, so a SOA-less response with such a chain is not cached. This makes
the classification order-independent (previously the first CNAME per owner
won, so a two-target owner was cacheable or not depending on wire order) and
closes the loop-with-co-located-record case. Duplicate CNAME records naming
the same target are still tolerated.
Adds regression tests for the two-distinct-targets case in both orders, the
CNAME loop with a co-located A, and the tolerated duplicate-identical-CNAME
case.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d59b4564-a9df-425f-858e-aadee0f35581
Signed-off-by: Nitin Nizhawan <nitin.nizhawan@gmail.com>
* plugin/cache: split canonicalName into self-documenting helpers
Extract the per-owner CNAME lookup into uniqueCNAMETarget and loop detection
into a small case-insensitive nameSet type, leaving canonicalName as a short
driver. Signature and algorithm are unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d59b4564-a9df-425f-858e-aadee0f35581
Signed-off-by: Nitin Nizhawan <nitin.nizhawan@gmail.com>
---------
Signed-off-by: Nitin Nizhawan <nitin.nizhawan@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d59b4564-a9df-425f-858e-aadee0f35581
Keep cache timestamps and TTL calculations on the time values returned by the cache clock. Converting them with UTC strips Go's monotonic clock reading and can extend cached entries when the wall clock moves backward.
Add a regression test that verifies new cache items retain the original monotonic timestamp.
Fixes#5478.
Signed-off-by: houyuwushang <liuluoqianqiu@outlook.com>
Detect zone cuts while following CNAME and DNAME targets so partial answers carry the child NS and glue records instead of returning SERVFAIL or apex authority. Keep AA set for authoritative alias partial answers while clearing it for referral-only responses.
Fixes#6405Fixes#6627
Signed-off-by: houyuwushang <liuluoqianqiu@outlook.com>
* plugin/shed: add UDP overload protection plugin
UDP responses written back through one listener socket serialize on the
Go runtime's internal fdMutex, which allows at most 2^20-1 concurrent
operations per file descriptor and panics the process when exceeded.
CoreDNS serves UDP with one goroutine per query, all writing through the
shared packet connection, so a sustained overload parks every excess
in-flight query in that wait queue until the process dies with
"too many concurrent operations on a single file or socket". Observed
in production: ~2.8M goroutines and 60GiB RSS before the panic.
The shed plugin makes the panic structurally unreachable. It installs,
via Config.UDPDecorateWriterFunc, a per-socket bounded evict-oldest
stack drained newest-first by a single writer goroutine, so the fd
never sees more than one writer and residual capacity under overload
always goes to the freshest response. While a socket's stack is full,
arriving queries are dropped before any plugin runs. Drops are silent
(the client's resolver retries elsewhere) and counted in
coredns_shed_dropped_total{server, reason}.
plugin/shed/fdmutex_test.go demonstrates the failure and the fix with
one shared flood harness. Two subprocess tests reproduce the exact
runtime panic without the plugin's write discipline - one deterministic
(a held write plus >2^20 queued writers), one with nothing held or
mocked; both exercise the Go runtime rather than the plugin, so they
are gated behind SHED_FLOOD_TEST=1. The counterfactual - the same load
through the plugin's stack, completing with every response accounted
for as written or dropped - runs in every test invocation, including
-race, at 50k responders, and at the full 1.5M with SHED_FLOOD_TEST=1:
SHED_FLOOD_TEST=1 go test ./plugin/shed/
Signed-off-by: Ryan Brewster <rpb@anthropic.com>
* test: add shed e2e test
Query a shed-enabled server over UDP (the plugin's deferred
single-writer path) and TCP (which shed passes through), and check
that coredns_shed_dropped_total is exported with its reason label.
No-Verification-Needed: test-only change
Signed-off-by: Ryan Brewster <rpb@anthropic.com>
---------
Signed-off-by: Ryan Brewster <rpb@anthropic.com>
* plugin/file: resolve each additional section target only once
additionalProcessing walked the answer and appended each target's address records without keeping track of the targets it had already resolved. An answer holding several records that point at one target, two MX records differing only in preference for instance, therefore repeated that target's A, AAAA and RRSIGs once per record. SRV, SVCB and HTTPS take the same path and behaved the same way. plugin/backend_lookup.go already skips targets it has seen; the file plugin never did.
Targets are compared in canonical form because SRV targets are not lowercased on insert, so one target can reach additional processing spelled two ways while the zone's tree still matches it case-insensitively. The map is allocated on first use, as additionalProcessing runs for every answer and most carry no target at all.
Fixes#6626
Signed-off-by: baltasarblanco <baltablanco9008@gmail.com>
* plugin/file: move additional section tests to lookup_test.go
The cases exercise additionalProcessing, which lives in lookup.go, so they belong with the rest of the lookup tests. No test logic is changed.
Addresses review feedback.
Signed-off-by: baltasarblanco <baltablanco9008@gmail.com>
---------
Signed-off-by: baltasarblanco <baltablanco9008@gmail.com>
The warning added in #8216 passed the skipped file where the kept file
belongs and vice versa, so it logged "using <skipped> instead of <kept>"
— the reverse of what Walk actually does. Swap the arguments (and use
cleanPath for both) so the message names the file that is being used.
Strengthen TestWalkWarnsForDuplicateOrigin to assert the direction; it
only checked that both names appeared, so the inversion passed before.
Signed-off-by: maximilize <3752128+maximilize@users.noreply.github.com>
Make mtime reload tests tolerate slow scheduling and coarse filesystem
timestamp resolution. This keeps immediate rewrites observable and
avoids checking zone contents before the reload completes.
Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
Plugin package tests previously ran only on Linux, so Windows-
specific failures were never caught. Run them in CI and make the
affected tests portable across platforms.
Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
This PR fixes the forward plugin incorrectly retrying deterministic
local DNS message serialization failures as if they were upstream transport errors.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Directory permission checks can be bypassed by privileged users and may
behave differently on filesystems with nonstandard permission semantics.
Make the bad-path case deterministic so TLS keylog tests remain portable
across supported environments.
Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
DoH, DoQ, and DNS-over-gRPC unpack messages without the acceptance
checks used by UDP and TCP. An unauthenticated request with a large
QDCOUNT can therefore force excessive allocations while names are
decoded and exhaust server memory.
Enforce the same request policy across all server transports.
Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
* plugin/proxyproto: Apply an explicitly configured default policy even when no allow list is present.
This PR fix the issue where explicitly configured default reject policy
is ignoreed when no allo list is present
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
* golint fix
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
---------
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
* plugin/forward: Fix incorrect failover counter reset
This PR fixes the isseue where resetting the failover counter caused
retry the same upstreams until timeout instead of stopping after one pass.
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
* golint fix
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
---------
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
A query for a name that sits below a closer empty non-terminal was wrongly
answered with a shallower wildcard record instead of NXDOMAIN. Per RFC 4592
2.2.1 a wildcard is the source of synthesis only when the wildcard owner's
parent is the closest encloser of the queried name; if an empty non-terminal
exists between that parent and the queried name, it is the closer encloser and
the shallower wildcard must not be expanded.
Guard the wildcard expansion in Zone.Lookup with closerENTExists, which walks
the strict ancestors of the queried name between the wildcard parent and the
name and reports whether any of them is an empty non-terminal.
Adds TestLookupWildcardRespectsCloserEmptyNonTerminal, which asserts NXDOMAIN
for a name below a closer empty non-terminal and keeps a no-regression case
where a plain wildcard with no closer empty non-terminal still applies.
Signed-off-by: Omkhar Arasaratnam <omkhar@linkedin.com>
Co-authored-by: Omkhar Arasaratnam <omkhar@linkedin.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Several tests slept a fixed duration then took a single snapshot of an
async result, racing whatever they waited on:
- forward health tests waited 20ms for the health-check goroutine to bump
an atomic counter,
- the auto plugin tests (dns + metrics) waited 50-110ms for a file-watch
reload to be picked up,
- the file ZoneReload test waited 30ms (self-described as could still be
racy) for a reload,
- the overloaded health test slept 1s for its background goroutine to fire
its first request.
Replace each with a bounded poll of the actual condition (the atomic
counter, a dns.Exchange response, a metrics scrape, z.ApexIfDefined, or a
channel signalled by the test's own handler) so they pass as soon as the
awaited state is reached and no longer flake when it is slower than the
fixed wait. Test-only.
Signed-off-by: Nikolaus Schuetz <nikolauspschuetz@gmail.com>
