# fancy-dns — development status and continuation plan Last updated: end of the race-fix / test-coverage session. Everything below reflects the working tree, including the known-failing tests and the exact commands to reproduce them. ## Goal A lightweight, high-performance recursive DNS server with: - **Go backend** — sharded LRU cache backed by bbolt, blocklist replication from multiple formats, per-client ACLs, query logging, Prometheus metrics. - **Secure DNS** — DNS-over-TLS (RFC 7858) and DNS-over-HTTPS (RFC 8484). DNSSEC validation was explicitly scoped **out**; AD/CD/extended-RCODE are passed through only. - **React + Tailwind admin console** embedded in the binary. - **Config as zone files + YAML**, hot-reloadable, with **Valkey/Redis** for cross-instance sync (chosen over AMQP). ## Architecture decisions (settled with the user) | Decision | Choice | Why | |---|---|---| | Scope | Full production | clustering, metrics, ACLs, query logs, hot reload, Docker, k8s, CI | | Config store | YAML + DNS master files | reviewable/diffable in git | | Cluster sync | Valkey (Redis protocol) | simple, mature Go client, easy local dev | | Secure DNS | DoT + DoH, no DNSSEC validation | avoids signer/anchor machinery; pass-through only | | Hot cache | Sharded LRU in memory, bbolt for snapshots | no I/O on the hot path; warm restart | ## Package layout ``` cmd/fancy-dns/main.go entrypoint: flags, signals, SIGHUP reload internal/ config/ Config struct, Duration type, Validate, atomic Store + hooks zonefile/ RFC 1035 master-file parser ($TTL/$ORIGIN, parens, serial math) blocklist/ Set (exact/sub/root maps), 4 parsers, Manager (fetch+compile+swap) cache/ Key, Entry, sharded LRU, TTL policy, serve-stale, bbolt Store resolver/ Upstream pool: health, circuit breakers, parallel/failover/random acl/ compiled groups, CIDR/PTR/tag matching, per-group domain policy localzone/ authoritative data from master files, wildcards, delegations engine/ THE QUERY PIPELINE (see below), reverse-PTR cache server/ UDP, TCP, DoT, DoH listeners; all funnel into one handler qlog/ bbolt query-log ring + live in-memory ring + SSE fanout metrics/ Prometheus collectors + rolling time-series rings cluster/ Valkey pub/sub, leader lease, divergence detection api/ admin REST + SSE, auth, CORS, rate limiting app/ wiring, lifecycle, API hook implementations web/ go:embed of the built console web/ Vite + React + TS + Tailwind console deploy/ example config, example zones, k8s manifests ``` ### Query pipeline order (engine.Handle) ``` validate → ACL deny → per-client concurrency → global concurrency → rate limit → group allow/deny domains → local zone → blocklist → cache → upstream → cache put → finalize (EDNS0, UDP truncation) → metrics + qlog ``` Cache is a single shard-local lookup; only a miss touches the network. ## Status ### Done and verified - `go build ./...`, `go vet ./...` and `gofmt -l` are all clean. - **`go test -race ./internal/app/` is green — zero data races.** The two `-race`-only failures and all ten reported races are fixed (see below). - `go test ./internal/zonefile/` is green (the full parser suite). - Frontend **builds and type-checks**: `npx tsc -b --noEmit` and `npm run build` both exit 0, `npx oxlint` reports 0 errors / 10 warnings, and the real bundle lands in `internal/web/dist/`. The `//go:embed all:dist` picks it up with no Go change. - One real frontend bug fixed along the way: four pages used a `toast(string)` duck-typed shim while `useToast()` takes `ToastOptions`, so every mutation toast rendered an empty title. ### Fixed this session (races and their root causes) All of these were found by `-race` and fixed at the source, not with extra locking: 1. **`cache.Entry.Hits` → `atomic.Uint32`.** A counter on an entry shared by every reader raced the LRU walk used by the API. The struct is now never copied by value. 2. **`LRU.perShard` → `atomic.Int64`.** `SetCapacity` (a config reload, from the API goroutine) rewrote the per-shard budget while every query shard was inserting. 3. **`LRU.SetCapacity` now evicts.** Shrinking the budget changed the number but left the old entries resident until they happened to be overwritten; a reload that lowers `cache.size` could not actually reclaim memory. It now trims each shard to the new budget and counts the evictions. 4. **`resolver.Pool` rewritten as copy-on-write.** This was the bulk of the races: `Rebind` mutated `p.ups` / `p.cfg` under a mutex that the query path never took, so `Exchange`, `Healthy`, `All` and the `dns.Client` memo map were all racing. The pool now holds an immutable `poolState` behind `atomic.Pointer`; `Rebind` builds the next state and swaps it with one store. `Upstream` is immutable and holds a shared `*upstreamStats`, so the live health, latency and breaker state of an unchanged server survives a reload while a query in flight keeps a consistent view. 5. **`resolver.Pool.Exchange` no longer marks dead upstreams healthy.** A pre-existing bug: `exchangeOne` called `up.Success(rtt)` before checking the error, so a dead upstream was marked up, had its consecutive-failure count reset and had `LastError` cleared on every failed exchange — **the circuit breaker could never trip through the query path.** 6. **`qlog` live ring → `[]atomic.Pointer[liveEntry]`.** `Log` wrote `l.live[pos%len]` from every query goroutine while `Recent` walked the ring under a read lock the writers never took. Publishing is now one atomic word. 7. **`blocklist.Holder.Get` is nil-safe**, and `Builder` keeps an `ordered` set so a source appears once in `Sources()` whether it was added by a rule or by `NoteSource` (it used to be appended twice). 8. **`cache.packKey` fully qualifies the name before packing.** Keys are stored canonically (lowercase, no trailing dot) but the wire format needs a trailing dot, so `dns.PackDomainName` rejected every key. ### Known failures (fix these next) Run: `go test -race -timeout 120s ./internal/... -count=1` 1. **bbolt snapshot round trip is broken — the warm-restart path does not work.** `internal/cache/store.go` `Snapshot` writes 25 entries and `Load` skips all 25 (`TestStoreSnapshotAndLoad`, `TestStoreLoadSkipsExpiredEntries`, `TestStoreTruncateAndStats` show `loaded = 0, skipped = N`). The write side and the read side disagree about the record layout — most likely the `binary.Uvarint` header that `Load` decodes (`expUnix`, `staleUnix`) is not what `Snapshot` emits. **This is the highest-value remaining bug**: without it every restart is a cold cache. Start at `Snapshot` (around the `put` call) and `Load` (`internal/cache/store.go:78`). 2. **`internal/blocklist` — 5 failing tests, all real implementation gaps**, not test bugs. Do not "fix" them by relaxing the assertions; they are the spec: - `TestSetIntrospection`: `SourceMeta("b")` is missing for a source that was only ever mentioned by a rule, and `Pattern()` reports the internal `*.` / `**.` form where the caller wrote a bare domain. - `TestHolderSwapIsAtomic`: reports a half-applied set. The swap is a single `atomic.Pointer` store, so either the test's invariant is wrong or `Holder` publishes before the set is fully built — check which before changing anything. - `TestParseHosts`: a hosts line whose first field is not an address should emit the notice "no address in first field, falling back to a plain domain list"; it silently produces nothing. - `TestParseRPZ`: bare RPZ owner names are sniffed as `FormatPlain` (the format sniffer only knows the classes `IN`/`CH`/`HS`), and the no-subdomain form is compiled as a wildcard. - `TestParseRobustness`: CRLF input to the adblock parser produces no rules at all, and an over-long line is accepted instead of rejected. - Also unreported by a test but found by inspection: no parser strips a leading UTF-8 BOM, and the documented inline allow marker `!allow example.com` does not work (only `!example.com` does). 3. **No tests yet for `acl`, `localzone`, `resolver`, `config`, `qlog`, `engine`, `server`, `api`, `metrics`, `cluster`.** `internal/acl` and `internal/localzone` were started but the agent ran out of budget; `internal/resolver` was not started at all. ### Not started / incomplete deliverables - **`README.md` is still a 0-byte file.** Everything else in the deploy set exists (written in a parallel pass) and is tracked: `Makefile` (4 KB), `Dockerfile`, `.gitignore`, `.github/workflows/ci.yml` (6 KB), `deploy/config.example.yaml`, `deploy/local.example.zone`, `deploy/zones/`. None of it has been *reviewed against the code* — the CI workflow and the Makefile targets (`make web`, `make race`, `make image`) need a `make -n`/dry-run pass, and every config key, port and CLI flag the docs mention has to be checked against `internal/config` and `cmd/fancy-dns`. - **`deploy/k8s/` has only two of the seven expected manifests** (`00-config.yaml`, `10-deployment.yaml`). Still missing: Service, PVC, ServiceAccount + Secret (the API key), Ingress (needs `proxy_buffering off` for the SSE log stream), namespace and kustomization. - End-to-end verification of the console against a running binary: the bundle compiles and embeds, but nobody has clicked through the pages. - `internal/web/dist` is gitignored except `.gitkeep`, so a fresh clone cannot build the binary without running `npm ci && npm run build` first. Either CI builds the bundle before `go build`, or the ignore rule comes off. ## Continuation plan, in order ### Step 1 — the bbolt round trip (highest priority) ```bash go test -race ./internal/cache/ -run 'TestStore' -count=1 -v ``` Read `Snapshot` and `Load` in `internal/cache/store.go` as a pair and make the record format explicit — a small `encodeRecord`/`decodeRecord` pair with one version byte, so a future change cannot silently desync them the way this one did. Then add a `t.TempDir()` round-trip test that survives a process restart and a partially written file. ### Step 2 — the five blocklist gaps Fix the implementation against the existing assertions (see item 2 above), then `go test -race ./internal/blocklist/ -count=1` to green. The BOM strip and the `!allow` marker are the two an operator is most likely to hit. ### Step 3 — the remaining unit tests Same checklist as before, minus what is now covered (`blocklist`, `cache`, `zonefile` have suites; `cache` and `blocklist` have benchmarks). Priority: `resolver` (breaker open/close, parallel race, TCP fallback on truncation, failover, `Rebind` under `-race` — with a `dns.Server` fake on `127.0.0.1:0`), `acl` (first-match-wins, most-specific CIDR, PTR suffix, `DomainSet` wildcard forms), `localzone` (delegation cut, closest-wildcard per RFC 1034 §4.3.3, NODATA vs NXDOMAIN), `config` (`Duration` parsing, `Validate`, the store hooks). ### Step 4 — review the deploy set and write the README The console pages exist and build; verify them in a browser against a running binary. Then read `Makefile`, `Dockerfile` and `.github/workflows/ci.yml` against the code and fix what does not match, add the missing k8s manifests (Service, PVC, ServiceAccount + Secret, Ingress with `proxy_buffering off` for `/api/logs/stream`, namespace, kustomization), and write `README.md` from the source: config reference, zone-file semantics, blocklist formats, API routes, the `trusted_proxies` / `metrics.protect` / `auth.mode` security caveats, and the limitations (no DNSSEC validation, no zone transfer, no dynamic update). CI must run `npm ci && npm run build` before `go build`, because the Go build embeds `internal/web/dist`. ## Design notes worth preserving - **Cache stores packed wire bytes, not `*dns.Msg`.** A hit costs one `Unpack` and yields a private copy, so serving never mutates shared state and TTL ageing is free. Same trade dnsmasq makes. - **Everything the query path reads is behind an `atomic.Pointer` holder** — ACL, blocklist, localzone, and now the upstream pool. Readers load the pointer once and work against a value that cannot change under them. This is the pattern the whole race cleanup settled on: prefer a new immutable snapshot over a lock. - **Cache invalidation on rule changes flushes the cache** rather than guessing which subtrees are affected. A newly blocked name must not keep serving its cached good answer. `invalidateBlocklistCache()` in `internal/app/hooks.go`. - **API responses share the field names in `internal/app/hooks.go`.** If you rename one, grep the frontend too. - **API keys are compared by SHA-256 hash, constant-time**, and `X-Forwarded-For` is honoured *only* from `trusted_proxies` — otherwise a client could spoof its identity past the ACL and the rate limiter. - **UDP listener resets its read deadline every iteration.** A deadline set once expires permanently and turns the loop into a spin that never reads another packet (a real bug caught by `TestBlocklistToggle`). - **Only the Valkey lease holder fetches lists**, so N instances do not hit a mirror N times. Followers serve their compiled copy. - **Blocked/serve-stale behaviour:** a served-stale answer gets TTL 1, an expired upstream answer is never cached, and a truncated answer is never cached (it would be served truncated forever). ## Test-hygiene traps - `tsc -b` replays **cached diagnostics** from `node_modules/.tmp/*.tsbuildinfo`. After a large edit, `rm node_modules/.tmp/*.tsbuildinfo` or you will debug errors that no longer exist. - `vite build` empties `outDir` and **deletes `internal/web/dist/.gitkeep`**. Restore it or the directory disappears from git. - A test that closes a stop channel *after* `wg.Wait()` deadlocks the whole package and looks like "the suite hangs". Close first, then wait.