98b65945f6f9fb0ea1f51da517568ee1452d1647
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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. |
||
|
|
b6d70a50fe |
Finish the server shutdown tests; fix the two bugs they found
build / build-push (push) Successful in 22s
internal/server was the last package whose shutdown was tested by assumption: four transports, three shutdown shapes, and every one of them can strand a goroutine that a client is holding open. Both bugs below were live in the tree as handed over and neither had a test. An idle client could hold the whole shutdown open. The stream accept loop waits for its connections before Stop returns, and a connection parked in ReadMsg with no deadline in front of it only ends when the client goes away — ctx.Err() is checked between reads, so cancelling the context cannot reach a read that is already blocked. One connection that connected and said nothing was enough to make Stop sit there for the caller's entire grace period (15s in main) and then return context.DeadlineExceeded, which the app reports as "shutdown completed with errors"; and the cache snapshot the app takes right after Stop never happened, because the orchestrator had given up first. It is a shutdown DoS that costs the attacker one connection. The fix is a read deadline in the past on every accepted connection when the context is cancelled, and deliberately not a close: a query already inside the pipeline is past its read, so its answer still goes out, which is the invariant TestInFlightQueryIsAnsweredAcrossStop pins. The accept loop keeps a small registry of live connections for this — the accept path, not the query path, and no query ever takes that mutex. A UDP query in flight at Stop was lost and nothing waited for it. The stream paths register their connections with s.wait; servePacket did not register at all, so a UDP query already inside the pipeline was gone twice over: nothing waited for it, and the answer had nowhere to go once it was ready, because a datagram is written to the very socket the query arrived on. The visible damage was not the lost answer (the client retries) but the app, which snapshots the cache and closes the store the moment Stop returns — a query still in the pipeline could be writing to a store on its way out. Packets are now counted in s.wait, and the UDP listener's close wakes the read loop with an expired deadline and then waits, bounded by udpDrainGrace (250ms), for the loop and the packets it spawned before closing the socket. The bound is deliberate: a shutdown may not spend its grace period on one slow upstream, and past the bound the answer is dropped, which is what the client would see anyway. Stop returning did not mean the process was done, so nothing could tell a leaking shutdown from a clean one. TestStopLeavesNoGoroutineRunningServerCode counts goroutines whose stack still has a frame in the package — not runtime.NumGoroutine(), which in a test binary is net/http, crypto/tls and the race detector talking over each other — and it found the idle-connection bug above. TestStopIsNotBlockedByAnIdleStreamConnection sets a 60s DoT idle timeout on purpose so a config value cannot hide the regression behind a lucky tick, and TestInFlightQueryIsAnsweredAcrossStop now covers all four transports. harness.idleStream exists because a test that only dials a stream transport races the accept loop: Stop can close the listener while the connection is still in the backlog, the server never accepts it, and a shutdown assertion that needs a live server-side goroutine passes for the wrong reason. AGENTS.md gains the two durable invariants (no step in the shutdown may wait for a client; every query goroutine is registered with s.wait, UDP included) and the two test-hygiene traps. STATUS.md drops the UDP query lost at Stop from its pinned list, since it is now a test rather than an accident, and the plan starts at the pinned decisions that are actually left. |
||
|
|
e8c5b414d9 |
half broken
build / build-push (push) Failing after 23s
|
||
|
|
4f49033f1b |
Cover the engine, config.Size and metrics; fix the five bugs they found
internal/engine, internal/config and internal/metrics had no test file at all, and writing them turned up five real bugs. internal/engine (23 tests, new harness_test.go): a wired engine over a fake upstream on loopback, a local zone, a blocklist, a cache, a query log and an injectable clock. It pins the pipeline order end to end -- ACL deny before the local zone, local zone before blocklist, blocklist before cache, drop sends nothing, the concurrency and rate-limit refusals, serve-stale served with TTL 1 and refreshed once per key, truncation against a 512-byte client, and exactly one metrics record and one query-log entry per query. Five fixes: - config.ParseSize ignored the unit multiplier, so 512MiB parsed as 512 bytes and the whole memory envelope was inert for anyone who wrote a unit. - finalize (EDNS0 and the UDP size) ran only on the cache-hit path, so a large uncached answer left oversized for a 512-byte client. It now runs on the upstream, local-zone and blocklist paths too. - The upstream UDP client read buffer was hard-coded to 1232 while we advertise dns.max_udp_size, so with a larger buffer configured every answer above 1232 bytes was read truncated and became SERVFAIL. The pool takes the size now. - LRU.prefetchGate was a 64-slot global cap, not the single flight its comment promised: a burst of stale hits launched up to 64 duplicate refreshes of one name. It is a per-key latch now; distinct names still refresh in parallel. - metrics.RecordMiss and RecordStaleHit moved only the Prometheus collector, so the Cache page's misses and stale hits read 0 for the life of the process. AGENTS.md gains the trap that cost the most time: this version of miekg/dns reports a character-string over 255 bytes as zero length, so such a message silently fails to pack, and the write failure looks like a client timeout. |
||
|
|
bbd8692bab |
Add AGENTS.md; bring STATUS.md up to a green tree
AGENTS.md is the part that should not churn: the settled decisions with their reasons, the invariants that hold the design together, the working rules, and the security-sensitive areas that have been wrong before. A new agent should read it and STATUS.md and know what not to re-litigate. Deliberately excluded: anything about how to search, read or patch. Those are being automated per round, and writing advice about them into the repo would rot. What is here instead is project-specific and cannot be automated away — for example that a PTR suffix must match on a label boundary, that a truncated answer must never be cached, and that the UDP read deadline has to be reset every iteration or the loop spins. The working rules are the part worth arguing for. Never leave the tree red, because nobody can detect the budget running out: three of five subagents left broken files, and each cost a round to discover. Report honestly, and when a test fails, decide first whether the test, the implementation or the specification is wrong — all three happen here, and the decision belongs in the commit message. STATUS.md now records a fully green tree: nine packages with suites, all passing under -race. The plan starts with internal/engine, which is the largest uncovered surface and the one every packet goes through. |