Files
fancy-dns/internal/server/config_test.go
T
developer 9881d2a095 Make the API response contract a test; fix what it found
The console's TypeScript types are hand-written and apiFetch ends in
`JSON.parse(text) as T`, so nothing in the toolchain compared them with the
Go structs. A field that lost its json tag, or a response a struct never had,
showed up as an empty table in a browser and nowhere else: the Go suite was
green and the console was wrong. internal/app/contract_test.go now asserts the
exact JSON key set of every admin endpoint, driven through the real router
with the real hook implementations, plus a test that drives the two write
endpoints with the exact payload web/src builds.

The drift it found, all four fixed here:

blocklist.Source and SourceMeta had no json tags at all, so GET /api/blocklists
sent Go field names ("URL"/"Format") where the console expects snake_case: the
Sources table rendered blank rows and its per-row toggle posted
`name: undefined`. Tagged to match ParseStats.

acl.GroupSummary was flat where the console reads a nested `match` object — the
same shape config.Group has — so every group card read "matches nothing". The
four match fields are nested now, under a new acl.Match.

PUT /api/clients answered 400 `json: unknown field "ptr_suffixes"` to the
console's payload, because config.Group and config.Match carried yaml tags only
and the decoder runs with DisallowUnknownFields. Both structs have json tags
now; the same applies to config.UpstreamServer and UpstreamBootstrap.

PUT /api/upstreams answered 400 `json: unknown field "health"`, because the
console round-trips the whole server object it read, runtime health fields
included. They are accepted and ignored, because the server owns them, and they
are `yaml:"-"` so settings.yaml does not gain runtime state.

Also fixed, each with a test:

A duration on the wire was a nanosecond count. config.Duration had a
MarshalYAML but no MarshalJSON, so bootstraps[].refresh arrived as 30000000000
while the console typed the field as a string, and healthcheck_interval was a
float of seconds in one place and a string in another. Durations on the wire
are duration strings everywhere now — the same spelling config.yaml uses — and a
bare JSON number is still accepted and means seconds, not nanoseconds.
TestDurationJSONForm used to pin the old behaviour and now pins the new one,
saying in its comment that the old behaviour was the bug.

A log row's key set depended on the row. api.LogEntry was omitempty on
client_group, response, upstream and list, so a query with no ACL group
arrived without the key and the console rendered "undefined" where the column
should be empty. The contract test can only pin one shape if the server emits
one, so nothing on that struct is omitempty any more.

A dead peer stayed in the cluster peer list forever. Cluster.seen was only ever
added to and deleted from on an explicit `bye`, so a node that crashed — or
whose Valkey connection died — was reported as a live peer for the life of the
process, and its stale blocklist digest was flagged as divergence forever.
Peers() now reaps peers unheard for peer_ttl*4 and stamps each state with the
receiver's own clock, so clock skew between two machines can neither reap a
healthy peer nor keep a dead one. Discover() ages out the same way on the
publisher's own last_seen stamp, because the keyed Valkey record outlives a
crash: a live node keeps refreshing the key's TTL, so a dead node's field stayed
readable for the life of the fleet.

peer_ttl: 0 would have crashed the process. time.NewTicker(PeerTTL/2) panics on
a non-positive period and `EXPIRE key 0` deletes the heartbeat key, so a config
that left peer_ttl unset would have taken the process down at Start, or made
every instance believe it is alone in the fleet. Both go through peerTTL() and
peerDeadline(), which fall back to the default.

Two cluster tests raced on the wire, not on the product, and one was
environment-dependent rather than wrong. A node that joined after another's
heartbeat was in flight hears about it on the next tick, so "b does not see a"
read immediately after "a sees b" is a coin flip; the divergence test waited for
a digest on the wrong node's view of the fleet; and 127.0.0.1:1 is only
unbindable outside a container that sets
net.ipv4.ip_unprivileged_port_start=0, where it binds and the partial-bind test
fails with both transports up. That test now occupies a port with a live
listener in the test process, which is a deterministic EADDRINUSE on every
machine and every uid.

internal/server/counters_test.go covers the counter-key layer that no other
test touched: the "<transport>.<field>" keyspace, that the aggregate folds only
the known fields, and that the snapshot is safe while counters move.

Two pinned behaviours changed on purpose, each with a test that says so:

An RPZ record did not cover the name it names. rpzRule gave a bare owner name
the subdomain form (*.name), so analytics.example.org. in a policy zone blocked
cdn.analytics.example.org. and left analytics.example.org. resolving normally —
and an rpz-passthru. record did not allowlist its own domain either. RPZ
semantics say the record applies to the domain and everything under it, so a
bare owner now takes the apex form **.name; an owner written with an explicit *.
is still strict, so a zone that wants subdomain-only behaviour can still ask for
it. This changes which queries get blocked for an existing policy zone, which is
why it wanted a decision. TestExampleRPZZoneCoversTheApexItNames pins both
halves and the example zone now says which half it relies on.

metrics.OutcomeLocal had no producer. The engine reported a local-zone answer as
qlog.ActionAllow, which outcomeFor mapped to OutcomeUpstream, so every locally
answered query was counted as an upstream one and the outcome="local" series was
never published — the dashboard's local column was permanently empty.
Response.Local is now set on the local-zone path and outcomeFor consults it
first. That moves published counters, so the README's metrics section says so
too.
2026-09-27 18:42:59 +00:00

396 lines
15 KiB
Go

package server
import (
"context"
"encoding/base64"
"io"
"log/slog"
"net"
"net/http"
"strings"
"testing"
"github.com/miekg/dns"
"git.k8s-git-k6n.net/mats/fancy-dns/internal/config"
)
// freeTCPAddr returns a loopback address that was free a moment ago, so a
// test can name the port a listener must NOT have taken.
func freeTCPAddr(t *testing.T) string {
t.Helper()
ln, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("probe listen: %v", err)
}
addr := ln.Addr().String()
if err := ln.Close(); err != nil {
t.Fatalf("probe close: %v", err)
}
return addr
}
// TestDisabledListenerIsNotStartedAndBindsNothing is the "configured off" half
// of the per-listener contract. A disabled transport must not appear in the
// snapshot the admin API serves, and — the part Listeners() alone cannot show —
// it must not have taken its port. An operator who turned DoT off because the
// certificate was broken must be able to bind that port themselves.
func TestDisabledListenerIsNotStartedAndBindsNothing(t *testing.T) {
doTAddr := freeTCPAddr(t)
doHAddr := freeTCPAddr(t)
h := newHarness(t, func(c *config.Config) {
c.DNS.DoT = config.DoT{Enabled: false, Listen: []string{doTAddr}}
c.DNS.DoH = config.DoH{Enabled: false, Listen: []string{doHAddr}, BasePath: "/dns-query", AllowPost: true}
})
for _, tr := range h.transports() {
if tr == "dot" || tr == "doh" {
t.Errorf("disabled listener %q appears in %v", tr, h.srv.Listeners())
}
}
if got := h.transports(); len(got) != 2 {
t.Errorf("transports = %v, want just udp and tcp", got)
}
for _, addr := range []string{doTAddr, doHAddr} {
ln, err := net.Listen("tcp", addr)
if err != nil {
t.Errorf("port %s is not free after Start with the listener disabled: %v", addr, err)
continue
}
_ = ln.Close()
}
}
// TestEnabledDoTWithoutCertIsAFailure covers the misconfiguration the disabled
// case hides: DoT switched on with no certificate cannot serve TLS, and the
// server says so instead of binding a port that answers nothing.
func TestEnabledDoTWithoutCertIsAFailure(t *testing.T) {
cfg := config.Default()
cfg.DNS.UDP = nil
cfg.DNS.TCP = nil
cfg.DNS.DoT = config.DoT{Enabled: true, Listen: []string{"127.0.0.1:0"}}
cfg.DNS.DoH = config.DoH{Enabled: false}
srv := New(Options{Engine: newFakeHandler(), Config: config.NewStore(cfg, ""), Logger: discardLogger()})
err := srv.Start(context.Background())
if err == nil {
_ = srv.Stop(context.Background())
t.Fatal("Start succeeded with DoT enabled and no certificate")
}
t.Cleanup(func() { _ = srv.Stop(context.Background()) })
if got := srv.Listeners(); len(got) != 0 {
t.Errorf("Listeners() = %v, want empty", got)
}
}
// TestStartReportsAPartialFailureButServes pins the two halves of the Start
// contract at once: a listener that cannot bind is an error worth returning,
// but it must not take the whole process down when another listener is fine.
// The admin API treats an empty listener list as degraded (app/hooks.go), so
// serving on the survivors is the difference between one broken transport and
// an outage.
func TestStartReportsAPartialFailureButServes(t *testing.T) {
// A port already held by a listener in this test process is the
// deterministic bind failure. "127.0.0.1:1" looks more convincing but is
// not: an unprivileged process cannot bind it *by default*, and plenty of
// container runtimes set net.ipv4.ip_unprivileged_port_start=0, where it
// succeeds and the test passes for the wrong reason — or fails, with the
// only signal being that both transports came up.
blocker, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("occupy a port: %v", err)
}
t.Cleanup(func() { _ = blocker.Close() })
cfg := config.Default()
cfg.DNS.UDP = []string{"127.0.0.1:0"}
cfg.DNS.TCP = []string{blocker.Addr().String()}
cfg.DNS.DoT = config.DoT{Enabled: false}
cfg.DNS.DoH = config.DoH{Enabled: false}
srv := New(Options{Engine: newFakeHandler(), Config: config.NewStore(cfg, ""), Logger: discardLogger()})
if err := srv.Start(context.Background()); err != nil {
t.Fatalf("Start: %v", err)
}
t.Cleanup(func() { _ = srv.Stop(context.Background()) })
got := []string{}
for _, l := range srv.Listeners() {
got = append(got, l.Transport)
}
if len(got) != 1 || got[0] != "udp" {
t.Errorf("transports = %v, want just udp", got)
}
// The failing transport is not in the snapshot at all: ListenerInfo has an
// Err field, but a listener that never bound is never added, so the API
// cannot show an operator WHICH address failed. Pinned as current
// behaviour — see the report.
for _, l := range srv.Listeners() {
if l.Err != "" {
t.Errorf("listener %s carries Err %q; a failed bind is expected to be absent, not reported", l.Transport, l.Err)
}
}
}
// TestStartTwiceIsRejected: a second Start must not re-bind and double every
// counter, and must say so rather than silently doing nothing useful.
func TestStartTwiceIsRejected(t *testing.T) {
h := newHarness(t, nil)
if err := h.srv.Start(context.Background()); err == nil {
t.Fatal("second Start succeeded, want an error")
}
}
// TestDoHBasePathIsServedAtTheConfiguredPath: the DoH path is operator
// configuration, and a request to any other path must 404 rather than being
// answered — an Ingress path or a stale client that keeps using /dns-query
// needs to fail loudly.
func TestDoHBasePathIsServedAtTheConfiguredPath(t *testing.T) {
h := newHarness(t, func(c *config.Config) { c.DNS.DoH.BasePath = "/query" })
cl := h.dohClient(false)
// Configured path answers.
if res := h.askDoHGET(question("base.test.", dns.TypeA)); answerAddr(res) != "203.0.113.9" {
t.Errorf("A over the configured base path = %q", answerAddr(res))
}
// RFC 8484 §4.1 alias still works when the base path is not the default.
res, _ := dohGetAt(cl, h, h.addr("doh"), "/.well-known/dns-query", question("alias.test.", dns.TypeA))
if res != http.StatusOK {
t.Errorf("GET /.well-known/dns-query = %d, want 200", res)
}
// Anything else 404s and never reaches the engine.
for _, path := range []string{"/dns-query", "/query/extra", "/"} {
res, _ := dohGetAt(cl, h, h.addr("doh"), path, question("nope.test.", dns.TypeA))
if res != http.StatusNotFound {
t.Errorf("GET %s = %d, want 404", path, res)
}
}
if n := len(h.fake.seen()); n != 2 {
t.Errorf("engine saw %d requests, want 2 (the configured path and the alias)", n)
}
}
// TestDoHRejectsPostWhenNotAllowed: allow_post exists because some networks
// block POST. Turning it off must reject POST with the documented status and
// leave GET working, or the operator has turned DoH off entirely.
func TestDoHRejectsPostWhenNotAllowed(t *testing.T) {
h := newHarness(t, func(c *config.Config) { c.DNS.DoH.AllowPost = false })
res, body := h.doHPOST(question("post.test.", dns.TypeA))
if res.StatusCode != http.StatusMethodNotAllowed {
t.Errorf("POST with allow_post=false = %d, want 405 (%s)", res.StatusCode, body)
}
if allow := res.Header.Get("Allow"); allow != "GET" {
t.Errorf("Allow = %q, want GET", allow)
}
if n := len(h.fake.seen()); n != 0 {
t.Errorf("rejected POST reached the engine %d times", n)
}
if res := h.askDoHGET(question("post.test.", dns.TypeA)); answerAddr(res) != "203.0.113.9" {
t.Errorf("GET stopped working when POST was disabled: %q", answerAddr(res))
}
}
// TestDoHPostRequiresTheRFC8484ContentType: a POST whose body is a DNS message
// but whose media type is something else is not RFC 8484, and answering it
// would let a browser (which can post anything) reach the resolver.
func TestDoHPostRequiresTheRFC8484ContentType(t *testing.T) {
h := newHarness(t, nil)
res, body := h.doHPostWith(question("ct.test.", dns.TypeA), "application/octet-stream")
if res.StatusCode != http.StatusUnsupportedMediaType {
t.Errorf("POST with the wrong content type = %d, want 415 (%s)", res.StatusCode, body)
}
if n := len(h.fake.seen()); n != 0 {
t.Errorf("wrong-content-type POST reached the engine %d times", n)
}
}
// TestDoHRejectsNonQueryMethods: the handler is not a general HTTP endpoint.
func TestDoHRejectsNonQueryMethods(t *testing.T) {
h := newHarness(t, nil)
for _, method := range []string{http.MethodPut, http.MethodDelete, http.MethodHead} {
req, err := http.NewRequest(method, h.dohURL(h.base()), nil)
if err != nil {
t.Fatalf("%s request: %v", method, err)
}
res, err := h.dohClient(false).Do(req)
if err != nil {
t.Fatalf("%s: %v", method, err)
}
_ = res.Body.Close()
if res.StatusCode != http.StatusMethodNotAllowed {
t.Errorf("%s = %d, want 405", method, res.StatusCode)
}
if allow := res.Header.Get("Allow"); allow != "GET, POST" {
t.Errorf("%s Allow = %q, want \"GET, POST\"", method, allow)
}
}
}
// TestDoHHidesVersionUnlessAsked pins cfgHideIdentity. The Server header is the
// only thing this switches off; the version string is an attacker's first
// question, and an operator who set hide_identity expects it gone from the
// encrypted listener too.
func TestDoHHidesVersionUnlessAsked(t *testing.T) {
h := newHarness(t, func(c *config.Config) { c.DNS.HideIdentity = true })
res, _ := h.doHGET(question("hide.test.", dns.TypeA))
if got := res.Header.Get("Server"); got != "" {
t.Errorf("Server header = %q, want it hidden", got)
}
if res := h.askDoHPOST(question("hide.test.", dns.TypeA)); answerAddr(res) != "203.0.113.9" {
t.Errorf("hide_identity broke the POST path: %q", answerAddr(res))
}
h2 := newHarness(t, nil)
res2, _ := h2.doHGET(question("show.test.", dns.TypeA))
if got, want := res2.Header.Get("Server"), "fancy-dns/"+testVersion; got != want {
t.Errorf("Server header = %q, want %q", got, want)
}
}
// TestDoHHideIdentityIsReadPerRequest: the flag lives in the hot-reloadable
// store, so turning it on must take effect without a restart. It is read on
// every request (cfgHideIdentity calls cfg.Get()), which is what makes the
// reload meaningful — and it is worth pinning, because freezing it at New
// would be an invisible regression.
func TestDoHHideIdentityIsReadPerRequest(t *testing.T) {
h := newHarness(t, nil)
res, _ := h.doHGET(question("reload.test.", dns.TypeA))
if res.Header.Get("Server") == "" {
t.Fatal("version header missing before the reload")
}
next := *h.cfg
next.DNS.HideIdentity = true
if err := h.store.Swap(&next); err != nil {
t.Fatalf("swap: %v", err)
}
res, _ = h.doHGET(question("reload.test.", dns.TypeA))
if got := res.Header.Get("Server"); got != "" {
t.Errorf("Server header = %q after hide_identity, want hidden", got)
}
}
// TestDoHDropStillGetsABody: a dropped query must not hang the HTTP exchange.
// Unlike UDP, where silence is the answer, a DoH client waiting on a socket
// reads a missing body as a transport error and retries against a resolver
// that is working exactly as configured.
func TestDoHDropStillGetsABody(t *testing.T) {
h := newHarness(t, nil)
res, body := h.doHPOST(question("drop.test.", dns.TypeA))
if res.StatusCode != http.StatusOK {
t.Fatalf("status = %d, want 200: %s", res.StatusCode, body)
}
m := unpackDoH(t, body)
if len(m.Answer) != 0 {
t.Errorf("dropped query came back with %d answers: %v", len(m.Answer), m.Answer)
}
if got := h.fake.only(t); got.name != "drop.test." {
t.Errorf("name = %q, want drop.test.", got.name)
}
}
// TestDoHMalformedRequestsAreRejected: every one of these is a 400/404 that
// must not reach the engine, because a request that cannot be understood is
// also a request the rate limiter and the query log should not see.
func TestDoHMalformedRequestsAreRejected(t *testing.T) {
h := newHarness(t, nil)
cl := h.dohClient(false)
addr := h.addr("doh")
base := h.base()
post := func(body string, ct string) int {
res, err := cl.Post("https://"+addr+base, ct, stringReader(body))
if err != nil {
t.Fatalf("POST: %v", err)
}
_ = res.Body.Close()
return res.StatusCode
}
cases := []struct {
name string
run func() int
want int
}{
{"no dns parameter", func() int {
res, _ := dohRawGet(cl, "https://"+addr+base+"?name=example.com")
return res
}, http.StatusBadRequest},
{"unknown parameter", func() int {
res, _ := dohRawGet(cl, "https://"+addr+base+"?dns=AAAB&extra=1")
return res
}, http.StatusBadRequest},
{"malformed base64", func() int {
res, _ := dohRawGet(cl, "https://"+addr+base+"?dns="+"not base64!!")
return res
}, http.StatusBadRequest},
{"not a DNS message", func() int {
res, _ := dohRawGet(cl, "https://"+addr+base+"?dns=YWJjZGVmZ2g")
return res
}, http.StatusBadRequest},
{"empty body", func() int { return post("", contentTypeDNS) }, http.StatusBadRequest},
{"garbage body", func() int { return post("this is not a DNS message", contentTypeDNS) }, http.StatusBadRequest},
{"header with no question", func() int {
// A well-formed DNS header with qdcount 0: parseable, unanswerable.
return post(string([]byte{0x12, 0x34, 0x01, 0x00, 0, 0, 0, 0, 0, 0, 0, 0}), contentTypeDNS)
}, http.StatusBadRequest},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := tc.run(); got != tc.want {
t.Errorf("status = %d, want %d", got, tc.want)
}
})
}
if n := len(h.fake.seen()); n != 0 {
t.Errorf("a malformed request reached the engine: %+v", h.fake.seen()[:n])
}
}
// TestDoHBasePathCannotBeUsedForOtherPaths: a base path of "/" is what an
// operator writes to "serve DoH at the root". It answers every path, which is
// why healthz is registered explicitly and why this is worth knowing.
func TestDoHBasePathCannotBeUsedForOtherPaths(t *testing.T) {
h := newHarness(t, func(c *config.Config) { c.DNS.DoH.BasePath = "/" })
if h.base() != "/" {
t.Fatalf("base = %q, want /", h.base())
}
res, body := h.doHGET(question("root.test.", dns.TypeA))
if res.StatusCode != http.StatusOK {
t.Fatalf("status = %d, want 200: %s", res.StatusCode, body)
}
// /healthz is registered ahead of the catch-all, so it still works.
hres, err := h.dohClient(false).Get(h.dohURL("/healthz"))
if err != nil {
t.Fatalf("healthz: %v", err)
}
_ = hres.Body.Close()
if hres.StatusCode != http.StatusOK {
t.Errorf("healthz = %d, want 200", hres.StatusCode)
}
}
// dohGetAt performs a GET against an arbitrary path on the DoH listener, so a
// test can check a path the server is not supposed to serve.
func dohGetAt(cl *http.Client, _ *harness, addr, path string, m *dns.Msg) (int, []byte) {
wire, err := m.Pack()
if err != nil {
return 0, nil
}
res, body := dohRawGet(cl, "https://"+addr+path+"?dns="+base64.RawURLEncoding.EncodeToString(wire))
return res, body
}
// dohRawGet issues a GET and returns the status and body.
func dohRawGet(cl *http.Client, url string) (int, []byte) {
res, err := cl.Get(url)
if err != nil {
return 0, nil
}
defer res.Body.Close()
body, _ := io.ReadAll(res.Body)
return res.StatusCode, body
}
// stringReader keeps the POST bodies in the malformed-request table readable.
func stringReader(s string) io.Reader { return strings.NewReader(s) }
// discardLogger keeps the package's log output out of the test run.
func discardLogger() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) }