From 4f49033f1b332ec53ef2bad6c9469f58c42d0436 Mon Sep 17 00:00:00 2001 From: developer Date: Sun, 27 Sep 2026 16:03:21 +0000 Subject: [PATCH] Cover the engine, config.Size and metrics; fix the five bugs they found internal/engine, internal/config and internal/metrics had no test file at all, and writing them turned up five real bugs. internal/engine (23 tests, new harness_test.go): a wired engine over a fake upstream on loopback, a local zone, a blocklist, a cache, a query log and an injectable clock. It pins the pipeline order end to end -- ACL deny before the local zone, local zone before blocklist, blocklist before cache, drop sends nothing, the concurrency and rate-limit refusals, serve-stale served with TTL 1 and refreshed once per key, truncation against a 512-byte client, and exactly one metrics record and one query-log entry per query. Five fixes: - config.ParseSize ignored the unit multiplier, so 512MiB parsed as 512 bytes and the whole memory envelope was inert for anyone who wrote a unit. - finalize (EDNS0 and the UDP size) ran only on the cache-hit path, so a large uncached answer left oversized for a 512-byte client. It now runs on the upstream, local-zone and blocklist paths too. - The upstream UDP client read buffer was hard-coded to 1232 while we advertise dns.max_udp_size, so with a larger buffer configured every answer above 1232 bytes was read truncated and became SERVFAIL. The pool takes the size now. - LRU.prefetchGate was a 64-slot global cap, not the single flight its comment promised: a burst of stale hits launched up to 64 duplicate refreshes of one name. It is a per-key latch now; distinct names still refresh in parallel. - metrics.RecordMiss and RecordStaleHit moved only the Prometheus collector, so the Cache page's misses and stale hits read 0 for the life of the process. AGENTS.md gains the trap that cost the most time: this version of miekg/dns reports a character-string over 255 bytes as zero length, so such a message silently fails to pack, and the write failure looks like a client timeout. --- AGENTS.md | 7 + internal/app/app.go | 2 +- internal/cache/lru.go | 48 +- internal/cache/manager.go | 4 +- internal/config/size.go | 15 +- internal/config/size_test.go | 214 +++++++++ internal/engine/engine.go | 14 +- internal/engine/engine_test.go | 790 +++++++++++++++++++++++++++++++ internal/engine/harness_test.go | 495 +++++++++++++++++++ internal/metrics/metrics.go | 8 +- internal/metrics/metrics_test.go | 250 ++++++++++ internal/resolver/pool.go | 27 +- 12 files changed, 1837 insertions(+), 37 deletions(-) create mode 100644 internal/config/size_test.go create mode 100644 internal/engine/engine_test.go create mode 100644 internal/engine/harness_test.go create mode 100644 internal/metrics/metrics_test.go diff --git a/AGENTS.md b/AGENTS.md index 0f7900e..c49be4b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -144,6 +144,13 @@ partial result described as done is not. of the published ones" is a real property and cannot flake. - Spin up `dns.Server` fakes on `127.0.0.1:0`: `net.Listen`/`net.ListenPacket` to find the port, `ActivateAndServe` in a goroutine, stop in `t.Cleanup`. +- A `dns.Msg` carrying a character-string over 255 bytes **does not pack** in + miekg/dns v1.1.73: `TXT.len()` reports such a string as zero length and + `EDNS0_NSID.len()` is likewise wrong, so `Pack` fails with "dns: overflow + packing opt". On a fake upstream's write path that failure is silent and + presents as a client timeout or a SERVFAIL several layers away. Build a + multi-kilobyte answer out of many strings under 255 bytes, and assert the + fake's write error before blaming the client. - `internal/app/integration_test.go` already builds a fully wired app. Reuse its harness rather than rebuilding one. - `tsc -b` replays **cached diagnostics** from `node_modules/.tmp/*.tsbuildinfo`. diff --git a/internal/app/app.go b/internal/app/app.go index 74e24ca..f81f222 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -344,7 +344,7 @@ func (a *App) reloadLocalZones() { // startUpstream builds the resolver pool. func (a *App) startUpstream(cfg *config.Config) { - a.pool = resolver.NewPool(cfg.Upstream) + a.pool = resolver.NewPoolWithUDPSize(cfg.Upstream, cfg.DNS.MaxUDPSize) } // startDNS builds the engine and the listeners. diff --git a/internal/cache/lru.go b/internal/cache/lru.go index 68de437..1248c11 100644 --- a/internal/cache/lru.go +++ b/internal/cache/lru.go @@ -46,9 +46,11 @@ type LRU struct { seed maphash.Seed - // prefetch delays revalidation without blocking Get. It is a semaphore of - // "refreshes in flight" so a burst of stale hits triggers one fetch. - prefetchGate chan struct{} + // prefetch delays revalidation without blocking Get. It holds the keys + // with a refresh in flight, so a burst of stale hits for one name starts + // one fetch while unrelated names still refresh in parallel. + prefetchMu sync.Mutex + prefetching map[Key]struct{} } type shard struct { @@ -88,10 +90,10 @@ func NewLRU(size, shards int) *LRU { size = shards } l := &LRU{ - shards: make([]*shard, shards), - mask: uint64(shards - 1), - seed: maphash.MakeSeed(), - prefetchGate: make(chan struct{}, 64), + shards: make([]*shard, shards), + mask: uint64(shards - 1), + seed: maphash.MakeSeed(), + prefetching: make(map[Key]struct{}), } l.perShard.Store(int64(max(1, size/shards))) for i := range l.shards { @@ -286,23 +288,29 @@ func (l *LRU) DeleteWhere(pred func(Key, *Entry) bool) int { return removed } -// TryPrefetch acquires a refresh slot without blocking. Callers use it to -// make sure a burst of stale hits triggers exactly one background refresh. -func (l *LRU) TryPrefetch() bool { - select { - case l.prefetchGate <- struct{}{}: - return true - default: +// TryPrefetch claims the refresh slot for one key without blocking. It +// reports false when a refresh for that key is already in flight, which is +// what makes a burst of stale hits for a hot name cost one upstream exchange +// instead of one per query. +// +// The latch is per key, not global: a single global slot would serialise +// recovery after an upstream outage, when thousands of unrelated stale names +// each want a refresh. +func (l *LRU) TryPrefetch(k Key) bool { + l.prefetchMu.Lock() + defer l.prefetchMu.Unlock() + if _, busy := l.prefetching[k]; busy { return false } + l.prefetching[k] = struct{}{} + return true } -// ReleasePrefetch returns a slot taken by TryPrefetch. -func (l *LRU) ReleasePrefetch() { - select { - case <-l.prefetchGate: - default: - } +// ReleasePrefetch frees the slot taken by TryPrefetch for k. +func (l *LRU) ReleasePrefetch(k Key) { + l.prefetchMu.Lock() + delete(l.prefetching, k) + l.prefetchMu.Unlock() } // HitKind describes how a lookup was satisfied. diff --git a/internal/cache/manager.go b/internal/cache/manager.go index ba98c3b..843f7ef 100644 --- a/internal/cache/manager.go +++ b/internal/cache/manager.go @@ -153,10 +153,10 @@ func (m *Manager) Get(k Key) Result { } if kind == Stale { res.TTL = 0 - if p := m.prefetchFn.Load(); p != nil && m.opt.Prefetch && m.lru.TryPrefetch() { + if p := m.prefetchFn.Load(); p != nil && m.opt.Prefetch && m.lru.TryPrefetch(k) { fn := *p go func() { - defer m.lru.ReleasePrefetch() + defer m.lru.ReleasePrefetch(k) fn(k) }() } diff --git a/internal/config/size.go b/internal/config/size.go index 95ecbea..1c0b9d3 100644 --- a/internal/config/size.go +++ b/internal/config/size.go @@ -56,23 +56,28 @@ func ParseSize(s string) (Size, error) { if err != nil { return 0, fmt.Errorf("invalid size %q", s) } - return scaleSize(n, s) + return scaleSize(n, u.mult, s) } n, err := strconv.ParseFloat(raw, 64) if err != nil { return 0, fmt.Errorf("invalid size %q", s) } - return scaleSize(n, s) + return scaleSize(n, 1, s) } -func scaleSize(n float64, orig string) (Size, error) { +// scaleSize applies the unit multiplier. A bare number has mult 1, which is +// why the multiplier is a parameter rather than a constant: it used to be +// forgotten on the suffixed path, so "512MiB" parsed as 512 *bytes* and every +// memory budget written in units was off by the unit's factor. +func scaleSize(n float64, mult int64, orig string) (Size, error) { if n < 0 { return 0, fmt.Errorf("invalid size %q: must not be negative", orig) } - if n >= float64(math.MaxInt64) { + bytes := n * float64(mult) + if bytes >= float64(math.MaxInt64) { return 0, fmt.Errorf("invalid size %q: out of range", orig) } - return Size(int64(n)), nil + return Size(int64(bytes)), nil } // UnmarshalYAML accepts both "512MiB" and a bare number. diff --git a/internal/config/size_test.go b/internal/config/size_test.go new file mode 100644 index 0000000..fb7c23c --- /dev/null +++ b/internal/config/size_test.go @@ -0,0 +1,214 @@ +package config + +import ( + "math" + "testing" + + "gopkg.in/yaml.v3" +) + +func TestParseSizeUnits(t *testing.T) { + cases := []struct { + in string + want int64 + }{ + {"0", 0}, + {"512", 512}, + {"512B", 512}, + {"1K", 1 << 10}, + {"1KB", 1000}, // decimal, unlike KiB + {"1KiB", 1 << 10}, + {"1M", 1 << 20}, + {"1MiB", 1 << 20}, + {"1G", 1 << 30}, + {"1GiB", 1 << 30}, + {"1T", 1 << 40}, + {"1TiB", 1 << 40}, + // "M" must not eat the "iB" of "MiB", and "B" must not eat the "iB" + // of "BiB"-looking input. These are the shadowing cases the longest + // suffix first ordering exists for. + {"1MiB", 1 << 20}, + {"1MIB", 1 << 20}, + {"512mib", 1 << 20 * 512}, + {" 2 MiB ", 2 << 20}, + // Fractions scale exactly for binary units. + {"1.5GiB", 1610612736}, + {"0.5KiB", 512}, + } + for _, tc := range cases { + got, err := ParseSize(tc.in) + if err != nil { + t.Errorf("ParseSize(%q): unexpected error %v", tc.in, err) + continue + } + if int64(got) != tc.want { + t.Errorf("ParseSize(%q) = %d, want %d", tc.in, int64(got), tc.want) + } + } +} + +func TestParseSizeRejectsJunk(t *testing.T) { + // Every one of these must be an error rather than a plausible number: + // a silently misread memory budget is worse than a refusal to start. + for _, in := range []string{ + "", + " ", + "MiB", + "abc", + "512MiBx", // trailing junk, not a truncated match + "1.2.3", + "12 34", + "-1", + "-1MiB", + "1MiB-", // negative after the unit + "KiB", // a unit with no number in front + ".", // a number with no digits + } { + if got, err := ParseSize(in); err == nil { + t.Errorf("ParseSize(%q) = %d, want an error", in, int64(got)) + } + } +} + +func TestParseSizeRangeAndRounding(t *testing.T) { + // A value that does not fit in an int64 is an error, not a wrap. + for _, in := range []string{"8EiB", "9223372036854775807B", "99999999999999999999"} { + if got, err := ParseSize(in); err == nil { + t.Errorf("ParseSize(%q) = %d, want an out-of-range error", in, int64(got)) + } + } + // The largest representable value still parses. + if got, err := ParseSize("8EiB"); err == nil && int64(got) <= 0 { + t.Errorf("ParseSize(%q) = %d, want a positive value or an error", "8EiB", int64(got)) + } + // A fractional value truncates rather than rounding up: a memory budget + // that rounds up is a budget the process does not have. + if got, err := ParseSize("0.1KiB"); err != nil || int64(got) != 102 { + t.Errorf("ParseSize(%q) = %d (err %v), want 102", "0.1KiB", int64(got), err) + } +} + +// TestParseSizeAcceptsExponents pins that ParseFloat's exponent syntax is +// accepted, with or without a unit: the parser is one ParseFloat call in both +// paths, and making "1e3" work while "1e3MiB" errors would be a surprise. +func TestParseSizeAcceptsExponents(t *testing.T) { + if got, err := ParseSize("1e3MiB"); err != nil || int64(got) != 1000*(1<<20) { + t.Errorf("ParseSize(%q) = %d (err %v), want %d", "1e3MiB", int64(got), err, 1000*(1<<20)) + } +} + +func TestSizeUnmarshalYAML(t *testing.T) { + type doc struct { + V Size `yaml:"v"` + } + cases := []struct { + in string + want int64 + }{ + {"v: 512MiB\n", 512 << 20}, + {"v: \"512MiB\"\n", 512 << 20}, + // A bare integer is bytes: that is what MarshalYAML writes, so a + // config the API rewrote must load back unchanged. + {"v: 1073741824\n", 1 << 30}, + {"v: 0\n", 0}, + } + for _, tc := range cases { + var d doc + if err := yaml.Unmarshal([]byte(tc.in), &d); err != nil { + t.Errorf("unmarshal %q: %v", tc.in, err) + continue + } + if int64(d.V) != tc.want { + t.Errorf("unmarshal %q = %d, want %d", tc.in, int64(d.V), tc.want) + } + } + + var d doc + if err := yaml.Unmarshal([]byte("v: not-a-size\n"), &d); err == nil { + t.Error("unmarshal of junk: want an error") + } + // "1Gi" is the k8s spelling and is *not* in the unit table: the parser is + // deliberately strict, so a memory budget must be written 1GiB. Kept as + // a test rather than left implicit, because the k8s manifests sit right + // next to this code and "1Gi" is what an operator reaches for first. + if err := yaml.Unmarshal([]byte("v: 1Gi\n"), &d); err == nil { + t.Error(`unmarshal of "1Gi": want an error, the unit is GiB`) + } +} + +func TestSizeMarshalYAMLRoundTrips(t *testing.T) { + type doc struct { + V Size `yaml:"v"` + } + in := doc{V: Size(1536 << 20)} + b, err := yaml.Marshal(in) + if err != nil { + t.Fatal(err) + } + var out doc + if err := yaml.Unmarshal(b, &out); err != nil { + t.Fatalf("round trip: %v", err) + } + if out.V != in.V { + t.Fatalf("round trip = %d, want %d", int64(out.V), int64(in.V)) + } +} + +func TestSizeString(t *testing.T) { + cases := []struct { + in Size + want string + }{ + {0, "0"}, + {512, "512B"}, + {1 << 10, "1KiB"}, + {1536, "1536B"}, // not a whole KiB, so bytes + {1 << 20, "1MiB"}, + {1 << 30, "1GiB"}, + {3<<30 + 7, "3221225479B"}, // 3Gi + 7: no whole unit left, so bytes + } + for _, tc := range cases { + if got := tc.in.String(); got != tc.want { + t.Errorf("Size(%d).String() = %q, want %q", int64(tc.in), got, tc.want) + } + } +} + +func TestMemoryEffectiveLimit(t *testing.T) { + if got := (Memory{}).EffectiveLimit(); got != math.MaxInt64 { + t.Errorf("no limit: got %d, want MaxInt64 (no ceiling)", got) + } + // A limit below the reserve still leaves the runtime its floor rather + // than a death spiral. + if got := (Memory{Limit: 128 << 20}).EffectiveLimit(); got != 64<<20 { + t.Errorf("limit below the reserve: got %d, want the 64MiB floor", got) + } + if got := (Memory{Limit: 64 << 20}).EffectiveLimit(); got != 64<<20 { + t.Errorf("limit equal to the floor: got %d, want 64MiB", got) + } + // The usual case: 1Gi with the default 256MiB reserve. + if got := (Memory{Limit: 1 << 30}).EffectiveLimit(); got != (1<<30)-(256<<20) { + t.Errorf("1Gi limit: got %d, want %d", got, (1<<30)-(256<<20)) + } + if got := (Memory{Limit: 1 << 30, Reserve: 300 << 20}).EffectiveLimit(); got != (1<<30)-(300<<20) { + t.Errorf("explicit reserve: got %d, want %d", got, (1<<30)-(300<<20)) + } +} + +func TestMemoryEffectiveCacheBytes(t *testing.T) { + // An explicit budget wins outright: the operator said so. + if got := (Memory{Limit: 1 << 30, CacheMaxBytes: 100 << 20}).EffectiveCacheBytes(); got != 100<<20 { + t.Errorf("explicit cache budget: got %d, want %d", got, 100<<20) + } + // Otherwise it is 60% of the effective limit, leaving the rest for the + // GC, the live query ring and the per-query working set. + m := Memory{Limit: 1 << 30, Reserve: 256 << 20} + want := m.EffectiveLimit() * 3 / 5 + if got := m.EffectiveCacheBytes(); got != want { + t.Errorf("derived cache budget: got %d, want %d", got, want) + } + // No memory limit at all means no byte budget either, not a made-up one. + if got := (Memory{}).EffectiveCacheBytes(); got != 0 { + t.Errorf("no limit: got %d, want 0 (unbounded)", got) + } +} diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 1a76ce4..852e52e 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -314,7 +314,7 @@ func (e *Engine) Handle(ctx context.Context, req Request) Response { // Group-level domain policy. if e.groupBlocks(decision, qname) { - res.Msg = blockedResponse(q, e.blockCfg(), nil) + res.Msg = e.finalize(blockedResponse(q, e.blockCfg(), nil), q, req) res.Action = qlog.ActionBlock res.BlockSource = "acl:" + decision.Group e.finish(req, res, start, decision) @@ -324,7 +324,7 @@ func (e *Engine) Handle(ctx context.Context, req Request) Response { // Local zone data. if store := e.local.Get(); store != nil && store.Count() > 0 { if local, ok := e.answerLocal(q, store); ok { - res.Msg = local + res.Msg = e.finalize(local, q, req) res.Action = qlog.ActionAllow e.finish(req, res, start, decision) return res @@ -348,7 +348,7 @@ func (e *Engine) Handle(ctx context.Context, req Request) Response { e.finish(req, res, start, decision) return res default: - res.Msg = e.blockedMsg(q, blocked) + res.Msg = e.finalize(e.blockedMsg(q, blocked), q, req) res.Action = qlog.ActionBlock res.BlockSource = blocked.Source res.CacheHit = false @@ -407,7 +407,13 @@ func (e *Engine) Handle(ctx context.Context, req Request) Response { return res } metrics.RecordUpstreamLatency(addr, time.Since(upStart)) - reply := e.fromUpstream(q, up, req) + // finalize is what makes the answer safe to put on the wire: it echoes the + // client's question and id verbatim and truncates to the client's + // advertised buffer. Without it an uncached multi-kilobyte answer left + // here oversized for a 512-byte UDP client, which no responder can deliver + // and the client silently drops. (The cache-hit path above has always + // done this; the uncached path did not.) + reply := e.finalize(e.fromUpstream(q, up, req), q, req) res.Msg = reply res.Upstream = addr res.Action = qlog.ActionAllow diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go new file mode 100644 index 0000000..1c368a2 --- /dev/null +++ b/internal/engine/engine_test.go @@ -0,0 +1,790 @@ +package engine + +import ( + "fmt" + "net" + "sync" + "testing" + "time" + + "github.com/miekg/dns" + + "git.k8s-git-k6n.net/mats/fancy-dns/internal/acl" + "git.k8s-git-k6n.net/mats/fancy-dns/internal/blocklist" + "git.k8s-git-k6n.net/mats/fancy-dns/internal/cache" + "git.k8s-git-k6n.net/mats/fancy-dns/internal/config" + "git.k8s-git-k6n.net/mats/fancy-dns/internal/qlog" +) + +// testZone is the local zone the harness loads when a test asks for one. +const testZone = `$TTL 60 +local. SOA ns.local. hostmaster.local. 1 3600 600 604800 60 +local. NS ns.local. +ns.local. A 192.168.1.2 +host.local. A 192.168.1.5 +*.wild.local. A 192.168.1.50 +` + +// loopbackGroup builds an ACL with one group that matches this test's client +// (127.0.0.1), so a test can attach a policy without inventing CIDRs. +func loopbackGroup(g config.Group) config.ACL { + if g.Name == "" { + g.Name = "test" + } + if g.Policy == "" { + g.Policy = "allow" + } + if len(g.Match.CIDRs) == 0 { + g.Match.CIDRs = []string{"127.0.0.0/8"} + } + return config.ACL{DefaultPolicy: "allow", Groups: []config.Group{g}} +} + +// TestACLDenyWinsOverLocalZone pins the first ordering rule: a denied client +// is refused before the local zone, the blocklist or the cache is consulted. +func TestACLDenyWinsOverLocalZone(t *testing.T) { + h := newHarness(t, hopt{ + acl: func(a *config.ACL) { *a = loopbackGroup(config.Group{Policy: "deny"}) }, + rules: []blocklist.Rule{{Name: "denied.local"}}, + localZone: testZone, + }) + + // host.local. is in the local zone and denied.local. is on the + // blocklist: both must come back REFUSED, which is only possible if the + // ACL ran first. + for _, name := range []string{"host.local.", "denied.local."} { + res := h.ask(name, dns.TypeA) + requireRcode(t, res, dns.RcodeRefused) + requireAction(t, res, qlog.ActionRefused) + if res.Group != "test" { + t.Fatalf("group = %q, want %q", res.Group, "test") + } + if res.GroupPolicy != "deny" { + t.Fatalf("group policy = %q, want deny", res.GroupPolicy) + } + if res.Client != "127.0.0.1" { + t.Fatalf("client = %q, want 127.0.0.1", res.Client) + } + requireNoUpstreamHit(t, h, name) + } +} + +// TestLocalZoneWinsOverBlocklist pins the second rule: locally defined +// authoritative data is answered before the blocklist, so an operator can +// un-block a name in a zone file. +func TestLocalZoneWinsOverBlocklist(t *testing.T) { + h := newHarness(t, hopt{ + rules: []blocklist.Rule{{Name: "host.local"}, {Name: "*.wild.local"}}, + localZone: testZone, + }) + + res := h.ask("host.local.", dns.TypeA) + requireAction(t, res, qlog.ActionAllow) + if got := answerA(res.Msg); got != "192.168.1.5" { + t.Fatalf("answer = %q, want 192.168.1.5 (blocked instead?)", got) + } + if res.Msg == nil || !res.Msg.Authoritative { + t.Fatal("local zone answer is not authoritative") + } + requireNoUpstreamHit(t, h, "host.local.") + + // A wildcard rule covers the subdomains of wild.local but the zone's + // own wildcard record answers first. + w := h.ask("anything.wild.local.", dns.TypeA) + if got := answerA(w.Msg); got != "192.168.1.50" { + t.Fatalf("wildcard answer = %q, want 192.168.1.50", got) + } +} + +// TestLocalZoneNxDomainForAZoneItOwns pins the negative case: a name inside a +// loaded zone that does not exist is NXDOMAIN with the zone's SOA, and never +// reaches the upstream. +func TestLocalZoneNxDomainForAZoneItOwns(t *testing.T) { + h := newHarness(t, hopt{localZone: testZone}) + res := h.ask("nope.local.", dns.TypeA) + requireRcode(t, res, dns.RcodeNameError) + if len(res.Msg.Ns) == 0 { + t.Fatal("NXDOMAIN without an SOA in the authority section") + } + requireNoUpstreamHit(t, h, "nope.local.") +} + +// TestBlocklistWinsOverCache pins the third rule: a cached answer must not +// outrank a blocklist rule. A name cached with a real address is still +// blocked, and the blocklist is answered without touching the network. +func TestBlocklistWinsOverCache(t *testing.T) { + h := newHarness(t, hopt{rules: []blocklist.Rule{{Name: "blocked.example.test"}}}) + + // Seed the cache with the exact key the engine will compute, so a pass + // would be visible as a cache hit. + seed := new(dns.Msg) + seed.SetQuestion("blocked.example.test.", dns.TypeA) + rr, _ := dns.NewRR("blocked.example.test. 60 IN A 192.0.2.55") + seed.Answer = append(seed.Answer, rr) + key := cache.QueryKey("blocked.example.test.", dns.TypeA, dns.ClassINET, false, 0) + h.cache.PutResponse(key, seed, "seed") + if got := h.cache.Get(key); got.Kind != cache.Fresh { + t.Fatalf("seeded cache entry is %v, want Fresh", got.Kind) + } + + res := h.ask("blocked.example.test.", dns.TypeA) + requireAction(t, res, qlog.ActionBlock) + requireRcode(t, res, dns.RcodeNameError) + if res.CacheHit { + t.Fatal("a blocked query reported a cache hit: the cache outranked the blocklist") + } + if res.BlockSource != "test" { + t.Fatalf("block source = %q, want %q", res.BlockSource, "test") + } + if answerA(res.Msg) == "192.0.2.55" { + t.Fatal("the cached address was served for a blocked name") + } + requireNoUpstreamHit(t, h, "blocked.example.test.") +} + +// TestBlocklistDropSendsNothing pins the "drop" action: no response at all, +// not an empty one. +func TestBlocklistDropSendsNothing(t *testing.T) { + h := newHarness(t, hopt{rules: []blocklist.Rule{ + {Name: "drop.example.test", Action: blocklist.ActionDrop}, + }}) + res := h.ask("drop.example.test.", dns.TypeA) + if !res.Drop { + t.Fatal("Drop = false, want true") + } + if res.Msg != nil { + t.Fatal("a dropped query produced a message to send") + } + requireAction(t, res, qlog.ActionDrop) + requireNoUpstreamHit(t, h, "drop.example.test.") +} + +// TestAllowRuleBeatsBlockRule pins the whitelist: an explicit allow rule wins +// over a list entry, and the name resolves. +func TestAllowRuleBeatsBlockRule(t *testing.T) { + h := newHarness(t, hopt{rules: []blocklist.Rule{ + {Name: "*.example.test"}, + {Name: "ok.example.test", Action: blocklist.ActionAllow}, + }}) + res := h.ask("ok.example.test.", dns.TypeA) + requireAction(t, res, qlog.ActionAllow) + if got := answerA(res.Msg); got != "203.0.113.7" { + t.Fatalf("answer = %q, want the upstream address 203.0.113.7", got) + } + blocked := h.ask("ads.example.test.", dns.TypeA) + requireAction(t, blocked, qlog.ActionBlock) +} + +// TestGlobalConcurrencyCapRefuses pins the overload refusal: a saturated +// global budget is answered SERVFAIL and counted as dropped, not queued. +func TestGlobalConcurrencyCapRefuses(t *testing.T) { + h := newHarness(t, hopt{ + server: func(s *config.Server) { s.MaxConcurrent = 1 }, + }) + open := h.up.hold() + + var wg sync.WaitGroup + first := make(chan Response, 1) + wg.Add(1) + go func() { + defer wg.Done() + first <- h.ask("slow.example.test.", dns.TypeA) + }() + + // The first query is now parked inside the upstream exchange, holding the + // one global slot. + h.up.waitArrived(t) + second := h.ask("other.example.test.", dns.TypeA) + requireRcode(t, second, dns.RcodeServerFailure) + requireAction(t, second, qlog.ActionError) + if h.eng.Dropped() != 1 { + t.Fatalf("dropped = %d, want 1", h.eng.Dropped()) + } + // The refusal must not have reached the network. + requireNoUpstreamHit(t, h, "other.example.test.") + + open() + wg.Wait() + res := <-first + requireAction(t, res, qlog.ActionAllow) + if got := answerA(res.Msg); got != "203.0.113.7" { + t.Fatalf("first answer = %q, want 203.0.113.7", got) + } + if n := h.eng.Inflight(); n != 0 { + t.Fatalf("inflight = %d after both queries finished, want 0", n) + } +} + +// TestPerClientConcurrencyCapRefuses pins the per-client cap, which is checked +// before the global one so one noisy client cannot take the whole budget. +func TestPerClientConcurrencyCapRefuses(t *testing.T) { + h := newHarness(t, hopt{ + acl: func(a *config.ACL) { + *a = loopbackGroup(config.Group{MaxConcurrent: 1}) + }, + // A generous global budget, so a refusal can only come from the + // per-client limiter. + server: func(s *config.Server) { s.MaxConcurrent = 64 }, + }) + open := h.up.hold() + + var wg sync.WaitGroup + first := make(chan Response, 1) + wg.Add(1) + go func() { + defer wg.Done() + first <- h.ask("slow.example.test.", dns.TypeA) + }() + h.up.waitArrived(t) + + second := h.ask("other.example.test.", dns.TypeA) + requireRcode(t, second, dns.RcodeServerFailure) + // A per-client refusal is not a global overload. + if h.eng.Dropped() != 0 { + t.Fatalf("dropped = %d, want 0: the global cap was not the one that refused", h.eng.Dropped()) + } + + open() + wg.Wait() + requireAction(t, <-first, qlog.ActionAllow) +} + +// TestRateLimitRefuses pins the fixed-window rate limit: the first N queries +// in a window go through, the next is REFUSED. +func TestRateLimitRefuses(t *testing.T) { + h := newHarness(t, hopt{ + acl: func(a *config.ACL) { *a = loopbackGroup(config.Group{RateLimit: 2}) }, + }) + for i := 0; i < 2; i++ { + // Distinct names, so the cache cannot turn the second query into a + // hit and mask what is being asserted. + res := h.ask(fmt.Sprintf("rate%d.example.test.", i), dns.TypeA) + requireAction(t, res, qlog.ActionAllow) + } + res := h.ask("rate2.example.test.", dns.TypeA) + requireRcode(t, res, dns.RcodeRefused) + requireAction(t, res, qlog.ActionRefused) +} + +// TestGroupDomainPolicy pins the per-group allow/deny lists, which run after +// the rate limit and before the local zone. +func TestGroupDomainPolicy(t *testing.T) { + t.Run("deny list", func(t *testing.T) { + h := newHarness(t, hopt{ + acl: func(a *config.ACL) { + // "**." is the spelling that covers a domain and its + // subdomains; a bare name is exact (see acl.NewDomainSet). + *a = loopbackGroup(config.Group{DenyDomains: []string{"**.denied.example.test."}}) + }, + }) + res := h.ask("denied.example.test.", dns.TypeA) + requireAction(t, res, qlog.ActionBlock) + if res.BlockSource != "acl:test" { + t.Fatalf("block source = %q, want acl:test", res.BlockSource) + } + // A subdomain of a denied name is denied too. + if sub := h.ask("sub.denied.example.test.", dns.TypeA); sub.Action != qlog.ActionBlock { + t.Fatalf("subdomain action = %q, want block", sub.Action) + } + if sib := h.ask("sibling.example.test.", dns.TypeA); sib.Action != qlog.ActionAllow { + t.Fatalf("sibling action = %q, want allow: the deny list leaked", sib.Action) + } + allowed := h.ask("allowed.example.test.", dns.TypeA) + requireAction(t, allowed, qlog.ActionAllow) + if got := answerA(allowed.Msg); got != "203.0.113.7" { + t.Fatalf("answer = %q, want 203.0.113.7", got) + } + }) + + t.Run("exact deny is exact", func(t *testing.T) { + h := newHarness(t, hopt{ + acl: func(a *config.ACL) { + *a = loopbackGroup(config.Group{DenyDomains: []string{"denied.example.test."}}) + }, + }) + if res := h.ask("denied.example.test.", dns.TypeA); res.Action != qlog.ActionBlock { + t.Fatalf("exact name: action = %q, want block", res.Action) + } + // Documented behaviour of a rule without a wildcard: the apex only. + if sub := h.ask("sub.denied.example.test.", dns.TypeA); sub.Action != qlog.ActionAllow { + t.Fatalf("subdomain of an exact rule: action = %q, want allow", sub.Action) + } + }) + + t.Run("allow list is exclusive", func(t *testing.T) { + h := newHarness(t, hopt{ + acl: func(a *config.ACL) { + *a = loopbackGroup(config.Group{AllowDomains: []string{"only.example.test."}}) + }, + }) + if res := h.ask("other.example.test.", dns.TypeA); res.Action != qlog.ActionBlock { + t.Fatalf("non-allowlisted name: action = %q, want block", res.Action) + } + allowed := h.ask("only.example.test.", dns.TypeA) + requireAction(t, allowed, qlog.ActionAllow) + if got := answerA(allowed.Msg); got != "203.0.113.7" { + t.Fatalf("answer = %q, want 203.0.113.7", got) + } + }) +} + +// TestSkipToggles pins the two per-group bypasses. Both are security-relevant +// switches, so a test that pins them must fail if the flag stops working. +func TestSkipToggles(t *testing.T) { + t.Run("skip blocklist", func(t *testing.T) { + h := newHarness(t, hopt{ + acl: func(a *config.ACL) { *a = loopbackGroup(config.Group{SkipBlocklist: true}) }, + rules: []blocklist.Rule{{Name: "blocked.example.test"}}, + }) + res := h.ask("blocked.example.test.", dns.TypeA) + requireAction(t, res, qlog.ActionAllow) + if got := answerA(res.Msg); got != "203.0.113.7" { + t.Fatalf("answer = %q, want 203.0.113.7 (the blocklist still applies)", got) + } + }) + + t.Run("skip cache", func(t *testing.T) { + h := newHarness(t, hopt{ + acl: func(a *config.ACL) { *a = loopbackGroup(config.Group{SkipCache: true}) }, + }) + for i := 0; i < 2; i++ { + res := h.ask("nocache.example.test.", dns.TypeA) + requireAction(t, res, qlog.ActionAllow) + if res.CacheKind != "" { + t.Fatalf("query %d: cache kind = %q, want none", i, res.CacheKind) + } + } + if n := h.up.hitCount("nocache.example.test."); n != 2 { + t.Fatalf("upstream saw %d queries, want 2: the cache was used", n) + } + }) +} + +// TestCacheHitDoesNotReachUpstream pins the cache fast path. +func TestCacheHitDoesNotReachUpstream(t *testing.T) { + h := newHarness(t, hopt{}) + first := h.ask("cached.example.test.", dns.TypeA) + requireAction(t, first, qlog.ActionAllow) + if first.CacheHit { + t.Fatal("the first query reported a cache hit") + } + if n := h.up.hitCount("cached.example.test."); n != 1 { + t.Fatalf("upstream saw %d queries, want 1", n) + } + + second := h.ask("cached.example.test.", dns.TypeA) + if !second.CacheHit || second.CacheKind != "hit" { + t.Fatalf("second query: hit=%v kind=%q, want a fresh hit", second.CacheHit, second.CacheKind) + } + requireAction(t, second, qlog.ActionCached) + if got := answerA(second.Msg); got != "203.0.113.7" { + t.Fatalf("answer = %q, want 203.0.113.7", got) + } + if ttl := firstTTL(second.Msg); ttl == 0 || ttl > 60 { + t.Fatalf("cached TTL = %d, want 1..60", ttl) + } + if second.Upstream == "" { + t.Fatal("the cached answer lost its upstream attribution") + } + if n := h.up.hitCount("cached.example.test."); n != 1 { + t.Fatalf("upstream saw %d queries after a cache hit, want 1", n) + } +} + +// TestServeStaleAgesToOneAndRefreshesOnce pins serve-stale: the stale answer +// is served with TTL 1, and the background refresh is single-flight however +// many queries pile up while it is running. +func TestServeStaleAgesToOneAndRefreshesOnce(t *testing.T) { + h := newHarness(t, hopt{serveStale: 10 * time.Minute, clock: true}) + + first := h.ask("stale.example.test.", dns.TypeA) + requireAction(t, first, qlog.ActionAllow) + + // Expire the entry: the upstream TTL is 60s. + h.advance(61 * time.Second) + + open := h.up.hold() + stale := h.ask("stale.example.test.", dns.TypeA) + if !stale.CacheHit || stale.CacheKind != "stale" { + t.Fatalf("hit=%v kind=%q, want a stale hit", stale.CacheHit, stale.CacheKind) + } + if got := answerA(stale.Msg); got != "203.0.113.7" { + t.Fatalf("stale answer = %q, want 203.0.113.7", got) + } + if ttl := firstTTL(stale.Msg); ttl != 1 { + t.Fatalf("stale TTL = %d, want 1: a stale answer must not be cached by the client", ttl) + } + + // The prefetch is now parked in the upstream. More stale queries must not + // start a second one. + h.up.waitArrived(t) + for i := 0; i < 3; i++ { + if res := h.ask("stale.example.test.", dns.TypeA); res.CacheKind != "stale" { + t.Fatalf("query %d during refresh: cache kind = %q, want stale", i, res.CacheKind) + } + } + // Give a second prefetch, if one were wrongly launched, time to arrive. + time.Sleep(250 * time.Millisecond) + if n := h.up.hitCount("stale.example.test."); n != 2 { + t.Fatalf("upstream saw %d queries, want 2 (one resolve, one refresh)", n) + } + + open() + // After the refresh lands the entry is fresh again. + deadline := time.Now().Add(5 * time.Second) + for { + if res := h.ask("stale.example.test.", dns.TypeA); res.CacheKind == "hit" { + break + } + if time.Now().After(deadline) { + t.Fatal("the entry never came back fresh after the background refresh") + } + time.Sleep(10 * time.Millisecond) + } +} + +// TestTruncationAgainstClientBuffer pins RFC 2181 truncation on the uncached +// path: a 4 KiB answer over a 512-byte client buffer comes back with TC and no +// records, and the same answer is delivered in full over TCP or with a large +// EDNS0 buffer. +func TestTruncationAgainstClientBuffer(t *testing.T) { + h := newHarness(t, hopt{}) + + small := h.ask("big.test.", dns.TypeTXT) + if small.Msg == nil || !small.Msg.Truncated { + t.Fatal("a 4 KiB answer over a 512-byte buffer was not truncated") + } + if len(small.Msg.Answer) != 0 { + t.Fatalf("truncated answer carries %d records, want none", len(small.Msg.Answer)) + } + if small.Msg.Rcode != dns.RcodeSuccess { + t.Fatalf("rcode = %s, want NOERROR", dns.RcodeToString[small.Msg.Rcode]) + } + + // A different name, so the 4 KiB answer in the cache is not the one being + // served: TCP clients are not size-limited. + overTCP := h.askTCP("big2.test.", dns.TypeTXT) + if overTCP.Msg == nil || overTCP.Msg.Truncated { + t.Fatal("a TCP client got a truncated answer") + } + if firstTTL(overTCP.Msg) == 0 { + t.Fatal("the TCP answer has no records") + } + + // A large advertised EDNS0 buffer gets the whole thing too. + edns := h.askEDNS("big3.test.", dns.TypeTXT, 4096) + if edns.Msg == nil || edns.Msg.Truncated { + t.Fatal("a 4096-byte EDNS0 client got a truncated answer") + } + if len(edns.Msg.Answer) == 0 { + t.Fatal("the EDNS0 answer has no records") + } + if b, err := edns.Msg.Pack(); err != nil || len(b) > 4096 { + t.Fatalf("packed size = %d (err %v), want <= 4096", len(b), err) + } +} + +// TestSmallClientDoesNotPoisonTheCache pins the reason the engine caches the +// untruncated upstream answer: a 512-byte client must not leave a truncated +// copy in the cache for every later client. +func TestSmallClientDoesNotPoisonTheCache(t *testing.T) { + h := newHarness(t, hopt{}) + + small := h.ask("big.test.", dns.TypeTXT) + if small.Msg == nil || !small.Msg.Truncated { + t.Fatal("precondition: the 512-byte client was not truncated") + } + full := h.askEDNS("big.test.", dns.TypeTXT, 4096) + if full.Msg == nil || full.Msg.Truncated { + t.Fatal("a large-buffer client got a truncated answer: the cache entry was poisoned") + } + if len(full.Msg.Answer) == 0 { + t.Fatal("the cached answer has no records") + } + if n := h.up.hitCount("big.test."); n != 1 { + t.Fatalf("upstream saw %d queries, want 1: the second query was not served from cache", n) + } +} + +// TestTruncatedUpstreamAnswerIsNotCached pins the other half of the rule: an +// answer the upstream flagged TC is never stored, because it would be served +// truncated forever. +func TestTruncatedUpstreamAnswerIsNotCached(t *testing.T) { + h := newHarness(t, hopt{}) + for i := 0; i < 2; i++ { + res := h.ask("tc.test.", dns.TypeA) + if res.CacheHit { + t.Fatalf("query %d was served from cache: a TC answer was cached", i) + } + } + if n := h.up.hitCount("tc.test."); n != 2 { + t.Fatalf("upstream saw %d queries, want 2", n) + } +} + +// TestMalformedQueriesAreFormErr pins the validation step, which runs before +// the ACL: a query with no question, two questions, or the response bit set is +// FORMERR, and still counts as a query. +func TestMalformedQueriesAreFormErr(t *testing.T) { + h := newHarness(t, hopt{ + acl: func(a *config.ACL) { *a = loopbackGroup(config.Group{Policy: "deny"}) }, + }) + + twoQuestions := new(dns.Msg) + twoQuestions.SetQuestion("a.example.test.", dns.TypeA) + twoQuestions.Question = append(twoQuestions.Question, dns.Question{Name: "b.example.test.", Qtype: dns.TypeA, Qclass: dns.ClassINET}) + + alreadyResponse := new(dns.Msg) + alreadyResponse.SetQuestion("a.example.test.", dns.TypeA) + alreadyResponse.Response = true + + empty := new(dns.Msg) + + for name, q := range map[string]*dns.Msg{"no question": empty, "two questions": twoQuestions, "response bit set": alreadyResponse} { + before := h.totals() + res := h.askWith(Request{Msg: q, Remote: h.request("a.example.test.", dns.TypeA).Remote, Transport: "udp"}) + requireRcode(t, res, dns.RcodeFormatError) + requireAction(t, res, qlog.ActionError) + if got := h.totals().Queries - before.Queries; got != 1 { + t.Fatalf("%s: metrics recorded %d queries, want 1", name, got) + } + } + if h.up.totalHits() != 0 { + t.Fatal("a malformed query reached the upstream") + } +} + +// TestMetricsAndQueryLogFireOncePerQuery pins the hook contract: every query +// that reaches Handle is counted exactly once and logged exactly once, +// including the ones refused before resolution. +func TestMetricsAndQueryLogFireOncePerQuery(t *testing.T) { + cases := []struct { + name string + opt hopt + // queries is how many queries the case sends: every one of them must + // be counted exactly once. + queries int + send func(*harness) + want qlog.Action + }{ + { + name: "upstream answer", + opt: hopt{}, + queries: 1, + send: func(h *harness) { h.ask("once.example.test.", dns.TypeA) }, + want: qlog.ActionAllow, + }, + { + name: "cache hit", + opt: hopt{}, + queries: 2, + send: func(h *harness) { + h.ask("once.example.test.", dns.TypeA) + h.ask("once.example.test.", dns.TypeA) + }, + want: qlog.ActionCached, + }, + { + name: "blocked", + opt: hopt{rules: []blocklist.Rule{{Name: "blocked.example.test"}}}, + queries: 1, + send: func(h *harness) { h.ask("blocked.example.test.", dns.TypeA) }, + want: qlog.ActionBlock, + }, + { + name: "refused", + opt: hopt{acl: func(a *config.ACL) { *a = loopbackGroup(config.Group{Policy: "deny"}) }}, + queries: 1, + send: func(h *harness) { + h.ask("denied.example.test.", dns.TypeA) + }, + want: qlog.ActionRefused, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h := newHarness(t, tc.opt) + before := h.totals() + beforeLog := h.logged() + tc.send(h) + after := h.totals() + if got := after.Queries - before.Queries; got != uint64(tc.queries) { + t.Fatalf("metrics counted %d queries, want exactly %d", got, tc.queries) + } + if got := h.logged() - beforeLog; got != tc.queries { + t.Fatalf("query log holds %d new entries, want exactly %d", got, tc.queries) + } + entries := h.entries() + if len(entries) == 0 { + t.Fatal("the newest entry is missing from the live ring") + } + e := entries[0] + if e.Action != tc.want { + t.Fatalf("logged action = %q, want %q", e.Action, tc.want) + } + if e.Client != "127.0.0.1" { + t.Fatalf("logged client = %q, want 127.0.0.1", e.Client) + } + if e.Protocol != "udp" { + t.Fatalf("logged protocol = %q, want udp", e.Protocol) + } + if len(entries) != tc.queries { + t.Fatalf("the live ring holds %d entries, want %d: the hook fired more than once per query", + len(entries), tc.queries) + } + for i := 0; i+1 < len(entries); i++ { + if entries[i].ID-entries[i+1].ID != 1 { + t.Fatalf("entries %d and %d are not adjacent (ids %d, %d): the hook fired more than once per query", + i+1, i, entries[i+1].ID, entries[i].ID) + } + } + }) + } +} + +// TestNoUpstreamIsServfail pins the degenerate wiring: a pool-less engine +// answers SERVFAIL rather than panicking or hanging. +func TestNoUpstreamIsServfail(t *testing.T) { + h := newHarness(t, hopt{noPool: true}) + res := h.ask("nowhere.example.test.", dns.TypeA) + requireRcode(t, res, dns.RcodeServerFailure) + requireAction(t, res, qlog.ActionError) +} + +// TestDeadUpstreamIsServfail pins upstream failure handling: the exchange +// error becomes a SERVFAIL to the client and an error in the metrics. +func TestDeadUpstreamIsServfail(t *testing.T) { + pc, err := net.ListenPacket("udp", "127.0.0.1:0") + if err != nil { + t.Fatalf("reserve port: %v", err) + } + dead := pc.LocalAddr().String() + _ = pc.Close() + + h := newHarness(t, hopt{ + timeout: 500 * time.Millisecond, + upstreams: []config.UpstreamServer{{Addr: dead, Proto: "udp", Weight: 1}}, + }) + before := h.totals() + res := h.ask("dead.example.test.", dns.TypeA) + requireRcode(t, res, dns.RcodeServerFailure) + requireAction(t, res, qlog.ActionError) + if got := h.totals().Errors - before.Errors; got != 1 { + t.Fatalf("errors rose by %d, want 1", got) + } +} + +// TestForwardedQueryShape pins what goes upstream: our own OPT (so an +// upstream can answer EDNS0), the client's ECS forwarded verbatim, our own +// NSID rather than the client's, and never the client's AD bit. +func TestForwardedQueryShape(t *testing.T) { + t.Run("plain query", func(t *testing.T) { + h := newHarness(t, hopt{}) + h.ask("plain.example.test.", dns.TypeA) + got := h.up.received() + if len(got) != 1 { + t.Fatalf("upstream saw %d queries, want 1", len(got)) + } + if got[0].IsEdns0() == nil { + t.Fatal("the forwarded query has no OPT record") + } + if opt := findOption(got[0], dns.EDNS0NSID); opt != nil { + t.Fatal("we leaked our own NSID to a client that did not ask for one") + } + if !got[0].RecursionDesired { + t.Fatal("the forwarded query lost RD") + } + }) + + t.Run("client subnet is forwarded", func(t *testing.T) { + h := newHarness(t, hopt{}) + req := h.request("ecs.example.test.", dns.TypeA) + req.Msg.SetEdns0(1232, false) + sub := &dns.EDNS0_SUBNET{Code: dns.EDNS0SUBNET, Family: 1, SourceNetmask: 24, Address: net.IP{203, 0, 113, 0}} + req.Msg.IsEdns0().Option = append(req.Msg.IsEdns0().Option, sub) + h.askWith(req) + got := h.up.received() + if len(got) != 1 { + t.Fatalf("upstream saw %d queries, want 1", len(got)) + } + opt := findOption(got[0], dns.EDNS0SUBNET) + if opt == nil { + t.Fatal("the client's EDNS0 client subnet was not forwarded") + } + fwd, ok := opt.(*dns.EDNS0_SUBNET) + if !ok || !fwd.Address.Equal(sub.Address) || fwd.SourceNetmask != 24 { + t.Fatalf("forwarded subnet = %+v, want %+v", fwd, sub) + } + }) + + // The NSID branch of prepareUpstream is deliberately not pinned here: + // miekg/dns v1.1.73 reports an EDNS0_NSID option as zero length, so any + // message carrying one fails to pack ("dns: overflow packing opt") and a + // client that asks for NSID gets SERVFAIL. That is a library defect, not + // a decision this package owns; see STATUS.md. +} + +func findOption(m *dns.Msg, code uint16) dns.EDNS0 { + opt := m.IsEdns0() + if opt == nil { + return nil + } + for _, o := range opt.Option { + if o.Option() == code { + return o + } + } + return nil +} + +// TestReplyEchoesTheClientQuestionAndID pins the wire contract every resolver +// owes a client: the reply carries the client's question verbatim (including +// its case, for 0x20 style checks) and the client's transaction ID. +func TestReplyEchoesTheClientQuestionAndID(t *testing.T) { + h := newHarness(t, hopt{}) + req := h.request("HoSt.ExAmPlE.TeSt.", dns.TypeA) + req.Msg.Id = 0x1234 + res := h.askWith(req) + requireAction(t, res, qlog.ActionAllow) + if res.Msg.Id != 0x1234 { + t.Fatalf("reply id = %#x, want 0x1234", res.Msg.Id) + } + if len(res.Msg.Question) != 1 { + t.Fatalf("reply carries %d questions, want 1", len(res.Msg.Question)) + } + if got := res.Msg.Question[0].Name; got != "HoSt.ExAmPlE.TeSt." { + t.Fatalf("reply question = %q, want the client's name verbatim", got) + } + if got := answerA(res.Msg); got != "203.0.113.7" { + t.Fatalf("answer = %q, want 203.0.113.7", got) + } +} + +// TestGroupStatsCountEveryQuery pins the per-group counters the UI reads. +func TestGroupStatsCountEveryQuery(t *testing.T) { + h := newHarness(t, hopt{ + acl: func(a *config.ACL) { *a = loopbackGroup(config.Group{}) }, + rules: []blocklist.Rule{{Name: "blocked.example.test"}}, + }) + h.ask("stats.example.test.", dns.TypeA) + h.ask("blocked.example.test.", dns.TypeA) + h.ask("blocked.example.test.", dns.TypeA) + + var found *acl.GroupSummary + for i, g := range h.eng.GroupStats() { + if g.Name == "test" { + found = &h.eng.GroupStats()[i] + break + } + } + if found == nil { + t.Fatalf("group %q missing from the stats table: %+v", "test", h.eng.GroupStats()) + } + if found.QueryCount != 3 { + t.Fatalf("query count = %d, want 3", found.QueryCount) + } + if found.BlockedCount != 2 { + t.Fatalf("blocked count = %d, want 2", found.BlockedCount) + } +} diff --git a/internal/engine/harness_test.go b/internal/engine/harness_test.go new file mode 100644 index 0000000..98fb889 --- /dev/null +++ b/internal/engine/harness_test.go @@ -0,0 +1,495 @@ +package engine + +import ( + "context" + "net" + "net/netip" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/miekg/dns" + + "git.k8s-git-k6n.net/mats/fancy-dns/internal/acl" + "git.k8s-git-k6n.net/mats/fancy-dns/internal/blocklist" + "git.k8s-git-k6n.net/mats/fancy-dns/internal/cache" + "git.k8s-git-k6n.net/mats/fancy-dns/internal/config" + "git.k8s-git-k6n.net/mats/fancy-dns/internal/localzone" + "git.k8s-git-k6n.net/mats/fancy-dns/internal/metrics" + "git.k8s-git-k6n.net/mats/fancy-dns/internal/qlog" + "git.k8s-git-k6n.net/mats/fancy-dns/internal/resolver" +) + +// fakeUpstream is a UDP resolver that answers every name under .test. with a +// fixed address, so a test can tell "resolved upstream" from "answered locally" +// without touching the internet. It records the queries it received, which is +// how the forwarded-message tests inspect what the engine put on the wire. +type fakeUpstream struct { + *dns.Server + addr string + + mu sync.Mutex + hits map[string]int + queries []*dns.Msg + // writeErr is the last error the handler's write returned: a fake upstream + // that silently fails to pack looks exactly like a client-side timeout. + writeErr error + + // gate, when set, parks handlers until it is closed. arrived is signalled + // before parking, so a test can wait for the query to be *inside* the + // pipeline rather than guessing with a sleep. + gate chan struct{} + arrived chan struct{} + // release is closed at cleanup so a held gate can never wedge shutdown. + release chan struct{} +} + +func newFakeUpstream(t *testing.T) *fakeUpstream { + t.Helper() + pc, err := net.ListenPacket("udp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + u := &fakeUpstream{ + addr: pc.LocalAddr().String(), + hits: make(map[string]int), + arrived: make(chan struct{}, 64), + release: make(chan struct{}), + } + u.Server = &dns.Server{PacketConn: pc, Handler: dns.HandlerFunc(u.handle)} + go func() { _ = u.ActivateAndServe() }() + t.Cleanup(func() { + close(u.release) + _ = u.Shutdown() + }) + return u +} + +func (u *fakeUpstream) handle(w dns.ResponseWriter, q *dns.Msg) { + u.mu.Lock() + if len(q.Question) > 0 { + u.hits[q.Question[0].Name]++ + } + u.queries = append(u.queries, q.Copy()) + u.mu.Unlock() + select { + case u.arrived <- struct{}{}: + default: + } + if g := u.gateCh(); g != nil { + select { + case <-g: + case <-u.release: + } + } + if len(q.Question) == 0 { + return + } + m := new(dns.Msg) + m.SetReply(q) + name := q.Question[0].Name + switch { + case !strings.HasSuffix(strings.ToLower(name), ".test."): + m.Rcode = dns.RcodeNameError + m.Ns = []dns.RR{testSOA("test.", 60)} + default: + // A "tc*" name answers *and* sets TC, which is how a real upstream + // reports an answer that did not fit; it must not be cached. + if strings.HasPrefix(strings.ToLower(name), "tc.") { + m.Truncated = true + } + switch q.Question[0].Qtype { + case dns.TypeTXT: + // "big*" names answer with a 4 KiB TXT so a 512-byte client + // buffer has something to overflow. + if strings.HasPrefix(strings.ToLower(name), "big") { + // Built as a record rather than parsed, and as many short + // strings rather than one long one: this version of miekg/dns + // reports a character-string over 255 bytes as zero length, so a + // single 4 KiB TXT never packs (a silent write failure that looks + // like a client timeout). 15 x 200 lands just over 3 KiB, inside + // the 4 KiB buffer the engine advertises, and far outside a + // 512-byte client buffer. + txt := make([]string, 15) + for i := range txt { + txt[i] = strings.Repeat("x", 200) + } + rr := &dns.TXT{ + Hdr: dns.RR_Header{Name: name, Rrtype: dns.TypeTXT, Class: dns.ClassINET, Ttl: 60}, + Txt: txt, + } + m.Answer = append(m.Answer, rr) + } else { + rr, _ := dns.NewRR(name + " 60 IN A 203.0.113.9") + m.Answer = append(m.Answer, rr) + } + case dns.TypeA: + rr, _ := dns.NewRR(name + " 60 IN A 203.0.113.7") + m.Answer = append(m.Answer, rr) + } + m.Ns = []dns.RR{testSOA("test.", 60)} + } + if err := w.WriteMsg(m); err != nil { + u.mu.Lock() + u.writeErr = err + u.mu.Unlock() + } +} + +// lastWriteErr reports the last write failure, or nil. +func (u *fakeUpstream) lastWriteErr() error { + u.mu.Lock() + defer u.mu.Unlock() + return u.writeErr +} + +func (u *fakeUpstream) gateCh() chan struct{} { + u.mu.Lock() + defer u.mu.Unlock() + return u.gate +} + +// hold parks every handler until the returned func is called. Handlers that +// arrive after it is released are not parked. +func (u *fakeUpstream) hold() func() { + u.mu.Lock() + defer u.mu.Unlock() + if u.gate != nil { + return func() {} + } + g := make(chan struct{}) + u.gate = g + return func() { close(g) } +} + +func (u *fakeUpstream) hitCount(name string) int { + u.mu.Lock() + defer u.mu.Unlock() + return u.hits[dns.Fqdn(name)] +} + +func (u *fakeUpstream) totalHits() int { + u.mu.Lock() + defer u.mu.Unlock() + n := 0 + for _, c := range u.hits { + n += c + } + return n +} + +// received returns the queries the upstream saw, in order. +func (u *fakeUpstream) received() []*dns.Msg { + u.mu.Lock() + defer u.mu.Unlock() + return append([]*dns.Msg(nil), u.queries...) +} + +// waitArrived blocks until at least one query has reached the handler. +func (u *fakeUpstream) waitArrived(t *testing.T) { + t.Helper() + select { + case <-u.arrived: + case <-time.After(5 * time.Second): + t.Fatal("upstream never received a query") + } +} + +// testSOA is the authority record the fake upstream attaches to every answer, +// which is what a real resolver does and what makes NXDOMAIN well-formed. +func testSOA(name string, ttl uint32) dns.RR { + return &dns.SOA{ + Hdr: dns.RR_Header{Name: dns.Fqdn(name), Rrtype: dns.TypeSOA, Class: dns.ClassINET, Ttl: ttl}, + Ns: "ns.test.", + Mbox: "hostmaster.test.", + Serial: 1, + Refresh: 3600, + Retry: 600, + Expire: 604800, + Minttl: 60, + } +} + +// testClock is the injectable time source the cache manager uses. Advancing it +// is how a test makes an entry expire without sleeping. +type testClock struct { + mu sync.Mutex + t time.Time +} + +func newTestClock() *testClock { return &testClock{t: time.Now()} } + +func (c *testClock) Now() time.Time { + c.mu.Lock() + defer c.mu.Unlock() + return c.t +} + +func (c *testClock) Advance(d time.Duration) { + c.mu.Lock() + defer c.mu.Unlock() + c.t = c.t.Add(d) +} + +// hopt tunes a harness. The zero value is a fully working engine: fake +// upstream, local zone, cache, blocklist and query log. +type hopt struct { + acl func(*config.ACL) + blockCfg func(*config.Block) + server func(*config.Server) + cacheCfg func(*config.Cache) + // rules are compiled into the blocklist set the engine holds. + rules []blocklist.Rule + // localZone is a master file; empty means no local zone at all. + localZone string + // upstreams replaces the default single fake upstream. A nil slice with + // noPool false means "use the fake upstream". + upstreams []config.UpstreamServer + // noPool wires a nil pool, which is the "no upstream configured" case. + noPool bool + // serveStale enables the stale window and prefetching. + serveStale time.Duration + // clock installs the injectable clock into the cache manager. + clock bool + // timeout is the per-exchange upstream timeout. + timeout time.Duration +} + +type harness struct { + t *testing.T + eng *Engine + cfg *config.Config + store *config.Store + pool *resolver.Pool + cache *cache.Manager + log *qlog.Logger + up *fakeUpstream + clock *testClock + blocks *blocklist.Holder + local *localzone.Holder + aclH *acl.Holder +} + +func newHarness(t *testing.T, o hopt) *harness { + t.Helper() + up := newFakeUpstream(t) + timeout := o.timeout + if timeout <= 0 { + timeout = 5 * time.Second + } + + cfg := config.Default() + cfg.Cache.Persist.Enabled = false + cfg.Server.Timeouts.Upstre = config.Duration(timeout) + // A generous upstream buffer: an upstream cannot send us more than we + // advertise, so the truncation tests need room for a 4 KiB answer to + // arrive whole and then be trimmed for a small client. + cfg.DNS.MaxUDPSize = 4096 + cfg.Cache.ServeStale = config.Duration(o.serveStale) + cfg.Block.Lists = nil + if o.acl != nil { + o.acl(&cfg.ACL) + } + if o.blockCfg != nil { + o.blockCfg(&cfg.Block) + } + if o.server != nil { + o.server(&cfg.Server) + } + if o.cacheCfg != nil { + o.cacheCfg(&cfg.Cache) + } + + // The default pool is the fake upstream; a dead address is what the + // upstream-failure test needs, and noPool needs no pool at all. + if !o.noPool { + servers := o.upstreams + if servers == nil { + servers = []config.UpstreamServer{{Addr: up.addr, Proto: "udp", Weight: 1}} + } + cfg.Upstream.Servers = servers + cfg.Upstream.Policy = "parallel" + cfg.Upstream.MaxAttempts = 1 + cfg.Upstream.HealthCheck = 0 + cfg.Upstream.BreakerThreshold = 5 + cfg.Upstream.BreakerCooldown = config.Duration(30 * time.Second) + } else { + cfg.Upstream.Servers = nil + } + + // A local zone file, if asked for. + var local *localzone.Store + if o.localZone != "" { + path := filepath.Join(t.TempDir(), "local.zone") + if err := os.WriteFile(path, []byte(o.localZone), 0o600); err != nil { + t.Fatalf("write zone: %v", err) + } + l, err := localzone.Load([]string{path}, localzone.Options{}) + if err != nil { + t.Fatalf("load zone: %v", err) + } + local = l + } else { + l, err := localzone.Load(nil, localzone.Options{}) + if err != nil { + t.Fatalf("load empty zone set: %v", err) + } + local = l + } + + clock := newTestClock() + cm := cache.NewManager(cache.Options{ + Size: 1000, + Shards: 4, + ServeStale: o.serveStale, + Prefetch: true, + Now: clock.Now, + }, nil) + + log, err := qlog.New(qlog.Options{ + LiveBuffer: 64, + SampleRate: 1, + // Every action is logged, so a test can count entries without + // reasoning about the sampling policy. + LogBlocked: true, + LogCached: true, + LogErrors: true, + TopDomains: 16, + TopTTL: time.Minute, + }) + if err != nil { + t.Fatalf("qlog: %v", err) + } + t.Cleanup(func() { _ = log.Close() }) + + blocks := blocklist.NewHolder(buildSet(o.rules...)) + aclH := acl.NewHolder(acl.Compile(cfg.ACL)) + localH := localzone.NewHolder(local) + store := config.NewStore(cfg, "") + + eng := New(Options{ + Config: store, + Logger: log, + Blocklists: blocks, + Cache: cm, + ACL: aclH, + Local: localH, + }) + if !o.noPool { + eng.pool.Store(resolver.NewPoolWithUDPSize(cfg.Upstream, cfg.DNS.MaxUDPSize)) + } + + h := &harness{ + t: t, eng: eng, cfg: cfg, store: store, cache: cm, log: log, + up: up, clock: clock, blocks: blocks, local: localH, aclH: aclH, + } + if !o.noPool { + h.pool = eng.Pool() + } + return h +} + +func buildSet(rules ...blocklist.Rule) *blocklist.Set { + b := blocklist.NewBuilder() + for _, r := range rules { + b.Add(r, "test", nil) + } + return b.Build() +} + +// setRules swaps in a new compiled set, as a reload would. +func (h *harness) setRules(rules ...blocklist.Rule) { + h.blocks.Set(buildSet(rules...)) +} + +func (h *harness) advance(d time.Duration) { h.clock.Advance(d) } + +// request builds a Request the way the UDP listener would. +func (h *harness) request(name string, qtype uint16) Request { + q := new(dns.Msg) + q.SetQuestion(dns.Fqdn(name), qtype) + q.RecursionDesired = true + return Request{Msg: q, Remote: netip.MustParseAddr("127.0.0.1"), Transport: "udp", Start: time.Now()} +} + +func (h *harness) ask(name string, qtype uint16) Response { + h.t.Helper() + return h.eng.Handle(context.Background(), h.request(name, qtype)) +} + +func (h *harness) askWith(req Request) Response { + h.t.Helper() + return h.eng.Handle(context.Background(), req) +} + +// askEDNS asks with an EDNS0 buffer of the given size. +func (h *harness) askEDNS(name string, qtype uint16, size uint16) Response { + h.t.Helper() + req := h.request(name, qtype) + req.Msg.SetEdns0(size, false) + return h.askWith(req) +} + +// askTCP asks with the TCP flag set, so the UDP size cap does not apply. +func (h *harness) askTCP(name string, qtype uint16) Response { + h.t.Helper() + req := h.request(name, qtype) + req.TCP = true + req.Transport = "tcp" + return h.askWith(req) +} + +func (h *harness) entries() []qlog.Entry { return h.log.Recent(64) } + +func (h *harness) logged() int { return h.log.Count() } + +func (h *harness) totals() metrics.Totals { return metrics.TotalsNow() } + +// answerA returns the first A record's address, or "" when the answer has no +// address record. +func answerA(m *dns.Msg) string { + if m == nil { + return "" + } + for _, rr := range m.Answer { + if a, ok := rr.(*dns.A); ok { + return a.A.String() + } + } + return "" +} + +func firstTTL(m *dns.Msg) uint32 { + if m == nil || len(m.Answer) == 0 { + return 0 + } + return m.Answer[0].Header().Ttl +} + +// requireRcode fails unless the response carries rcode. +func requireRcode(t *testing.T, res Response, rcode int) { + t.Helper() + if res.Msg == nil { + t.Fatalf("no response message (drop=%v)", res.Drop) + } + if res.Msg.Rcode != rcode { + t.Fatalf("rcode = %s, want %s", dns.RcodeToString[res.Msg.Rcode], dns.RcodeToString[rcode]) + } +} + +func requireAction(t *testing.T, res Response, want qlog.Action) { + t.Helper() + if res.Action != want { + t.Fatalf("action = %q, want %q", res.Action, want) + } +} + +func requireNoUpstreamHit(t *testing.T, h *harness, name string) { + t.Helper() + if n := h.up.hitCount(name); n != 0 { + t.Fatalf("upstream saw %d queries for %s, want 0", n, name) + } +} diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index fcd4094..8b1e974 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -200,11 +200,13 @@ func Record(transport, outcome string, d time.Duration) { } // RecordMiss counts a cache miss. A miss is not an error: it just means the -// cache could not answer, so only the Prometheus counter moves. -func RecordMiss() { initCollectors(); cacheMisses.Inc() } +// cache could not answer, so the error counter does not move. The in-process +// counter does, like every other outcome: the dashboard reads it and must +// never disagree with the Prometheus series. +func RecordMiss() { initCollectors(); cacheMisses.Inc(); misses.Add(1) } // RecordStaleHit counts a serve-stale hit. -func RecordStaleHit() { initCollectors(); cacheStaleHits.Inc() } +func RecordStaleHit() { initCollectors(); cacheStaleHits.Inc(); staleHits.Add(1) } // RecordUpstreamError counts an upstream failure. func RecordUpstreamError() { initCollectors(); upstreamErrors.Inc() } diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go new file mode 100644 index 0000000..3bca055 --- /dev/null +++ b/internal/metrics/metrics_test.go @@ -0,0 +1,250 @@ +package metrics + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +// The counters are package singletons, so every assertion here is a delta +// around the call rather than an absolute value. Nothing in this package runs +// in parallel, so a delta is exact. + +func TestRecordMovesTheOutcomeCounter(t *testing.T) { + cases := []struct { + outcome string + check func(Totals) uint64 + name string + wantMove bool + }{ + {OutcomeCache, func(x Totals) uint64 { return x.Hits }, "hits", true}, + {OutcomeBlocked, func(x Totals) uint64 { return x.Blocked }, "blocked", true}, + {OutcomeError, func(x Totals) uint64 { return x.Errors }, "errors", true}, + {OutcomeRefused, func(x Totals) uint64 { return x.Refused }, "refused", true}, + {OutcomeDropped, func(x Totals) uint64 { return x.Drops }, "dropped", true}, + // local and upstream are outcomes for the query log and the + // duration histogram, but they have no counter of their own: a + // resolution that succeeded is counted as a query and nothing + // more. + {OutcomeLocal, func(x Totals) uint64 { return x.Queries }, "queries only", true}, + {OutcomeUpstream, func(x Totals) uint64 { return x.Queries }, "queries only", true}, + } + for _, tc := range cases { + before := TotalsNow() + Record("udp", tc.outcome, 2*time.Millisecond) + after := TotalsNow() + if got := after.Queries - before.Queries; got != 1 { + t.Errorf("%s: queries moved by %d, want 1", tc.name, got) + } + if tc.outcome != OutcomeLocal && tc.outcome != OutcomeUpstream { + if got := tc.check(after) - tc.check(before); got != 1 { + t.Errorf("%s: outcome counter moved by %d, want 1", tc.name, got) + } + } + // No other counter may move: a blocked query is not also a miss, and + // a refused one is not an error, or the operator cannot trust the + // sum on the dashboard. + if tc.outcome == OutcomeLocal || tc.outcome == OutcomeUpstream { + continue + } + for _, other := range cases { + if other.outcome == tc.outcome { + continue + } + if other.outcome == OutcomeLocal || other.outcome == OutcomeUpstream { + continue + } + // A drop is the one deliberate overlap: it is also an error. + // See TestRecordDropIsAlsoAnError. + if tc.outcome == OutcomeDropped && other.outcome == OutcomeError { + continue + } + if got := other.check(after) - other.check(before); got != 0 { + t.Errorf("%s: %s also moved by %d, want 0", tc.name, other.name, got) + } + } + } +} + +// A drop is a refusal to resolve, so an operator counts it as both. Pin that, +// because the two series are what the "why are queries failing" dashboard +// panel sums. +func TestRecordDropIsAlsoAnError(t *testing.T) { + before := TotalsNow() + Record("udp", OutcomeDropped, time.Millisecond) + after := TotalsNow() + if after.Drops != before.Drops+1 { + t.Errorf("drops moved by %d, want 1", after.Drops-before.Drops) + } + if after.Errors != before.Errors+1 { + t.Errorf("errors moved by %d, want 1: a drop is an error for the operator", after.Errors-before.Errors) + } +} + +// Record with no duration still counts the query: the duration is for the +// histogram, the counter is for the total. +func TestRecordWithoutDurationStillCounts(t *testing.T) { + before := TotalsNow() + Record("udp", OutcomeCache, 0) + after := TotalsNow() + if after.Queries != before.Queries+1 { + t.Errorf("queries moved by %d, want 1", after.Queries-before.Queries) + } +} + +func TestRecordMissAndStaleAreNotOutcomes(t *testing.T) { + // A cache miss is not an error and a stale hit is not a cache hit: each + // has its own counter precisely so the two can be told apart. + before := TotalsNow() + RecordMiss() + RecordStaleHit() + RecordUpstreamError() + after := TotalsNow() + if after.Misses != before.Misses+1 { + t.Errorf("misses moved by %d, want 1", after.Misses-before.Misses) + } + if after.StaleHits != before.StaleHits+1 { + t.Errorf("stale hits moved by %d, want 1", after.StaleHits-before.StaleHits) + } + if after.Queries != before.Queries { + t.Errorf("queries moved by %d: these three do not go through Record", after.Queries-before.Queries) + } +} + +func TestCountHelpersDoNotCountQueries(t *testing.T) { + // These exist for the few places that report an outcome without a full + // Response. They must not inflate the query total, or the queries counter + // stops meaning "queries". + before := TotalsNow() + CountBlocked() + CountError() + CountDrop() + CountRefused() + after := TotalsNow() + if after.Queries != before.Queries { + t.Errorf("queries moved by %d, want 0", after.Queries-before.Queries) + } + if after.Blocked != before.Blocked+1 || after.Errors != before.Errors+1 || + after.Drops != before.Drops+1 || after.Refused != before.Refused+1 { + t.Errorf("a helper missed its own counter: %+v", after) + } +} + +// Totals is a value, not a view: a caller holding a snapshot must not see it +// change under them. +func TestTotalsIsASnapshot(t *testing.T) { + before := TotalsNow() + queries := before.Queries + Record("udp", OutcomeCache, time.Millisecond) + if before.Queries != queries { + t.Errorf("the snapshot moved: %d -> %d", queries, before.Queries) + } +} + +func TestGaugesArePublished(t *testing.T) { + SetConcurrent(7) + SetUp(true) + SetUpstreamHealth("10.0.0.1:53", "udp", true) + SetUpstreamHealth("10.0.0.2:53", "udp", false) + SetBlocklistStats(1234, 4096) + SetACLGroups(3) + SetLocalRecords(11) + RecordBlocklistCompile(15 * time.Millisecond) + RecordUpstreamLatency("10.0.0.1:53", 4*time.Millisecond) + + body := scrape(t) + for _, want := range []string{ + // The name and the value: a dashboard or a ServiceMonitor that + // reads the wrong series fails silently otherwise. + "fancydns_concurrent_queries 7", + "fancydns_up 1", + `fancydns_upstream_health{addr="10.0.0.1:53",proto="udp"} 1`, + `fancydns_upstream_health{addr="10.0.0.2:53",proto="udp"} 0`, + "fancydns_blocklist_rules 1234", + "fancydns_blocklist_source_bytes 4096", + "fancydns_acl_groups 3", + "fancydns_local_records 11", + } { + if !strings.Contains(body, want) { + t.Errorf("scrape is missing %q\nbody (%d bytes):\n%s", want, len(body), truncate(body, 1200)) + } + } + // The histogram's observation count is cumulative across runs of the + // suite, so only its presence is pinned here, never its value. + if !strings.Contains(body, "fancydns_blocklist_compile_seconds_count") { + t.Errorf("scrape is missing the blocklist compile histogram\nbody (%d bytes):\n%s", len(body), truncate(body, 1200)) + } +} + +// The counter names are the contract with the Grafana dashboards and the +// ServiceMonitor, so they are pinned here rather than assumed. +func TestHandlerExposesTheCounterSeries(t *testing.T) { + Record("udp", OutcomeBlocked, time.Millisecond) + RecordMiss() + body := scrape(t) + for _, want := range []string{ + "fancydns_queries_total", + "fancydns_blocked_total", + "fancydns_cache_hits_total", + "fancydns_cache_misses_total", + "fancydns_cache_stale_hits_total", + "fancydns_upstream_errors_total", + "fancydns_errors_total", + "fancydns_dropped_total", + "fancydns_refused_total", + "fancydns_query_duration_seconds_bucket", + "fancydns_query_duration_seconds_count", + } { + if !strings.Contains(body, want) { + t.Errorf("scrape is missing the series %q\nbody (%d bytes):\n%s", want, len(body), truncate(body, 1200)) + } + } +} + +func TestSetUpFalseClearsLiveness(t *testing.T) { + SetUp(true) + if !strings.Contains(scrape(t), "fancydns_up 1") { + t.Fatal("up was not published as 1") + } + SetUp(false) + if !strings.Contains(scrape(t), "fancydns_up 0") { + t.Error("up was not published as 0 after SetUp(false)") + } +} + +func scrape(t *testing.T) string { + t.Helper() + srv := httptest.NewServer(Handler()) + t.Cleanup(srv.Close) + res, err := srv.Client().Get(srv.URL) + if err != nil { + t.Fatalf("scrape: %v", err) + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + t.Fatalf("scrape status = %d", res.StatusCode) + } + b, err := io.ReadAll(res.Body) + if err != nil { + t.Fatalf("read scrape: %v", err) + } + return string(b) +} + +func truncate(s string, n int) string { + if len(s) <= n { + return s + } + return s[:n] + "..." +} + +// The namespace is part of the contract with the dashboards, the ServiceMonitor +// and every saved query: changing it orphans all of them silently. +func TestNamespaceIsStable(t *testing.T) { + if Namespace != "fancydns" { + t.Fatalf("Namespace = %q, want %q: every dashboard and alert is keyed on it", Namespace, "fancydns") + } +} diff --git a/internal/resolver/pool.go b/internal/resolver/pool.go index 3b6bc58..1d1e37c 100644 --- a/internal/resolver/pool.go +++ b/internal/resolver/pool.go @@ -283,13 +283,36 @@ type Pool struct { // rng backs the "random" policy. rngMu sync.Mutex rng *rand.Rand + + // udpSize is the read buffer for UDP clients. It must be at least as large + // as the EDNS0 buffer we advertise upstream: a UDP datagram bigger than + // this buffer is read truncated, fails to unpack, and turns into a + // SERVFAIL. See clientFor. + udpSize uint16 } +// DefaultClientUDPSize is the read buffer used when the caller does not say +// otherwise. It matches the historical hard-coded client buffer. +const DefaultClientUDPSize uint16 = 1232 + // NewPool builds a pool from config. Bootstraps are used for named upstreams; // anything unresolved falls back to the system resolver. func NewPool(cfg config.Upstream) *Pool { + return NewPoolWithUDPSize(cfg, DefaultClientUDPSize) +} + +// NewPoolWithUDPSize builds a pool whose UDP clients read up to udpSize bytes. +// Callers that advertise an EDNS0 buffer to their upstreams must pass that +// same value here, or every answer larger than it is lost. +func NewPoolWithUDPSize(cfg config.Upstream, udpSize uint16) *Pool { + // 512 is the floor a resolver may assume; below it we would truncate + // replies a plain client asked for. + if udpSize < dns.MinMsgSize { + udpSize = dns.MinMsgSize + } p := &Pool{ - rng: rand.New(rand.NewPCG(uint64(time.Now().UnixNano()), 0x9e3779b9)), + rng: rand.New(rand.NewPCG(uint64(time.Now().UnixNano()), 0x9e3779b9)), + udpSize: udpSize, } p.state.Store(buildState(cfg, nil, false)) return p @@ -406,7 +429,7 @@ func (p *Pool) clientFor(u *Upstream) *dns.Client { if c, ok := p.clients.Load(u); ok { return c.(*dns.Client) } - nc := &dns.Client{Net: "udp", UDPSize: 1232, Timeout: 3 * time.Second} + nc := &dns.Client{Net: "udp", UDPSize: p.udpSize, Timeout: 3 * time.Second} switch u.cfg.Proto { case "tls": nc = &dns.Client{