feat: Add admin DNS record management API#222
Open
ZPascal wants to merge 3 commits into
Open
Conversation
6 tasks
There was a problem hiding this comment.
Pull request overview
Adds an authenticated admin HTTP surface and backing storage to manage DNS records, and wires those records into the DNS response path as a fallback when no static/ACME records match.
Changes:
- Introduce
/admin/recordsCRUD endpoints guarded by an admin bearer token, enabled only when[api.admin].tokenis set. - Add
dns_recordspersistence and integrate DB-backed records into DNS answers as a fallback path. - Add record type/TTL/value validators and related unit tests; extend CORS configuration for admin endpoints.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
main.go |
Registers admin routes conditionally; expands CORS methods/headers. |
api.go |
Implements admin bearer middleware and record CRUD handlers. |
db.go |
Adds dns_records DDL and CRUD methods. |
dns.go |
Falls through to DB-managed records when static/ACME records don’t match. |
types.go |
Adds [api.admin] config struct, DNSRecord, and DB interface extensions. |
validation.go |
Adds record type/value/TTL validation helpers. |
validation_test.go |
Adds unit tests for the new validation helpers. |
config.cfg |
Adds [api.admin] section and token configuration. |
docs/superpowers/specs/2026-05-30-dns-api-extended-design.md |
Design doc describing the admin DNS management API and DNS integration. |
docs/superpowers/specs/2026-05-30-security-ha-design.md |
Security/HA design doc (appears unrelated to this PR’s stated scope). |
docs/superpowers/specs/2026-05-30-ai-agent-support-design.md |
AI agent support design doc (appears unrelated to this PR’s stated scope). |
docs/superpowers/plans/2026-05-30-security-ha.md |
Security/HA implementation plan (appears unrelated to this PR’s stated scope). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ZPascal
force-pushed
the
feature/dns-api-extended
branch
2 times, most recently
from
e929f6d to
c941f89
Compare
ZPascal
force-pushed
the
feature/dns-api-extended
branch
from
c711231 to
73c8bf9
Compare
ZPascal
changed the title
ZPascal
force-pushed
the
feature/dns-api-extended
branch
from
73c8bf9 to
f608c89
Compare
* fix: tighten record validators and add missing test cases
* feat: add dns_records table and CRUD methods to DB
* fix: rows.Err check, UpdateRecord not-found sentinel, DNSRecord to types.go
* feat: add admin HTTP handlers for DNS record CRUD
* feat: register admin routes with Bearer middleware in HTTP API
* feat: extend DNS server to serve managed records from dns_records table
* fix: normalize DNS record names to lowercase on ingress
* fix: add mutex protection to SetBackend to eliminate data race with ListRecords
* fix: address PR review — CORS headers, FQDN normalization, TXT quoting, RR validation, DB index
- CORS: restore X-Api-User and X-Api-Key to AllowedHeaders (required by /update endpoint)
- api: normalize record names to FQDN (trailing dot) on create/update/list-filter so DNS
lookups against q.Name (which always ends with dot) match stored records
- api: normalize type filter to uppercase for case-insensitive GET /admin/records?type=
- api: add probeRR helper — validates record value parses as a real DNS RR before storing,
preventing values that are silently dropped at serve time
- dns: guard answerManaged against unknown QTYPEs (empty TypeToString result)
- dns: quote TXT and CAA values in RR string so values with spaces parse correctly
- db: add index on dns_records(name, type) to avoid full table scans on every DNS request
- fix: replace full-table scan in adminUpdateRecord with GetRecord by ID
- Add GetRecord(id) DB method with SELECT WHERE id=$1 to replace the
O(N) ListRecords("","") scan used to fetch the updated record
- Eliminates TOCTOU window where concurrent delete between UPDATE and
re-fetch would incorrectly return HTTP 500 instead of 404
- Add GetRecord to database interface and acmedb implementation
- Bump DBVersion to 2 and add handleDBUpgradeTo2 migration for the
dns_records schema change
- Set authoritative=true when answerManaged returns records, so DNS
responses for DB-managed records carry the AA bit
- Remove Lock/Unlock from SetBackend to prevent deadlock when callers
swap backends from within a locked test context
- Fix db_test.go fixtures to use FQDN names with trailing dot matching
how toFQDN() stores records through the API
- test: add missing admin API test coverage
- TestAdminRoutesAbsentWithNoToken: verifies admin routes return 404
when token is empty, mirroring the main.go conditional registration
- TestAdminUpdatePreservesCreated: verifies PUT response includes the
original created timestamp (regression for the GetRecord fix)
- fix: remove dead token=="" branch and add probeRR HTTP-layer test
Signed-off-by: ZPascal <pascal.zimmermann@theiotstudio.com>
ZPascal
force-pushed
the
feature/dns-api-extended
branch
from
c4045a1 to
360a8ae
Compare
handleDBUpgrades now loops through all pending versions rather than returning after a single step, so a v0 database correctly runs both handleDBUpgradeTo1 and handleDBUpgradeTo2 before reaching DBVersion 2. stripOuterQuotes() removes any surrounding double-quotes from TXT/CAA values before storage, ensuring the DB always holds raw string content. answerManaged already re-wraps values at serve time, so submitting a pre-quoted value (e.g. '"token"') no longer results in corrupt wire data.
…records probeRR and answerManaged were applying TXT-style double-quote wrapping to CAA records. CAA wire format is "0 issue <value>" — wrapping the entire value in quotes caused dns.NewRR to reject every CAA record. Remove CAA from the quoting logic; only TXT needs it. answerManaged now returns ([]dns.RR, bool) where the bool signals whether the queried name exists at all (in any record type). answer() uses this to set NOERROR when the name exists but no records match the queried type (RFC 2308 NODATA), rather than leaving the rcode as NXDOMAIN. Add TestAnswerManagedNODATA, TestAnswerManagedCAARecord (dns_test.go) and TestAdminCreateCAARecord (api_test.go) to cover both fixes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test Plan