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.

### 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.