mats/fancy-dns
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
47ef5c1133ebf38217a198410af963da6dcbcb52
fancy-dns/AGENTS.md
T
developer 504512f247 Add a Zones console: create and manage local zones, records and delegations 
The console could not see local zones at all. `blocklist.local_zones` compiled
a store that nothing read back: there was no API for a zone, its records, its
delegations or the file a record came from, and no way to change one without
editing YAML and reloading. The Domains page is the blocklist's inline rules,
not local data, so nothing in the console covered the half of the server that
answers authoritatively from disk. There is now a Zones page and the endpoints
behind it: `GET /api/zones`, `POST`/`PUT`/`DELETE /api/zones/records`,
`POST`/`DELETE /api/zones/delegations` and `DELETE /api/zones?zone=`.

**The console owns one directory and nothing else.** `blocklist.managed_zones`
holds one `<apex>.zone` per zone, with that zone's SOA, apex NS set and the
records added there, and it is compiled by the same `localzone.Load` as the
operator's files — so a record added from the console is served on the next
query, with no reload and no restart. Files in `blocklist.local_zones` are
listed and shown read-only and are never rewritten; the key defaults to empty,
so a deployment that never sets it gets a zone browser and no zone writes.
Which file a record came from is now part of the store (`zone.files`, keyed by
the index the store already uses), so the snapshot can say "console" or "your
file" per record; the query path does not read it.

**The writer proves its own output.** A record is validated by the project's own
master-file parser, so `www` means `www.<zone>` and a bare CNAME target is
completed exactly as it would be in a zone file; the file is rendered, and the
rendering is re-parsed and compared with the records it meant to write before
the file is renamed into place. A writer bug that dropped a record would
otherwise present as a zone that silently stopped answering. A zone that cannot
answer is refused rather than created: no SOA means no NXDOMAIN or NODATA
authority, and no apex NS means nothing is delegated and no server is named. The
SOA's primary follows the apex NS set — a zone whose SOA names a server outside
its own NS set hands out a referral no resolver can follow — and the serial
moves on every write.

**A delegation takes the names under it, so a record below a cut is refused.**
The store answers those names as delegated without reading the zone file, which
would make the record data that looks alive and never answers. `DELETE
/api/zones?zone=` exists for the same reason from the other side: a zone the
console created is a real zone, so it says NXDOMAIN for what it does not hold,
and deleting a record is not what gives a name back to upstream. It is
refused for a zone that also has records outside the managed directory, because
removing that file would remove none of the zone.

**Two bugs the work turned up.** `engine.applyConfig` published local-record
metrics only when `blocklist.local_zones` was non-empty, so a zone the console
created was served but never counted; the gate is now the store's own record
count. And the record-value renderer fell back to `RR.String()` for every type
it did not know, which is the whole line rather than the RDATA — it wrote a
name and a TTL into the value column for DHCID, LOC, SSHFP and SVCB, and would
have fed that back as a record value on the next edit.

Every write recompiles the store and flushes the cache. Which names changed is
not knowable from the API, and a cached upstream answer for a name the zone now
answers is simply wrong, so the flush is blunt on purpose.

The wire shapes are type aliases in `internal/api` rather than copies, so there
is one set of json tags, and `TestZoneEndpointKeySets` pins the key set at every
level with a record and a delegation actually present — an empty list only
proves the envelope. `TestZoneWritesNeedAManagedDirectory` pins the read-only
default, and `TestZoneConsoleRoundTrip` walks the whole thing: a record added
through the API is answered authoritatively on the next query, a cut stops the
parent answering below it, and deleting the zone hands the name back.

Verified against a running instance built from this tree: records and
wildcards answer with `aa`, relative names are qualified, a record below a cut
is refused with a message that says why, and the file on disk is a zone file
an operator can read. The console page is typechecked and built; the browser was
not available in this round, so it has not been looked at.
2026-09-28 07:45:16 +00:00

237 lines
14 KiB
Markdown
Raw Blame History

# AGENTS.md — working on fancy-dns
Project-specific knowledge for anyone (human or agent) changing this repo.
General state and the continuation plan live in `STATUS.md`; read that first for
what is done and what is next. This file is the part that should *not* change
every week: the decisions, the invariants, and the rules that have been learned
the hard way.
## What this is
A recursive DNS server in Go with an embedded React console. Config is YAML plus
RFC 1035 master files, hot-reloaded over SIGHUP. Blocklists in four formats, a
sharded LRU cache with a bbolt snapshot for warm restart, per-client ACLs,
query logging, Prometheus metrics, and optional Valkey/Redis for cross-instance
sync.
## 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, 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.
## Decisions already made — do not relitigate
| 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 |
| Console-written zone data | Its own directory, `blocklist.managed_zones`, one file per apex | the console is a real operator surface, so it needs somewhere to write; a separate directory keeps the operator's master files read-only to it, and an unset key keeps the console read-only by default |
| Cluster sync | Valkey (Redis protocol) | simple, mature Go client, easy local dev |
| Secure DNS | DoT + DoH, **no DNSSEC validation** | deliberately out of scope; AD/CD/extended-RCODE are passed 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 |
| Scale-out | StatefulSet, **no HPA** | an HPA cannot drive a StatefulSet; scale by adding replicas |
| Ingress | A separate ClusterIP Service | a headless Service has no virtual IP for an Ingress to route to |
| Profiling | pprof refused unless admin is loopback | the handlers sit outside the API's auth middleware |
## Invariants
Break these and you break the design, not just the test.
- **Everything the query path reads is behind an `atomic.Pointer` holder** — ACL,
blocklist, localzone, upstream pool. Readers load the pointer once and work
against a value that cannot change under them. **A lock on the hot path is a
regression.** Prefer building a new immutable snapshot and swapping the
pointer.
- **Cache entries are 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.
- **The cache is bounded by bytes as well as by entry count.** `cache.size` alone
bounds nothing: one cached ANY or TXT answer can be tens of kilobytes.
- **Serve-stale** answers are served with TTL 1. An expired upstream answer is
never cached. A truncated answer is never cached — it would be served
truncated forever.
- **The UDP read deadline is reset every iteration.** A deadline set once expires
for good and turns the loop into a spin that never reads another packet.
- **No step in the shutdown may wait for a client.** `Stop` is: cancel, close
every listener (not interruptible — releasing the ports is the work the next
process start needs), then wait for the goroutines (interruptible by the
caller's context). The wait is the step that breaks when something depends on
a client: a connection parked in a read with no deadline in front of it never
notices the cancellation, and one idle connection is enough to spend the whole
grace period and then report a context error. Reach those connections with a
read deadline in the past, **not** a close — a query already inside the
pipeline is past its read, and its answer still has to go out.
- **Every query goroutine is registered with `s.wait`, UDP included**, so `Stop`
returning means nothing is still inside the engine: the app snapshots the
cache and closes the store immediately afterwards, and a query still in flight
would be writing to a store on its way out. A UDP answer has nowhere else to
go, so that transport also gets a short bounded drain before its socket
closes — see `udpDrainGrace` in `internal/server/server.go`.
- **Only the Valkey lease holder fetches blocklists**, so N instances do not hit
a mirror N times. Followers serve their compiled copy.
- **Blocklist changes flush the cache** rather than guessing which subtrees are
affected (`invalidateBlocklistCache()` in `internal/app/hooks.go`).
- **API keys are compared by SHA-256 hash, constant-time**, and
`X-Forwarded-For` is honoured *only* from `trusted_proxies`. Honouring it from
anyone lets a client spoof its identity past the ACL and the rate limiter.
- **The API response field names are shared with the frontend.** If you rename
one, grep `web/src` too.
- **`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.
- **The Go heap does not account for goroutine stacks, runtime metadata, or the
bbolt mmap.** `memory.reserve` exists to cover them; the cache snapshot and
query log are mmapped and live outside the heap entirely.
- **The zone writer proves its own output.** Every record is validated by the
project's own master-file parser, the file is rendered, and the rendering is
re-parsed and compared with the records it meant to write *before* the file
is renamed into place. A writer bug that lost a record would otherwise show
up as a zone that silently stopped answering.
- **A zone the console creates is a real zone.** It is authoritative, so it
answers NXDOMAIN for the names it does not hold, and `DELETE /api/zones` is
the only thing that hands a name back to upstream. Deleting a record is not,
and the API never pretends it is.
- **A delegation takes the names under it.** The store answers them as
delegated without reading the zone file, so a record at or below a cut is
refused rather than written as data that looks alive and never answers.
- **Unknown config keys are silently ignored** (a settled decision, pinned by
`TestLoadIgnoresUnknownKeys` in `internal/config`: `yaml`
`KnownFields(true)` was considered and rejected as a behaviour change for
anyone carrying extra keys). If a config key seems to have no effect, check
the spelling against the struct tags before anything else. The leniency
covers unknown keys only — a known key with a bad *type* is still a startup
failure.
- **`dns.EDNS0_NSID.Nsid` is hex on the wire.** A human-readable value there
builds a message that will not pack, so the client gets SERVFAIL. See the
test-hygiene traps below.
## Working rules
**Never leave the tree red.** You have a hard budget and no way to detect you are
close to it. Whatever is on disk at the end of any round must compile and pass.
Get a few tests running early and keep them running. If you are running long,
stop adding tests and make the existing ones pass — a smaller green suite is worth
far more than a larger red one.
**Scope your edits.** Check `git status` before you start; other work may be in
flight. Stage only the packages you touched. `.codesearch/` is tracked but
gitignored — never stage it. If you edit a package someone else is also editing,
say so in your report rather than silently overwriting.
**Verify before you claim.** `make check-go` is the fast loop (gofmt, vet,
compile, ~1s). `make check` adds the full test suite. Push only a green tree.
**Commit and push when the task says so.** Read `git log --oneline -5` and
`git show --stat HEAD` first and match the style: an imperative subject line, then
one paragraph per bug covering root cause and fix. A commit that says *what* was
changed without saying *why* is not finished work.
**When a test fails, decide which of three things is wrong before editing
anything:** the test, the implementation, or the specification. All three happen
here. Say which one you concluded and why. Do not relax an assertion to make a
suite pass — if an existing assertion pins buggy behaviour, change it and say so
explicitly in the commit message, because that decision belongs in the history.
**Report honestly.** If you did not finish, say what is unfinished and what state
the tree is in. A partial result that is accurately described is usable; a
partial result described as done is not.
## Test-hygiene traps
- `dns.NewRR` needs a TTL column: `dns.NewRR("host.example.com")` does not parse.
A failure there is a bad fixture, not a product bug.
- A test that closes a stop channel *after* `wg.Wait()` deadlocks the whole
package and looks like "the suite hangs". Close first, then wait.
- Assert on invariants, not on what a concurrent goroutine happened to observe.
"A reader must have seen address X" is a flake; "every list a reader sees is one
of the published ones" is a real property and cannot flake.
- **A test that only dials a stream transport races the accept loop.** A
shutdown can close the listener while the connection is still in the backlog,
so the server never accepts it and the test passes for the wrong reason.
Exchange one query first, then leave the connection idle
(`harness.idleStream`).
- **Count leaked goroutines by stack, not by `runtime.NumGoroutine()`.** The
test binary shares its process with net/http, crypto/tls and the race
detector, so a baseline count is noise. "Is any goroutine still running code
in this package?" is a property of the product — and it needs the caller's own
goroutine filtered out by id, or every run finds exactly one.
- 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 `TXT` string over 255 bytes **does not pack** in
miekg/dns v1.1.73 — the error is `dns: string exceeded 255 bytes in txt`, and
255 is the exact boundary. 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.
- **`dns.EDNS0_NSID.Nsid` is a hex string, not the bytes on the wire.** The
library hex-decodes it in `pack` and hex-encodes it in `unpack`, so a plain
value such as `"fancy-dns"` fails with `dns: overflow packing opt` and a
long NSID packs fine (unlike `TXT`). This bit the engine: an unencoded
literal made every query from an NSID-using client SERVFAIL. Always
hex-encode on the way in; `TestPinnedEDNS0NSIDIsHex` in
`internal/engine/pins_test.go` pins the contract.
- `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`.
After a large frontend edit: `rm node_modules/.tmp/*.tsbuildinfo`.
- `vite build` empties `outDir` and **deletes `internal/web/dist/.gitkeep`**.
Restore it or the directory disappears from git.
- The Go binary embeds `internal/web/dist`, so `npm ci && npm run build` must run
before a fresh clone will build a binary with the console in it.
- `make build` depends on `web` and shells out to npm. `make go-build` skips the
console and is what you want in an inner loop.
## Security-sensitive areas
These have been wrong before. Changes here need a test that fails without the fix.
- **Auth** (`internal/api/auth.go`): API key comparison must stay constant-time;
the key is hashed before comparison so the plaintext is not in a long-lived
string.
- **Client identity**: `X-Forwarded-For` is only trusted from `trusted_proxies`,
and `httpRemoteAddr` (used by the DoH handler) deliberately does **not**
consult it at all. Behind an Ingress every DoH client therefore lands in the
proxy's ACL group — fail-closed if that group is restrictive, fail-open if it
is permissive. Pin this behaviour with a test before changing it.
- **Config expansion**: `${VAR}` is expanded in `Load` and an unset variable
with no default is fatal, so a missing Secret is a crash loop rather than an
unauthenticated console. The expansion works inside quoted scalars — that is
the form operators write — and skips YAML comments. Do not "simplify" it into a
plain `os.ExpandEnv`; that silently breaks the API key path.
- **pprof** is loopback-only by construction, so no config flag can turn it into
a network-reachable unauthenticated endpoint. Keep it that way.
- **Amplification**: the DoH/UDP handlers cap the client's advertised EDNS0
buffer. Do not remove the cap.
Reference in new issue View Git Blame Copy Permalink