fancy-dns/STATUS.md

# fancy-dns — development status and continuation plan

Last updated after the env-expansion fix, the memory envelope and `make
check-go`. Everything below reflects the tree at `4c00bb4`.

## 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 |
| Memory | Soft GOMEMLIMIT, cache byte budget | a cgroup kill is an outage; a cache miss is not |

## Package layout

```
cmd/fancy-dns/main.go          entrypoint: flags, signals, SIGHUP reload
internal/
  config/       Config struct, Duration, Size, 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, memory envelope
  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

### Green

`gofmt` clean, `go build ./...` clean, `go vet` clean.

`go test -race ./internal/... -count=1` is green for **acl, app, blocklist, cache,
config, qlog, resolver, zonefile**.

`make check-go` is the inner loop (1.3 s): gofmt check, vet, compile. It uses
`go-build`, not `build`, because `build` depends on `web` and shells out to npm.
`make check` = `check-go test`.

### Red — one package

**`internal/localzone`**, 5 failures. One is a real production bug:

- **A relative CNAME target is emitted unqualified.** A master file line
  `www CNAME host` produces `www.example.com. CNAME host.` instead of
  `host.example.com.` — the alias points at the DNS root. Per RFC 1035 §5.1 the
  target is relative to the zone origin.

  Narrowed, not yet confirmed: `zonefile.assemble` only calls `relative()` on the
  *owner* and passes RDATA tokens through untouched, and `qualifyRdata(rr,
  z.Origin)` is invoked with the origin that the owner path used successfully.
  So the parser looks correct and the defect is more likely in how
  `internal/localzone` stores or re-serialises records. **Look there first**;
  do not "fix" `qualifyRdata`, it is right.

- `TestCNAMELoopIsNotFollowed` — bad fixture, not a product bug:
  `dns.NewRR("alias.example.com")` has no TTL column so it will not parse.
- `TestLoadOriginDerivation` — a file whose first record has no owner name fails
  with `record has no owner name and no previous owner to inherit`; decide
  whether the parser should accept it when the origin is known.
- `TestLoadMergesFilesForOneOrigin` — `Count() = 3, want 4`. A record is still
  dropped when two files feed one origin. Same class as the bug reproduced by
  hand: a record whose first token is consumed as RDATA of the previous one.

### No test suite yet

`server`, `api`, `metrics`, `cluster`. (`app` has integration tests that pass;
`engine` and `qlog` now have suites.)

### Fixed earlier and worth not re-breaking

- **The cache snapshot 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. `Load` 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.
- **`${VAR}` expansion never worked inside quotes** — which is the only form
  anyone writes. The reference check sat in the same switch as quote tracking,
  below the `inDouble`/`inSingle` branches, so `api_keys: ["${KEY}"]` came out of
  `Load` unchanged and the admin API key was the literal string
  `"${FANCYDNS_API_KEY}"`. The k8s set looked correct and was not. A variable
  that is set but empty now expands to empty rather than erroring (the
  `${X}` / `${X-y}` / `${X:-y}` distinction); `\${FOO}` is literal.
- **`$TTL` was ignored** by the zone parser; every record got the SOA minimum.
- **ACL PTR suffix matched across label boundaries**, so a group scoped to
  `example.com.` also swallowed `ample.com.` — a client could land in a group it
  does not belong to.
- **The ACL `*` catch-all was dropped** from the rendered domain patterns.
- **`Upstream.Success` never closed an open breaker.** A recovered upstream
  stayed excluded for the full cooldown — the mirror of the older bug where a
  failed exchange marked a dead upstream healthy.
- **`Exchange` fanned out in parallel even under `policy: failover`.**
- **The cache byte budget was never fed to the LRU.** `maxBytes` was enforced
  per shard on insert but had no setter and nothing called one, so `cache.size`
  had no predictable memory cost.
- **The memory envelope** (`memory.limit/reserve/cache_max_bytes`) is new. The
  reserve exists because the Go heap does not account for goroutine stacks,
  runtime metadata, or the bbolt mmap for the cache snapshot and the query log.
- **Blocklist:** leading BOM stripped, `!allow` works, `SourceMeta` exists for a
  rule-only source, `Pattern()` reports the bare name (`WireName()` gives the
  wildcard form for the API), RPZ inherits the zone default TTL, hosts files
  complain about a missing address, adblock handles CRLF and over-long lines.
- **`TestHolderSwapIsAtomic` had the wrong invariant**, not the Holder.
- **CI could not have passed:** `go.mod` requires Go 1.27.1, CI pinned 1.23 and
  the Dockerfile pinned `golang:1.23-alpine`.
- **The Dockerfile clobbered the console** — it copied the source tree after the
  console bundle.
- **The k8s set could not have worked:** `hostNetwork: true` with `replicas: 2`
  and no SO_REUSEPORT. Now a LoadBalancer Service, a separate ClusterIP Service
  for the Ingress, a ServiceAccount, an API-key Secret, per-pod PVCs, a
  kustomization. The HPA is gone — it cannot drive a StatefulSet.
- **`serveUDP` called `net.ResolveUDPAddr` per packet**; the peer already is a
  `*net.UDPAddr`.
- **`api.pprof`** mounts `net/http/pprof`, refused unless every admin address is
  loopback — the handlers sit on the app-level mux and would bypass the API's own
  auth.

### Open decisions

- **Unknown config keys are silently ignored.** Writing `block:` where the struct
  wants `blocklist:` produced a plausible-looking server. `KnownFields(true)`
  would reject it, but that is a behaviour change for anyone carrying extra
  keys. Two test fixtures were caught by this, not by the thing they tested.
- **DoH clients behind an Ingress all present as the proxy.** `httpRemoteAddr`
  uses the raw `RemoteAddr` and never consults `trusted_proxies`, so every DoH
  client lands in the Ingress's ACL group. Fail-closed if that group is
  restrictive, fail-open if it is permissive. Pin the behaviour with a test
  before changing it.

## Continuation plan, in order

1. **Unblock `internal/localzone`.** Start with how the Store re-serialises
   records; the parser side looks correct. Then the fixture, the origin
   derivation and the merge count.
2. `internal/server` suite: all four listeners behind one handler, per-listener
   config, the UDP read-deadline reset, shutdown ordering.
3. `internal/api` suite: every route, the three auth modes, 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, divergence detection. `miniredis` is the
   obvious test server.
5. Tests for `config.Size` — the newest parsing code in the repo and the most
   likely to be wrong at a boundary like `1.5GiB`.
6. Wire `memory:` into `deploy/k8s/00-config.yaml`, matched to the StatefulSet's
   1Gi limit.
7. Decide on `KnownFields(true)`.
8. Re-run the console end to end; the API surface has changed several times.

## 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`.
- **The UDP listener resets its read deadline every iteration.** A deadline set
  once expires for good 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.
- **Serve-stale** answers get TTL 1; an expired upstream answer is never cached;
  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.
- **pprof is loopback-only by construction**, not by policy, so no profiler flag
  can turn it into a network-reachable unauthenticated endpoint.

## Test-hygiene traps

- **Subagent briefs must require "whatever is on disk when you run out of
  budget must compile and pass".** Three of five agents left a red tree; the
  check has to be in the brief, because an agent cannot know it is out of budget.
- `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.
- `gofmt -l internal/` is the fastest way to see that an agent left a file that
  does not parse.
- `dns.NewRR` needs a TTL column. `dns.NewRR("host.example.com")` does not
  parse; that is a fixture bug, not a product bug.