remapStringRewriter matches a record name against orig and its sub domains. The
sub domain check was strings.HasSuffix(src, "."+r.orig), which built the
dot-prefixed string on every call and threw it away. Match the label boundary by
index instead: src is a sub domain of orig when orig sits at the end of src with
a "." immediately before it, which is exactly what the HasSuffix call tested.
Go concatenates short strings into a 32-byte stack buffer, so "."+orig only
reached the heap once orig passed 31 bytes. Below that the temporary was free
and this saves a few ns per record. Above it, 48 B was allocated per record.
Kubernetes service names are past the threshold -
my-service.my-namespace.svc.cluster.local. is 42 bytes - and those are the names
an auto rule rewrites to when a Corefile maps an external name onto an
in-cluster one. orig is the name the question was rewritten to, so whether a
deployment sees the allocation is a property of its Corefile, not its queries.
Caching "."+orig on the rewriter instead does not work: responseRuleFor
constructs a new rewriter for every request an auto name rule rewrites, so the
concatenation would run once per request rather than once per rule, and escapes
to the heap from there. That buys per-record work with a per-request allocation
and regresses every response short enough not to amortize it.
Per record, one rewriteString call on an existing rewriter:
name master this PR
RemapStringRewriter/short/match 186.6n 16 B/1 120.4n 16 B/1 -35%
RemapStringRewriter/short/nomatch 61.5n 0 B/0 16.5n 0 B/0 -73%
RemapStringRewriter/long/match 381.9n 72 B/2 165.3n 24 B/1 -57%
RemapStringRewriter/long/nomatch 219.1n 48 B/1 18.7n 0 B/0 -91%
Per request - rewrite the question, build the response rules, apply them to the
answer:
name master this PR
AutoNameRuleResponse/exact 935n 96 B/4 945n 96 B/4 ~
AutoNameRuleResponse/subdomain 1.248µ 112 B/5 1.242µ 112 B/5 ~
AutoNameRuleResponse/nomatch 1.016µ 96 B/4 945n 96 B/4 -7%
AutoNameRuleResponse/subdomain-8 3.376µ 224 B/12 2.891µ 224 B/12 -14%
AutoNameRuleResponse/k8s/subdomain 1.611µ 176 B/6 1.380µ 128 B/5 -14%
AutoNameRuleResponse/k8s/subdomain-8 5.144µ 672 B/20 3.305µ 288 B/12 -36%
benchstat over 8 runs, i7-1065G7. With short names this is flat at the request
level: one rewriteString call is small next to the four allocations that
building the rules costs. The saving is per record and per byte of name, so it
shows up where responses carry several records and the rewritten-to name is
long.
This applies to exact, prefix, substring and regex name rules with answer auto.
suffix rules build a suffixStringRewriter instead and are not affected.
TestRemapStringRewriter pins the label boundary semantics the index arithmetic
now carries, notably that notexample.com. is not a sub domain of example.com.
It passes against the previous implementation too.
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
* 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>
Kubernetes rejects a pod at admission when runAsNonRoot is set and the image
declares its user by name: "container has runAsNonRoot and image has
non-numeric user (nonroot), cannot verify user is non-root". The Dockerfile
set `USER nonroot:nonroot`, so the built image's config user was the name
rather than a uid, which the kubelet cannot verify.
distroless "nonroot" is uid/gid 65532, so pin the numeric id. The image runs
as the same user and the binary keeps cap_net_bind_service from the build
stage, so it can still bind :53. Verified by building the image: config user
is now `65532:65532`.
Fixes#7542
Signed-off-by: mehrdadbn9 <mehrdadbiukian@gmail.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>
