atenet/dns: answer non-A actor queries instead of SERVFAIL - #874
atenet/dns: answer non-A actor queries instead of SERVFAIL#874Yuan Gao (ygao-g) wants to merge 3 commits into
Conversation
7a240f7 to
0272dc0
Compare
|
Bowei Du (@bowei) Julian Gutierrez Oschmann (@juli4n) mind taking a look? This is the SERVFAIL half of #246, step one of your three: the actor zone can't Julian Gutierrez Oschmann (@juli4n), the substance is three Verified against |
f4bc2e5 to
fd6e47c
Compare
|
Yuan -- it would be good to be very specific on what we are doing in this PR. From what I can tell:
Is this what you are trying to do? |
|
Close, but that's two PRs and this is the smaller one. This PR doesn't publish an AAAA. It makes the zone return a correct rcode for what it doesn't answer — NODATA for a real actor name on a non-A qtype, NXDOMAIN for a name in the zone that doesn't exist. Both SERVFAIL today, on IPv4-only clusters too, which is why musl-based actors can't resolve each other at all: #888. Publishing AAAA is #938. The three together:
Every actor name resolves to the same address — the router ClusterIP, with per-actor demux at Envoy on the Host header — so an AAAA for a Substrate name is those three things, and no one of them is useful alone. End state on dual-stack: an actor name resolves in both families and Envoy answers on either. Each PR carries its own verification; the real gap is that dual-stack isn't testable until #877 lands, so #911's dual-stack behaviour is argued rather than run. On "fix any associated things" — past DNS, three more sit between a published AAAA and a usable IPv6 path, now filed as step-3 sub-tasks: #943, #944, #945. |
1634ab3 to
f8b1782
Compare
Yuan Gao (ygao-g)
changed the title
482ec1d to
9cf8384
Compare
9cf8384 to
fd76e7b
Compare
Yuan Gao (ygao-g)
changed the title
8c2a75e to
8eaf242
Compare
Not intended to merge. The unit test in agent-substrate#874 pins the rendered zone but nothing exercises CoreDNS, so this serves that zone with the pinned coredns/coredns:1.11.1 and checks the rcode returned for A, AAAA, HTTPS, SRV, a name in the zone that is not an actor, and a malformed one, plus an Alpine getent for the musl path. Pointing --corefile at a zone rendered before the fix turns the run into a negative control: the AAAA case SERVFAILs and getent stops resolving, while plain dig A still succeeds.
|
haiyanmeng Bowei Du (@bowei) PTAL when you have time. Thanks! |
haiyanmeng
left a comment
haiyanmeng
left a comment
There was a problem hiding this comment.
LGTM
Left a comment regarding the test cvoerage
The zone answered A queries and failed everything else -- AAAA for a valid actor, and any name in the zone that is not an actor -- and no test caught it, because Go's resolver masks a SERVFAIL that musl treats as fatal. These assert the rcode class rather than the record: a non-A qtype and a name that misses the actor regex must come back NODATA or NXDOMAIN, and an A query must carry the router's ClusterIP. A separate test covers the AAAA record, skipped where the router has no v6 address. Second of two commits. The assertions are red until agent-substrate#874 and agent-substrate#938 land, so this stays a draft until then. Part of agent-substrate#246.
Before, the actor zone answered A queries and failed everything else -- AAAA for a valid actor, and any name in the zone that is not an actor. A failure reads as a temporary error rather than an answer, so clients retry it and then give up on the name; Alpine actors could not resolve each other at all, even on an IPv4-only cluster. After, those queries return a correct empty answer, and one that resolvers can cache. A unit test pins the whole rendered zone as a literal, so editing the name pattern or the suffix fails there rather than passing silently.
Nothing in the e2e harness could query the actor DNS zone. Suites reach actors by port-forwarding atenet-router and passing the actor name as a Host header, so the zone CoreDNS actually serves went unasserted, and a suite that wanted to check it had no way to distinguish an empty answer from a server failure. Adds a DNS client that port-forwards the atenet DNS Service and reports the rcode class alongside the addresses, plus a helper for the router's ClusterIP in each family. The tests that use these follow. clusterIPsByFamily here is a stopgap that agent-substrate#938 replaces with internal/ipfamily.
The zone answered A queries and failed everything else -- AAAA for a valid actor, and any name in the zone that is not an actor -- and no test caught it, because Go's resolver masks a SERVFAIL that musl treats as fatal. These assert the rcode class rather than the record: a non-A qtype and a name that misses the actor regex must come back NODATA or NXDOMAIN, and an A query must carry the router's ClusterIP. Both rcode assertions fail on a tree without the first commit here and pass with it, measured on a single-stack IPv4 kind cluster. Part of agent-substrate#246.
The zone answered A queries and failed everything else -- AAAA for a valid actor, and any name in the zone that is not an actor -- and no test caught it, because Go's resolver masks a SERVFAIL that musl treats as fatal. These assert the rcode class rather than the record: a non-A qtype and a name that misses the actor regex must come back NODATA or NXDOMAIN, and an A query must carry the router's ClusterIP. A separate test covers the AAAA record, skipped where the router has no v6 address. Second of two commits. The assertions are red until agent-substrate#874 and agent-substrate#938 land, so this stays a draft until then. Part of agent-substrate#246.
8eaf242 to
dca53af
Compare
3 participants
Fixes #888
The actor DNS zone only answered
A; everything else —AAAAfor a validactor, and any other name in the zone — got SERVFAIL. Strict resolvers treat
that as a hard network error rather than an empty answer, and musl maps it to
EAI_AGAIN, sinking the pairedAquery with it, so Alpine actors could notresolve each other even on an IPv4-only cluster.
Valid actor names now return NODATA (NOERROR + SOA) for non-A types, and
anything else in the zone returns NXDOMAIN — both negatively cacheable. This
does not publish an
AAAArecord; that is #938.The last two commits cover this end to end. The harness could not query the
zone at all before them: every suite reaches an actor by port-forwarding
atenet-router with a
Hostheader, so what CoreDNS serves went unasserted.