199 lines
11 KiB
Markdown
199 lines
11 KiB
Markdown
# 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 |
|
|
| 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.
|
|
- **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.
|
|
- **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.
|
|
- 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.
|