fancy-dns/STATUS.md

# fancy-dns — development status and continuation plan

Last updated after the bbolt/blocklist/acl/resolver fix session and the first
end-to-end run of the console. Everything below reflects the working tree.

## 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 |
| k8s networking | No hostNetwork, LoadBalancer Service | the server never sets SO_REUSEPORT, so two host-network pods cannot share :53 |

## Package layout

```
cmd/fancy-dns/main.go          entrypoint: flags, signals, SIGHUP reload
internal/
  config/       Config struct, Duration type, Validate, atomic Store + hooks,
                ${VAR} environment expansion in Load
  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

- `gofmt -l ./cmd ./internal` empty, `go build ./...` clean, `go vet ./...` clean.
- **`go test -race ./internal/... -count=1` is green** for `acl`, `app`,
  `blocklist`, `cache`, `config`, `resolver`, `zonefile`.
- Test suites exist and are green for: `acl` (26 tests + 4 benchmarks),
  `blocklist`, `cache` (incl. benchmarks), `config`, `resolver`, `zonefile`.
- `internal/localzone`, `internal/engine` and `internal/qlog` are being written
  right now — see "in flight" below.
- `README.md` is being written from the source right now.

### The four bugs from the first end-to-end run

Found by running the binary and querying it, not by a test. All four are being
fixed in `internal/zonefile` + `internal/localzone`:

1. **`$TTL 60` is ignored.** Every record came back with the SOA minimum
   (3600). `Zone.DefaultTTL` is documented as the `$TTL` value; RFC 1035 §5.2
   says an explicit `$TTL` wins.
2. **Records are swallowed when the previous record has an odd token count.**
   `ns1 A 192.0.2.53` followed by `txt TXT "hello world"` parses as one record
   `ns1 TXT "TXT" "hello world"` — the TXT record is lost entirely. Data loss
   in `assemble`/`Parse`.
3. **A delegated name answers NXDOMAIN instead of a referral.** With
   `sub NS ns1.sub.example.test.`, `host.sub.example.test. A` returned
   `NXDOMAIN aa=true`; it must be NOERROR, AA false, NS in Authority.
4. **A wildcard answer keeps the literal wildcard owner.** A query for
   `x.wild.example.test. A` returned an answer owned by
   `*.wild.example.test.`. RFC 4592 §3.3.1 requires the owner name to be
   replaced with the query name.

### Fixed earlier and worth not re-breaking

- **`cache.Snapshot`/`Load` never round-tripped.** `Snapshot` packed the whole
  key while `Load` decoded the record header out of the *key's* remainder
  instead of the value, so all 25 entries were skipped — every restart was a
  cold cache. There is now an explicit `encodeRecord`/`decodeRecord` pair with
  a version byte and a CRC-32C trailer, so a future format change is detected
  rather than silently misparsed. `Load` also drops entries already past
  `ExpiresAt` and rewrites the TTL to the time actually left, so a served TTL
  never jumps back up across a restart.
- **ACL PTR suffix matched across label boundaries.** A group scoped to
  `example.com.` also swallowed `ample.com.` and `notexample.com.` — a client
  could land in a group it does not belong to. `matchPTR` now requires the
  suffix to start a label, and `ptrSuffixes` normalises the config value to FQDN
  form so the check works even for a config that skipped `Validate`.
- **The ACL `*` catch-all was dropped** from the rendered domain patterns, so
  the engine's per-query `NewDomainSet` recompile lost the "everything" case.
- **`Upstream.Success` never closed an open breaker.** A recovered upstream
  stayed excluded for the whole cooldown — the mirror image of the older bug
  where a failed exchange marked a dead upstream healthy.
- **`Exchange` fanned out in parallel even under `policy: failover`.** It only
  took the parallel path when exactly one upstream was chosen, so a failover
  list of two raced both.
- **Blocklist:** a leading UTF-8 BOM is stripped, `!allow example.com` works,
  `SourceMeta` exists for a source that only contributed a rule, `Pattern()`
  reports the bare name again (`WireName()` produces the wildcard form for the
  API), an RPZ record with no explicit TTL inherits the zone default, a hosts
  line with no address in the first field says so, the adblock parser handles
  CRLF and adblock lines pasted into a plain list, and over-long lines are
  reported rather than accepted.
- **`TestHolderSwapIsAtomic` had the wrong invariant**, not the Holder: after a
  swap to a set that does not block `a.example.com`, that name is legitimately
  unblocked. The swap is a single `atomic.Pointer` store.
- **CI could not have passed.** `go.mod` requires Go 1.27.1; CI pinned 1.23 and
  the Dockerfile pinned `golang:1.23-alpine`. Both now track the toolchain.
- **The Dockerfile clobbered the console.** It copied the source tree *after*
  the console bundle, and a fresh clone's `internal/web/dist` holds only
  `.gitkeep`.
- **The k8s set could not have worked.** `hostNetwork: true` with
  `replicas: 2`, and the server never sets SO_REUSEPORT, so the second pod
  could not bind :53. It now runs without hostNetwork behind a LoadBalancer
  Service, with a separate ClusterIP Service for the Ingress (a headless
  Service cannot back one), a ServiceAccount, an API-key Secret, per-pod PVCs,
  and a kustomization. The HPA is gone — it cannot drive a StatefulSet.
- **The API key could not come from a Secret**, because the config had no
  variable expansion. `config.Load` now expands `${VAR}`, `${VAR:-default}` and
  `${NAME-default}` and fails to start on an unset variable with no default, so
  a missing secret is a crash loop rather than an unauthenticated console.
  Expansion skips YAML comments, so documenting `${VAR}` in the ConfigMap does
  not break the file.

### Memory envelope (added)

`memory.limit` / `memory.reserve` / `memory.cache_max_bytes` are new. `limit`
becomes a `GOMEMLIMIT` via `debug.SetMemoryLimit`, reduced by `reserve`; the
cache byte budget is wired into the LRU, which already enforced `maxBytes` per
shard but had no setter and was never fed one.

Rationale: a cgroup OOM kill is the worst outcome available to a resolver — an
outage rather than a cache miss, with no chance to serve stale on the way out.
`GOMEMLIMIT` turns the same limit into a soft one the GC works towards. The
reserve exists because the Go heap does not account for goroutine stacks,
runtime metadata, or the bbolt mmap backing the cache snapshot and the query log
— handing the runtime the full cgroup limit is how you get killed by a mechanism
that believed it had headroom.

`ParseSize` accepts `512MiB`, `2GiB`, `1000` (bytes), and is strict about junk
(`512MiBx` is an error). `config.Size` has no test coverage yet — **that is the
first thing to add**.

### Known red

- `internal/config`: `TestLoadUnsetEnvIsFatalAndNamesVariable` fails. The
  fixture now has a blocklist entry, so `Validate` passes, but the expected
  `${FD_TEST_MISSING}` expansion error never fires — `Load` returns the
  *Validate* error instead. `TestExpandEnvErrors` covers the same
  reference-in-a-flow-sequence case and passes, so the difference is specific to
  the quoted, flow-sequence form `api_keys: ["${X}"]`. Undiagnosed; do not
  assume the expander is fine.
- `internal/qlog` and `internal/localzone` have test files mid-write from
  subagents that ran out of budget. `internal/qlog` does not compile
  (`internal/... /qlog.go:520: undefined: liveEntry`, `viewsFor: undefined:
  Resize`). Fix that before anything else in those packages.

### Open decision

- **Unknown config keys are silently ignored.** Writing `block:` where the
  struct wants `blocklist:` produced a plausible-looking server instead of an
  error. `yaml.Decoder.KnownFields(true)` would reject it, but that is a
  behaviour change for anyone carrying extra keys in their config. Not done
  unilaterally.

### No tests yet for

`server`, `api`, `metrics`, `cluster`. `app` has integration-level tests that
pass; the rest have no suite at all.

### In flight (subagents — do not edit these files)

- `internal/zonefile` + `internal/localzone` — repair the truncated
  `localzone_test.go` and fix the four bugs above.
- `internal/engine` + `internal/qlog` — first suites for the query pipeline and
  the query log.
- `README.md` — written from the source, with a "known inconsistencies" list.

Still to delegate after those: `server` + `api`, then `metrics` + `cluster`.

## Continuation plan, in order

1. Land the three in-flight pieces, then re-run
   `go test -race ./internal/... -count=1 -timeout 300s`.
2. `internal/server` suite: all four listeners behind one handler, per-listener
   config, the UDP read-deadline reset (see the design notes), shutdown.
3. `internal/api` suite: every route, the three auth modes, the constant-time
   API-key comparison, `X-Forwarded-For` honoured *only* from `trusted_proxies`,
   CORS, rate limiting, and the SSE stream (open a subscriber, assert
   delivery, unsubscribe, assert no goroutine leak).
4. `internal/metrics` + `internal/cluster`: the collectors, the rolling rings,
   the Valkey lease, pub/sub sync, and divergence detection (a `miniredis`
   instance is the obvious test server).
5. Decide on `KnownFields(true)`.
6. Re-run the console end to end (`/api/healthz`, `/api/whoami`, a page from
   each route) — it was verified once and the API surface has changed since.

## 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. Prefer a new
  immutable snapshot over a lock.
- **Cache invalidation on rule changes flushes the cache** rather than guessing
  which subtrees are affected. `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`.
- **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 (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).
- **`PatternKind.Pattern` is the display form; `WireName` is the wildcard
  form.** The internal `*.` / `**.` prefixes must not leak into the UI, but the
  API needs them to re-add a rule unchanged.

## Test-hygiene traps

- `tsc -b` replays **cached diagnostics** from `node_modules/.tmp/*.tsbuildinfo`.
  After a large edit, `rm node_modules/.tmp/*.tsbuildinfo`.
- `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.
- An agent that runs out of budget mid-append leaves a file that does not
  parse. `gofmt -l` on the package is the fastest way to see it.