diff --git a/plugin.go b/plugin.go index dfeb5f8..8891dfb 100644 --- a/plugin.go +++ b/plugin.go @@ -1,61 +1,67 @@ -// Package rfc2136 implements a CoreDNS plugin that accepts dynamic DNS -// updates per RFC 2136 (UPDATE opcode), authenticated via TSIG. The -// primary use case is self-hosted ACME DNS-01 cert automation: an ACME -// client (e.g. Caddy via caddy-dns/rfc2136) injects _acme-challenge TXT -// records into a delegated sub-zone that this plugin serves. +// Package rfc2136 is a CoreDNS plugin that accepts dynamic DNS updates +// per RFC 2136 (UPDATE opcode), authenticated via TSIG, and applies +// them to on-disk zone files. This is the right shape for stacks where +// the operator wants to keep zones in flat files (perhaps under git, +// with HE pulling AXFR), but also wants programmatic updates from +// clients like Caddy's caddy-dns/rfc2136 module. // -// Phase 1.3 status: store + query dispatch. ServeDNS now answers -// authoritatively for the configured zone(s) from the in-memory store -// (plus synthetic SOA/NS at apex). UPDATE handling still rejected — -// that lands in Phase 1.4. See plan at +// The plugin does NOT serve any queries — that's the job of the +// `auto`/`file` plugin running alongside it. This plugin's only +// responsibility is the UPDATE opcode path: verify TSIG, dissect the +// UPDATE, write the zone file, bump the SOA serial, optionally +// auto-commit to git. CoreDNS's auto plugin notices the mtime change +// and re-serves the zone within its reload interval. // -// ~/.claude/plans/dood-does-coredns-offer-enumerated-piglet.md +// See the plan at +// ~/.claude/plans/dood-does-coredns-offer-enumerated-piglet.md +// for the architectural rationale. package rfc2136 import ( "context" - "strings" "github.com/coredns/coredns/plugin" "github.com/miekg/dns" ) -// DefaultTTL is the TTL applied to dynamically-added records when the -// Corefile doesn't specify one. 60s matches the short-lived nature of -// ACME challenge TXT records and keeps stale answers from lingering in +// DefaultTTL is applied to dynamically-added records whose UPDATE +// messages carry TTL=0. 60s matches the short-lived nature of ACME +// challenge records and keeps stale answers from lingering in // resolver caches. const DefaultTTL uint32 = 60 // RFC2136 is the plugin handler. One instance per Corefile server block. type RFC2136 struct { - // Next is the downstream plugin in the chain. + // Next is the downstream plugin in the chain — queries always + // pass through; only UPDATE opcode is intercepted. Next plugin.Handler // Zones is the set of canonical (dot-terminated, lowercase) zone - // names this instance is authoritative for. Queries outside these - // zones pass through to Next. + // names this instance accepts UPDATEs for. UPDATEs for any other + // zone are rejected with NOTAUTH. Zones []string // TSIGKeys is keyed by canonical key name (lowercased, trailing - // dot). Empty means TSIG is disabled — UPDATEs without TSIG are - // rejected unconditionally in Phase 1.4. + // dot). Empty means TSIG is disabled — UPDATEs are refused + // unconditionally as a safety default. TSIGKeys map[string]tsigKey // TTL is applied to dynamically-injected records that don't carry // an explicit TTL in the UPDATE message. TTL uint32 - // PersistPath, when non-empty, names a file the plugin writes a - // JSON snapshot of its in-memory store to on a periodic schedule. - // Empty means in-memory only (acceptable for ACME challenges). - PersistPath string + // ZonesDir is the directory where .zone files live (matching + // the mount path inside the CoreDNS container). The plugin reads + // and writes files at /.zone. + ZonesDir string - // Nameserver is the host returned in synthetic NS records and as - // the SOA's MNAME. Defaults (set in setup) to the first zone apex. - Nameserver string + // AutoCommit governs whether the plugin auto-commits zone-file + // changes to git after every successful UPDATE. + AutoCommit bool - // store holds the dynamic records. Always non-nil after setup. - store *recordStore + // zones holds per-zone file handlers, keyed by canonical zone name. + // Populated in setup; mutexes live inside each zoneFile. + zones map[string]*zoneFile } // Name implements plugin.Handler. @@ -65,19 +71,11 @@ func (p *RFC2136) Name() string { return "rfc2136" } // // Dispatch: // -// UPDATE opcode → rejected with REFUSED (Phase 1.4 implements properly). -// Query opcode: -// - Not in our zones → pass to Next. -// - Apex SOA → synthetic SOA. -// - Apex NS → synthetic NS. -// - Match in store → return RRset. -// - Name exists, wrong type → NODATA (NOERROR + SOA in authority). -// - Name doesn't exist → NXDOMAIN (NameError + SOA in authority). +// UPDATE opcode → verify TSIG, then apply via the UPDATE handler. +// Anything else → pass through to Next (the auto plugin handles +// queries against the zone files we maintain). func (p *RFC2136) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { if r.Opcode == dns.OpcodeUpdate { - // TSIG verification was performed by the underlying dns.Server - // (because setup.go populated dnsserver.Config.TsigSecret). We - // just need to check the result here. if err := p.checkTSIG(w, r); err != nil { log.Warningf("UPDATE rejected: %v", err) resp := new(dns.Msg) @@ -87,123 +85,5 @@ func (p *RFC2136) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg } return p.handleUpdate(w, r) } - - if len(r.Question) == 0 { - return plugin.NextOrFailure(p.Name(), p.Next, ctx, w, r) - } - - q := r.Question[0] - zone := p.findZone(q.Name) - if zone == "" { - return plugin.NextOrFailure(p.Name(), p.Next, ctx, w, r) - } - - // We're authoritative for this name. Build a reply. - msg := new(dns.Msg) - msg.SetReply(r) - msg.Authoritative = true - - qname := strings.ToLower(dns.Fqdn(q.Name)) - isApex := qname == zone - - // Apex SOA / NS are synthetic. - if isApex { - switch q.Qtype { - case dns.TypeSOA: - msg.Answer = []dns.RR{p.syntheticSOA(zone)} - _ = w.WriteMsg(msg) - return dns.RcodeSuccess, nil - case dns.TypeNS: - msg.Answer = p.syntheticNS(zone) - _ = w.WriteMsg(msg) - return dns.RcodeSuccess, nil - } - } - - // Look up the asked type in the store. - if rrs := p.store.Lookup(qname, q.Qtype); rrs != nil { - msg.Answer = rrs - _ = w.WriteMsg(msg) - return dns.RcodeSuccess, nil - } - - // Special case: ANY at the apex isn't in the store but we have - // synthetic SOA + NS. Return them rather than NODATA. - if isApex && q.Qtype == dns.TypeANY { - msg.Answer = append(msg.Answer, p.syntheticSOA(zone)) - msg.Answer = append(msg.Answer, p.syntheticNS(zone)...) - _ = w.WriteMsg(msg) - return dns.RcodeSuccess, nil - } - - // Distinguish NODATA from NXDOMAIN. - if p.store.NameExists(qname) || isApex { - // NODATA: name exists, but not with this type. - msg.Ns = []dns.RR{p.syntheticSOA(zone)} - _ = w.WriteMsg(msg) - return dns.RcodeSuccess, nil - } - - // NXDOMAIN: name doesn't exist anywhere in this zone. - msg.Rcode = dns.RcodeNameError - msg.Ns = []dns.RR{p.syntheticSOA(zone)} - _ = w.WriteMsg(msg) - return dns.RcodeNameError, nil -} - -// findZone returns the longest matching zone for qname, or "" if qname -// is outside all configured zones. The returned zone is in canonical -// form (lowercase, trailing dot). -func (p *RFC2136) findZone(qname string) string { - qname = strings.ToLower(dns.Fqdn(qname)) - // Longest-suffix wins so nested zones work correctly. - var best string - for _, z := range p.Zones { - if qname == z || strings.HasSuffix(qname, "."+z) { - if len(z) > len(best) { - best = z - } - } - } - return best -} - -// syntheticSOA returns the SOA RR for a zone. Serial is derived from -// the store's monotonic generation counter — every UPDATE bumps it, -// so downstream observers can detect "something changed" without -// having to AXFR. -func (p *RFC2136) syntheticSOA(zone string) *dns.SOA { - return &dns.SOA{ - Hdr: dns.RR_Header{ - Name: zone, - Rrtype: dns.TypeSOA, - Class: dns.ClassINET, - Ttl: p.TTL, - }, - Ns: p.Nameserver, - Mbox: "admin." + zone, - Serial: uint32(p.store.generation()), - Refresh: 3600, // 1 hour - Retry: 600, // 10 min - Expire: 604800, // 1 week - Minttl: 60, // negative-cache TTL - } -} - -// syntheticNS returns the NS RRset for a zone. Currently a single NS -// pointing at p.Nameserver (the host that runs this plugin). For -// resiliency, future versions could accept multiple `nameserver` -// directives. -func (p *RFC2136) syntheticNS(zone string) []dns.RR { - return []dns.RR{ - &dns.NS{ - Hdr: dns.RR_Header{ - Name: zone, - Rrtype: dns.TypeNS, - Class: dns.ClassINET, - Ttl: p.TTL, - }, - Ns: p.Nameserver, - }, - } + return plugin.NextOrFailure(p.Name(), p.Next, ctx, w, r) } diff --git a/plugin_test.go b/plugin_test.go deleted file mode 100644 index 3aa40af..0000000 --- a/plugin_test.go +++ /dev/null @@ -1,224 +0,0 @@ -package rfc2136 - -import ( - "context" - "net" - "testing" - - "github.com/coredns/coredns/plugin" - "github.com/miekg/dns" -) - -// captureWriter implements dns.ResponseWriter and stashes the message -// passed to WriteMsg so tests can inspect it after ServeDNS returns. -type captureWriter struct { - msg *dns.Msg -} - -func (cw *captureWriter) WriteMsg(m *dns.Msg) error { cw.msg = m; return nil } -func (cw *captureWriter) Write([]byte) (int, error) { return 0, nil } -func (cw *captureWriter) Close() error { return nil } -func (cw *captureWriter) TsigStatus() error { return nil } -func (cw *captureWriter) TsigTimersOnly(bool) {} -func (cw *captureWriter) Hijack() {} -func (cw *captureWriter) LocalAddr() net.Addr { return &net.IPAddr{IP: net.IPv4(127, 0, 0, 1)} } -func (cw *captureWriter) RemoteAddr() net.Addr { return &net.IPAddr{IP: net.IPv4(127, 0, 0, 1)} } -func (cw *captureWriter) Network() string { return "udp" } - -// passthroughNext is a stand-in for the next plugin in the chain. -// Returns a fixed rcode so we can detect "we passed through" in tests. -type passthroughNext struct{ called bool } - -func (n *passthroughNext) ServeDNS(_ context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { - n.called = true - msg := new(dns.Msg) - msg.SetReply(r) - msg.Rcode = dns.RcodeRefused // arbitrary marker - _ = w.WriteMsg(msg) - return dns.RcodeRefused, nil -} -func (n *passthroughNext) Name() string { return "passthroughNext" } - -// newTestPlugin builds an RFC2136 with sensible defaults for tests. -func newTestPlugin(zone, ns string, next plugin.Handler) *RFC2136 { - return &RFC2136{ - Next: next, - Zones: []string{dns.Fqdn(zone)}, - TTL: 60, - Nameserver: dns.Fqdn(ns), - store: newStore(), - } -} - -func TestServeDNS_OutsideZone_PassesThrough(t *testing.T) { - next := &passthroughNext{} - p := newTestPlugin("auth.example.com.", "ns.example.com.", next) - - req := new(dns.Msg) - req.SetQuestion("unrelated.other.tld.", dns.TypeA) - - w := &captureWriter{} - rcode, _ := p.ServeDNS(context.Background(), w, req) - - if !next.called { - t.Errorf("expected pass-through to Next, but Next was not called") - } - if rcode != dns.RcodeRefused { - t.Errorf("rcode = %d (want %d from passthroughNext marker)", rcode, dns.RcodeRefused) - } -} - -func TestServeDNS_ApexSOA_Synthetic(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - - req := new(dns.Msg) - req.SetQuestion("auth.example.com.", dns.TypeSOA) - - w := &captureWriter{} - rcode, _ := p.ServeDNS(context.Background(), w, req) - - if rcode != dns.RcodeSuccess { - t.Fatalf("rcode = %d, want NOERROR", rcode) - } - if w.msg == nil || !w.msg.Authoritative { - t.Fatalf("response not authoritative: %+v", w.msg) - } - if len(w.msg.Answer) != 1 { - t.Fatalf("Answer len = %d, want 1", len(w.msg.Answer)) - } - soa, ok := w.msg.Answer[0].(*dns.SOA) - if !ok { - t.Fatalf("answer is not SOA: %T", w.msg.Answer[0]) - } - if soa.Ns != "ns.example.com." { - t.Errorf("SOA.Ns = %q, want ns.example.com.", soa.Ns) - } - if soa.Mbox != "admin.auth.example.com." { - t.Errorf("SOA.Mbox = %q, want admin.auth.example.com.", soa.Mbox) - } -} - -func TestServeDNS_ApexNS_Synthetic(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - - req := new(dns.Msg) - req.SetQuestion("auth.example.com.", dns.TypeNS) - - w := &captureWriter{} - p.ServeDNS(context.Background(), w, req) - - if len(w.msg.Answer) != 1 { - t.Fatalf("Answer len = %d, want 1", len(w.msg.Answer)) - } - ns, ok := w.msg.Answer[0].(*dns.NS) - if !ok { - t.Fatalf("answer is not NS: %T", w.msg.Answer[0]) - } - if ns.Ns != "ns.example.com." { - t.Errorf("NS.Ns = %q", ns.Ns) - } -} - -func TestServeDNS_ExistingTXT_Returned(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - p.store.Add(mustRR(t, `foo.auth.example.com. 60 IN TXT "token-1"`)) - - req := new(dns.Msg) - req.SetQuestion("foo.auth.example.com.", dns.TypeTXT) - - w := &captureWriter{} - rcode, _ := p.ServeDNS(context.Background(), w, req) - - if rcode != dns.RcodeSuccess { - t.Fatalf("rcode = %d, want NOERROR", rcode) - } - if len(w.msg.Answer) != 1 { - t.Fatalf("Answer len = %d, want 1", len(w.msg.Answer)) - } - txt := w.msg.Answer[0].(*dns.TXT) - if txt.Txt[0] != "token-1" { - t.Errorf("TXT = %q, want token-1", txt.Txt[0]) - } -} - -func TestServeDNS_NonExistentName_NXDOMAIN(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - - req := new(dns.Msg) - req.SetQuestion("missing.auth.example.com.", dns.TypeTXT) - - w := &captureWriter{} - rcode, _ := p.ServeDNS(context.Background(), w, req) - - if rcode != dns.RcodeNameError { - t.Errorf("rcode = %d, want NXDOMAIN (%d)", rcode, dns.RcodeNameError) - } - if len(w.msg.Answer) != 0 { - t.Errorf("expected empty Answer for NXDOMAIN, got %v", w.msg.Answer) - } - if len(w.msg.Ns) != 1 { - t.Errorf("expected SOA in authority section, got %v", w.msg.Ns) - } - if _, ok := w.msg.Ns[0].(*dns.SOA); !ok { - t.Errorf("authority section is not SOA: %T", w.msg.Ns[0]) - } -} - -func TestServeDNS_WrongType_NODATA(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - p.store.Add(mustRR(t, `foo.auth.example.com. 60 IN A 192.0.2.1`)) - - req := new(dns.Msg) - req.SetQuestion("foo.auth.example.com.", dns.TypeTXT) - - w := &captureWriter{} - rcode, _ := p.ServeDNS(context.Background(), w, req) - - if rcode != dns.RcodeSuccess { - t.Errorf("rcode = %d, want NOERROR (NODATA)", rcode) - } - if len(w.msg.Answer) != 0 { - t.Errorf("NODATA must have empty Answer, got %v", w.msg.Answer) - } - if len(w.msg.Ns) != 1 { - t.Errorf("expected SOA in authority for NODATA") - } -} - -func TestServeDNS_UpdateOpcode_Refused(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - - req := new(dns.Msg) - req.SetUpdate("auth.example.com.") - - w := &captureWriter{} - rcode, _ := p.ServeDNS(context.Background(), w, req) - - if rcode != dns.RcodeRefused { - t.Errorf("UPDATE rcode = %d, want REFUSED (%d)", rcode, dns.RcodeRefused) - } -} - -func TestFindZone_LongestSuffixWins(t *testing.T) { - p := &RFC2136{ - Zones: []string{"example.com.", "auth.example.com."}, - } - got := p.findZone("foo.auth.example.com.") - if got != "auth.example.com." { - t.Errorf("findZone returned %q, expected longest-match auth.example.com.", got) - } -} - -func TestFindZone_OutsideAllZones(t *testing.T) { - p := &RFC2136{Zones: []string{"auth.example.com."}} - if got := p.findZone("other.tld."); got != "" { - t.Errorf("findZone for unrelated qname returned %q, want empty", got) - } -} - -func TestFindZone_CaseInsensitive(t *testing.T) { - p := &RFC2136{Zones: []string{"auth.example.com."}} - if got := p.findZone("Foo.AUTH.example.COM."); got != "auth.example.com." { - t.Errorf("case-insensitive findZone returned %q", got) - } -} diff --git a/setup.go b/setup.go index 131ba5e..e868c98 100644 --- a/setup.go +++ b/setup.go @@ -2,13 +2,15 @@ package rfc2136 import ( "encoding/base64" + "fmt" + "os" + "path/filepath" "strconv" "github.com/coredns/caddy" "github.com/coredns/coredns/core/dnsserver" "github.com/coredns/coredns/plugin" clog "github.com/coredns/coredns/plugin/pkg/log" - "github.com/miekg/dns" ) // log is the package logger, scoped so messages are prefixed `[rfc2136]`. @@ -19,25 +21,24 @@ func init() { } // setup is invoked by the CoreDNS plugin registry once per Corefile -// `rfc2136` directive. It parses the directive's arguments and block, -// constructs an RFC2136 handler, and links it into the plugin chain. +// `rfc2136` directive. It parses the directive, validates that each +// declared zone has a corresponding file in zones-dir, registers +// TSIG keys with the underlying dns.Server, and links the handler +// into the plugin chain. func setup(c *caddy.Controller) error { p, err := parse(c) if err != nil { return plugin.Error("rfc2136", err) } + if err := p.validateZoneFiles(); err != nil { + return plugin.Error("rfc2136", err) + } cfg := dnsserver.GetConfig(c) - // Register our TSIG keys with the underlying dns.Server so miekg/dns + // Register TSIG keys with the underlying dns.Server so miekg/dns // auto-verifies incoming signatures. We then just inspect the - // verification result via dns.ResponseWriter.TsigStatus() in our - // UPDATE handler — no need to do MAC arithmetic ourselves. - // - // dns.Server.TsigSecret expects base64-encoded secrets, so we - // re-encode (the parser decoded them at Corefile-load time, and - // keeping the raw bytes lets future code do other things with - // them). + // result via dns.ResponseWriter.TsigStatus() in our UPDATE handler. if len(p.TSIGKeys) > 0 { if cfg.TsigSecret == nil { cfg.TsigSecret = make(map[string]string) @@ -52,38 +53,37 @@ func setup(c *caddy.Controller) error { return p }) - log.Infof("registered for zones=%v keys=%d ttl=%d persist=%q", - p.Zones, len(p.TSIGKeys), p.TTL, p.PersistPath) + log.Infof("ready: zones=%v keys=%d ttl=%d dir=%q auto-commit=%t", + p.Zones, len(p.TSIGKeys), p.TTL, p.ZonesDir, p.AutoCommit) return nil } -// parse reads a single `rfc2136 [...] { ... }` block from -// the Corefile and returns a fully-populated RFC2136 handler with all -// values validated at parse time (so configuration errors fail fast at -// CoreDNS startup, not later mid-request). +// parse reads a single `rfc2136 [...] { ... }` block. // // Grammar: // // rfc2136 [...] { +// zones-dir ; required // tsig-key ; may repeat // ttl ; default 60 -// persist ; default off (in-memory only) +// auto-commit ; default true +// git-author ; optional // } func parse(c *caddy.Controller) (*RFC2136, error) { p := &RFC2136{ - TSIGKeys: make(map[string]tsigKey), - TTL: DefaultTTL, - store: newStore(), + TSIGKeys: make(map[string]tsigKey), + TTL: DefaultTTL, + AutoCommit: true, } + // Per-zone git author overrides. Defaults are applied later. + var gitAuthorName, gitAuthorEmail string + for c.Next() { args := c.RemainingArgs() if len(args) < 1 { return nil, c.ArgErr() } - // Normalize each declared zone to lowercase + trailing dot - // (CoreDNS canonical form). This makes later zone-membership - // checks an exact match against r.Question[0].Name. for _, z := range args { p.Zones = append(p.Zones, plugin.Host(z).NormalizeExact()...) } @@ -91,15 +91,14 @@ func parse(c *caddy.Controller) (*RFC2136, error) { for c.NextBlock() { switch c.Val() { - case "nameserver": - nArgs := c.RemainingArgs() - if len(nArgs) != 1 { + case "zones-dir": + dArgs := c.RemainingArgs() + if len(dArgs) != 1 { return nil, c.ArgErr() } - p.Nameserver = dns.Fqdn(nArgs[0]) + p.ZonesDir = dArgs[0] case "tsig-key": - // tsig-key kArgs := c.RemainingArgs() if len(kArgs) != 3 { return nil, c.Errf("tsig-key requires 3 args (name algorithm secret), got %d", len(kArgs)) @@ -127,17 +126,29 @@ func parse(c *caddy.Controller) (*RFC2136, error) { if err != nil { return nil, c.Errf("ttl must be a non-negative integer: %v", err) } - // Anything over a week is almost certainly a mistake - // for ACME challenge records, but allow up to the - // uint32 max so we don't ship an arbitrary cap. p.TTL = uint32(ttl) - case "persist": - pArgs := c.RemainingArgs() - if len(pArgs) != 1 { + case "auto-commit": + aArgs := c.RemainingArgs() + if len(aArgs) != 1 { return nil, c.ArgErr() } - p.PersistPath = pArgs[0] + switch aArgs[0] { + case "true", "yes", "on": + p.AutoCommit = true + case "false", "no", "off": + p.AutoCommit = false + default: + return nil, c.Errf("auto-commit must be true|false, got %q", aArgs[0]) + } + + case "git-author": + gArgs := c.RemainingArgs() + if len(gArgs) != 2 { + return nil, c.Errf("git-author requires 2 args (name email), got %d", len(gArgs)) + } + gitAuthorName = gArgs[0] + gitAuthorEmail = gArgs[1] default: return nil, c.Errf("unknown directive: %s", c.Val()) @@ -148,14 +159,45 @@ func parse(c *caddy.Controller) (*RFC2136, error) { if len(p.Zones) == 0 { return nil, c.Err("at least one zone must be specified") } + if p.ZonesDir == "" { + return nil, c.Err("zones-dir is required") + } - // Default nameserver to the first zone apex. The user can override - // via the `nameserver` directive — e.g. when the delegating parent - // zone publishes `auth NS dns.supported.systems`, this should be - // set to `dns.supported.systems.` to match. - if p.Nameserver == "" { - p.Nameserver = p.Zones[0] + // Build zoneFile handles for each declared zone. + p.zones = make(map[string]*zoneFile, len(p.Zones)) + for _, z := range p.Zones { + // Trailing dot → filename. supported.systems. → supported.systems.zone + stem := z + if l := len(stem); l > 0 && stem[l-1] == '.' { + stem = stem[:l-1] + } + path := filepath.Join(p.ZonesDir, stem+".zone") + zf := openZoneFile(path, z) + zf.AutoCommit = p.AutoCommit + if gitAuthorName != "" { + zf.GitAuthorName = gitAuthorName + } + if gitAuthorEmail != "" { + zf.GitAuthorEmail = gitAuthorEmail + } + p.zones[z] = zf } return p, nil } + +// validateZoneFiles ensures every configured zone has an accessible +// file on disk at the expected path. Catches typos at CoreDNS startup +// rather than the first UPDATE. +func (p *RFC2136) validateZoneFiles() error { + for zone, zf := range p.zones { + st, err := os.Stat(zf.Path) + if err != nil { + return fmt.Errorf("zone %q: file not accessible at %s: %w", zone, zf.Path, err) + } + if st.IsDir() { + return fmt.Errorf("zone %q: %s is a directory, expected a regular file", zone, zf.Path) + } + } + return nil +} diff --git a/setup_test.go b/setup_test.go index 2bcbce2..a00379e 100644 --- a/setup_test.go +++ b/setup_test.go @@ -1,6 +1,8 @@ package rfc2136 import ( + "os" + "path/filepath" "strings" "testing" @@ -13,93 +15,126 @@ import ( // value in production — it's literally in this file in plaintext. const testSecret = "xTgset4zj7kHqniSslYFn+OcdCf419olek9MNmOvlUM=" -// TestParse exercises the Corefile parser across the matrix of valid -// and invalid configurations. Each row is fully self-contained. +// withTempZonesDir creates a zones-dir with the named .zone files +// (each containing a minimal valid zone) and returns the directory +// path plus a cleanup func. The minimal zone has an SOA + NS, which +// satisfies validateZoneFiles(). +func withTempZonesDir(t *testing.T, zones ...string) string { + t.Helper() + dir := t.TempDir() + for _, z := range zones { + path := filepath.Join(dir, strings.TrimSuffix(z, ".")+".zone") + content := minimalZone(z) + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", path, err) + } + } + return dir +} + +func minimalZone(zone string) string { + z := strings.TrimSuffix(zone, ".") + return "$ORIGIN " + z + ".\n" + + "$TTL 3600\n" + + "@ 3600 IN SOA ns." + z + ". admin." + z + ". 2026052101 300 120 604800 60\n" + + "@ 3600 IN NS ns." + z + ".\n" +} + func TestParse(t *testing.T) { + dir := withTempZonesDir(t, "auth.example.com", "acme.example.org") + tests := []struct { name string input string shouldErr bool - errMatch string // substring to look for in the error (when shouldErr) + errMatch string check func(t *testing.T, p *RFC2136) }{ { - name: "single zone, no block", - input: `rfc2136 auth.example.com.`, - check: func(t *testing.T, p *RFC2136) { - wantZone(t, p, "auth.example.com.") - if p.TTL != DefaultTTL { - t.Errorf("TTL = %d, want default %d", p.TTL, DefaultTTL) - } - if len(p.TSIGKeys) != 0 { - t.Errorf("expected 0 TSIG keys, got %d", len(p.TSIGKeys)) - } - }, - }, - { - name: "multiple zones in one directive", - input: `rfc2136 auth.example.com. acme.example.org.`, - check: func(t *testing.T, p *RFC2136) { - if len(p.Zones) != 2 { - t.Fatalf("Zones = %v, want 2", p.Zones) - } - wantZone(t, p, "auth.example.com.") - wantZone(t, p, "acme.example.org.") - }, - }, - { - name: "full block with all directives", + name: "minimal: zone + zones-dir", input: `rfc2136 auth.example.com. { - tsig-key acme-key. hmac-sha256 ` + testSecret + ` - ttl 120 - persist /var/lib/coredns/rfc2136/auth.db + zones-dir ` + dir + ` }`, check: func(t *testing.T, p *RFC2136) { wantZone(t, p, "auth.example.com.") + if p.ZonesDir != dir { + t.Errorf("ZonesDir = %q, want %q", p.ZonesDir, dir) + } + if p.TTL != DefaultTTL { + t.Errorf("TTL = %d, want default %d", p.TTL, DefaultTTL) + } + if !p.AutoCommit { + t.Errorf("AutoCommit should default to true") + } + if len(p.zones) != 1 { + t.Errorf("expected 1 zoneFile, got %d", len(p.zones)) + } + }, + }, + { + name: "multiple zones", + input: `rfc2136 auth.example.com. acme.example.org. { + zones-dir ` + dir + ` + }`, + check: func(t *testing.T, p *RFC2136) { + if len(p.Zones) != 2 || len(p.zones) != 2 { + t.Errorf("Zones=%v zoneFiles=%d", p.Zones, len(p.zones)) + } + }, + }, + { + name: "full block: tsig-key, ttl, auto-commit, git-author", + input: `rfc2136 auth.example.com. { + zones-dir ` + dir + ` + tsig-key acme-key. hmac-sha256 ` + testSecret + ` + ttl 120 + auto-commit false + git-author "RFC 2136" "rfc2136@example.com" + }`, + check: func(t *testing.T, p *RFC2136) { if p.TTL != 120 { t.Errorf("TTL = %d, want 120", p.TTL) } - if p.PersistPath != "/var/lib/coredns/rfc2136/auth.db" { - t.Errorf("PersistPath = %q", p.PersistPath) + if p.AutoCommit { + t.Errorf("AutoCommit should be false") } - k, ok := p.TSIGKeys["acme-key."] - if !ok { - t.Fatalf("acme-key. not in TSIGKeys; have keys=%v", keysOf(p.TSIGKeys)) + if k, ok := p.TSIGKeys["acme-key."]; !ok { + t.Errorf("acme-key. not in TSIGKeys") + } else if k.Algorithm != dns.HmacSHA256 || len(k.Secret) != 32 { + t.Errorf("TSIG key wrong: algo=%q len=%d", k.Algorithm, len(k.Secret)) } - if k.Algorithm != dns.HmacSHA256 { - t.Errorf("Algorithm = %q, want %q", k.Algorithm, dns.HmacSHA256) + // git-author should propagate to each zoneFile + zf := p.zones["auth.example.com."] + if zf.GitAuthorName != "RFC 2136" || zf.GitAuthorEmail != "rfc2136@example.com" { + t.Errorf("git author not propagated: %q %q", zf.GitAuthorName, zf.GitAuthorEmail) } - if len(k.Secret) != 32 { - t.Errorf("Secret length = %d, want 32 (decoded SHA-256-sized)", len(k.Secret)) + if zf.AutoCommit { + t.Errorf("zoneFile.AutoCommit should match p.AutoCommit (false)") } }, }, { name: "tsig-key name normalised to trailing-dot lowercase", input: `rfc2136 auth.example.com. { + zones-dir ` + dir + ` tsig-key Acme-Key hmac-sha256 ` + testSecret + ` }`, check: func(t *testing.T, p *RFC2136) { if _, ok := p.TSIGKeys["acme-key."]; !ok { - t.Errorf("expected canonicalised key 'acme-key.', got keys=%v", keysOf(p.TSIGKeys)) + t.Errorf("expected canonicalised 'acme-key.', keys=%v", keysOf(p.TSIGKeys)) } }, }, { - name: "multiple tsig-keys allowed", + name: "multiple tsig-keys for rotation", input: `rfc2136 auth.example.com. { + zones-dir ` + dir + ` tsig-key key-a. hmac-sha256 ` + testSecret + ` tsig-key key-b. hmac-sha512 ` + testSecret + ` }`, check: func(t *testing.T, p *RFC2136) { if len(p.TSIGKeys) != 2 { - t.Errorf("expected 2 keys, got %d (%v)", len(p.TSIGKeys), keysOf(p.TSIGKeys)) - } - if p.TSIGKeys["key-a."].Algorithm != dns.HmacSHA256 { - t.Errorf("key-a algorithm wrong") - } - if p.TSIGKeys["key-b."].Algorithm != dns.HmacSHA512 { - t.Errorf("key-b algorithm wrong") + t.Errorf("want 2 keys, got %d", len(p.TSIGKeys)) } }, }, @@ -112,14 +147,40 @@ func TestParse(t *testing.T) { errMatch: "Wrong argument count", }, { - name: "unknown directive", - input: `rfc2136 auth.example.com. { bogus value }`, + name: "missing zones-dir", + input: `rfc2136 auth.example.com.`, + shouldErr: true, + errMatch: "zones-dir is required", + }, + { + name: "zones-dir points at non-existent dir", + input: `rfc2136 auth.example.com. { + zones-dir /definitely/not/a/real/path + }`, + shouldErr: true, + errMatch: "file not accessible", + }, + { + name: "zone declared but no matching file", + input: `rfc2136 missing.example.com. { + zones-dir ` + dir + ` + }`, + shouldErr: true, + errMatch: "file not accessible", + }, + { + name: "unknown directive", + input: `rfc2136 auth.example.com. { + zones-dir ` + dir + ` + bogus value + }`, shouldErr: true, errMatch: "unknown directive", }, { name: "tsig-key with too few args", input: `rfc2136 auth.example.com. { + zones-dir ` + dir + ` tsig-key only-name }`, shouldErr: true, @@ -128,6 +189,7 @@ func TestParse(t *testing.T) { { name: "unsupported TSIG algorithm", input: `rfc2136 auth.example.com. { + zones-dir ` + dir + ` tsig-key key. hmac-md5 ` + testSecret + ` }`, shouldErr: true, @@ -136,73 +198,72 @@ func TestParse(t *testing.T) { { name: "malformed base64 secret", input: `rfc2136 auth.example.com. { + zones-dir ` + dir + ` tsig-key key. hmac-sha256 not_base64_at_all!!! }`, shouldErr: true, errMatch: "invalid base64", }, { - name: "secret too short after decode", + name: "secret too short", input: `rfc2136 auth.example.com. { + zones-dir ` + dir + ` tsig-key key. hmac-sha256 c2hvcnQ= - }`, // "short" → 5 bytes < 8 min + }`, shouldErr: true, errMatch: "too short", }, { - name: "duplicate tsig-key name", + name: "duplicate tsig-key", input: `rfc2136 auth.example.com. { + zones-dir ` + dir + ` tsig-key dup. hmac-sha256 ` + testSecret + ` tsig-key dup. hmac-sha512 ` + testSecret + ` }`, shouldErr: true, errMatch: "duplicate tsig-key", }, - { - name: "nameserver directive overrides default", - input: `rfc2136 auth.example.com. { - nameserver dns.example.com. - }`, - check: func(t *testing.T, p *RFC2136) { - if p.Nameserver != "dns.example.com." { - t.Errorf("Nameserver = %q, want dns.example.com.", p.Nameserver) - } - }, - }, - { - name: "default nameserver is first zone apex", - input: `rfc2136 auth.example.com.`, - check: func(t *testing.T, p *RFC2136) { - if p.Nameserver != "auth.example.com." { - t.Errorf("default Nameserver = %q, want auth.example.com.", p.Nameserver) - } - }, - }, - { - name: "store is initialised even with no records", - input: `rfc2136 auth.example.com.`, - check: func(t *testing.T, p *RFC2136) { - if p.store == nil { - t.Errorf("store should be initialised by parse()") - } - }, - }, { name: "ttl non-numeric", input: `rfc2136 auth.example.com. { + zones-dir ` + dir + ` ttl not-a-number }`, shouldErr: true, errMatch: "ttl must be a non-negative integer", }, + { + name: "auto-commit bogus value", + input: `rfc2136 auth.example.com. { + zones-dir ` + dir + ` + auto-commit maybe + }`, + shouldErr: true, + errMatch: "auto-commit must be true|false", + }, + { + name: "git-author wrong arg count", + input: `rfc2136 auth.example.com. { + zones-dir ` + dir + ` + git-author OnlyName + }`, + shouldErr: true, + errMatch: "git-author requires 2 args", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := caddy.NewTestController("dns", tt.input) p, err := parse(c) + if err == nil { + // Always run validateZoneFiles when parse succeeds — + // some error cases (missing zones-dir, missing file) + // trigger here, not in parse() itself. + err = p.validateZoneFiles() + } if (err != nil) != tt.shouldErr { - t.Fatalf("parse() err = %v, shouldErr = %v", err, tt.shouldErr) + t.Fatalf("err = %v, shouldErr = %v", err, tt.shouldErr) } if err != nil { if tt.errMatch != "" && !strings.Contains(err.Error(), tt.errMatch) { @@ -217,7 +278,6 @@ func TestParse(t *testing.T) { } } -// wantZone asserts that p.Zones contains the expected canonical zone. func wantZone(t *testing.T, p *RFC2136, want string) { t.Helper() for _, z := range p.Zones { @@ -228,7 +288,6 @@ func wantZone(t *testing.T, p *RFC2136, want string) { t.Errorf("zone %q not in p.Zones=%v", want, p.Zones) } -// keysOf is a debug helper for failure messages. func keysOf(m map[string]tsigKey) []string { out := make([]string, 0, len(m)) for k := range m { diff --git a/store.go b/store.go deleted file mode 100644 index 0cac4cb..0000000 --- a/store.go +++ /dev/null @@ -1,175 +0,0 @@ -package rfc2136 - -import ( - "strings" - "sync" - "sync/atomic" - - "github.com/miekg/dns" -) - -// recordStore is the in-memory store of dynamic records. It is keyed -// by canonical (lowercased, trailing-dot) owner name, then by RR type, -// to a slice of RRs forming the RRset. -// -// Concurrency: a single RWMutex covers all mutations. Read paths -// (queries) are far more frequent than writes (UPDATEs), so the -// RW split is the right choice. The lock is held only across the -// O(1) map operations; nothing slow happens under the lock. -// -// Serial: every successful mutation bumps the atomic generation -// counter. The synthetic SOA uses this counter so resolvers see a -// fresh serial after every UPDATE — even though we don't run AXFR -// against secondaries, future use-cases (DNS-NOTIFY, debugging, -// cache invalidation) benefit from a monotonic serial. -type recordStore struct { - mu sync.RWMutex - // rrs[name][rrtype] = RRset (slice). The map-of-maps shape means - // per-type lookups are O(1), and "name exists at all" is O(1) on - // the outer map. - rrs map[string]map[uint16][]dns.RR - - gen atomic.Uint64 -} - -func newStore() *recordStore { - return &recordStore{rrs: make(map[string]map[uint16][]dns.RR)} -} - -// generation returns the current monotonic counter. Used for SOA serial. -func (s *recordStore) generation() uint64 { - return s.gen.Load() -} - -// canon normalises a name to the store's internal form: lowercase + -// trailing dot. miekg/dns sometimes returns names without the dot; -// always passing through this keeps everything consistent. -func canon(name string) string { - return strings.ToLower(dns.Fqdn(name)) -} - -// Add inserts a single RR into the store. Duplicate RRs (same owner, -// type, AND rdata) are silently de-duplicated — matches RFC 2136 §3.4.2.2 -// behavior for `add` semantics. -func (s *recordStore) Add(rr dns.RR) { - s.mu.Lock() - defer s.mu.Unlock() - - name := canon(rr.Header().Name) - rtype := rr.Header().Rrtype - - byType, ok := s.rrs[name] - if !ok { - byType = make(map[uint16][]dns.RR) - s.rrs[name] = byType - } - - // De-duplicate by string comparison of the rdata-bearing form. - rrStr := rr.String() - for _, existing := range byType[rtype] { - if existing.String() == rrStr { - return - } - } - byType[rtype] = append(byType[rtype], rr) - s.gen.Add(1) -} - -// RemoveRRset deletes ALL records of the given (name, type) — i.e. -// "delete the RRset" semantics from RFC 2136 §3.4.2.3. If the name -// has no records left after this, the name entry is reaped so -// NameExists returns false. -func (s *recordStore) RemoveRRset(name string, rtype uint16) { - s.mu.Lock() - defer s.mu.Unlock() - - name = canon(name) - byType, ok := s.rrs[name] - if !ok { - return - } - if _, hadType := byType[rtype]; !hadType { - return - } - delete(byType, rtype) - if len(byType) == 0 { - delete(s.rrs, name) - } - s.gen.Add(1) -} - -// RemoveRR deletes one specific RR (matching owner, type, and rdata). -// This implements RFC 2136 §3.4.2.4 "delete an RR from an RRset". -func (s *recordStore) RemoveRR(rr dns.RR) { - s.mu.Lock() - defer s.mu.Unlock() - - name := canon(rr.Header().Name) - rtype := rr.Header().Rrtype - byType, ok := s.rrs[name] - if !ok { - return - } - rrs := byType[rtype] - target := rr.String() - for i, existing := range rrs { - if existing.String() == target { - byType[rtype] = append(rrs[:i], rrs[i+1:]...) - if len(byType[rtype]) == 0 { - delete(byType, rtype) - } - if len(byType) == 0 { - delete(s.rrs, name) - } - s.gen.Add(1) - return - } - } -} - -// RemoveName deletes all records for an owner name (§3.4.2.3 "delete -// all RRsets from a name"). -func (s *recordStore) RemoveName(name string) { - s.mu.Lock() - defer s.mu.Unlock() - - name = canon(name) - if _, ok := s.rrs[name]; !ok { - return - } - delete(s.rrs, name) - s.gen.Add(1) -} - -// Lookup returns the RRset for (name, rtype). Returns nil for both -// "name doesn't exist" and "name exists with other types but not this -// one" — use NameExists to distinguish NODATA from NXDOMAIN. -// -// The returned slice is a copy so callers can freely mutate it without -// affecting store state. -func (s *recordStore) Lookup(name string, rtype uint16) []dns.RR { - s.mu.RLock() - defer s.mu.RUnlock() - - byType, ok := s.rrs[canon(name)] - if !ok { - return nil - } - rrs := byType[rtype] - if len(rrs) == 0 { - return nil - } - out := make([]dns.RR, len(rrs)) - copy(out, rrs) - return out -} - -// NameExists reports whether ANY records exist for the given name. -// Used to distinguish NODATA (name exists, no records of asked type) -// from NXDOMAIN (name doesn't exist at all). -func (s *recordStore) NameExists(name string) bool { - s.mu.RLock() - defer s.mu.RUnlock() - _, ok := s.rrs[canon(name)] - return ok -} diff --git a/store_test.go b/store_test.go deleted file mode 100644 index dbb759c..0000000 --- a/store_test.go +++ /dev/null @@ -1,180 +0,0 @@ -package rfc2136 - -import ( - "testing" - - "github.com/miekg/dns" -) - -func mustRR(t *testing.T, s string) dns.RR { - t.Helper() - rr, err := dns.NewRR(s) - if err != nil { - t.Fatalf("failed to parse RR %q: %v", s, err) - } - return rr -} - -func TestStore_AddLookup(t *testing.T) { - s := newStore() - - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "token-1"`)) - - got := s.Lookup("foo.example.com.", dns.TypeTXT) - if len(got) != 1 { - t.Fatalf("Lookup TXT: got %d records, want 1", len(got)) - } - if got[0].String() != `foo.example.com. 60 IN TXT "token-1"` { - t.Errorf("unexpected RR: %s", got[0].String()) - } -} - -func TestStore_AddMultipleRRsetEntries(t *testing.T) { - s := newStore() - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "token-1"`)) - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "token-2"`)) - - got := s.Lookup("foo.example.com.", dns.TypeTXT) - if len(got) != 2 { - t.Errorf("RRset size = %d, want 2 (both TXT values)", len(got)) - } -} - -func TestStore_AddDedupes(t *testing.T) { - s := newStore() - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "same"`)) - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "same"`)) - - got := s.Lookup("foo.example.com.", dns.TypeTXT) - if len(got) != 1 { - t.Errorf("RRset size = %d, want 1 (duplicate ignored)", len(got)) - } -} - -func TestStore_LookupCaseInsensitive(t *testing.T) { - s := newStore() - s.Add(mustRR(t, `FOO.example.com. 60 IN TXT "token"`)) - - if got := s.Lookup("foo.EXAMPLE.com.", dns.TypeTXT); len(got) != 1 { - t.Errorf("case-insensitive lookup failed: got %d", len(got)) - } -} - -func TestStore_LookupMissingNameReturnsNil(t *testing.T) { - s := newStore() - if got := s.Lookup("nope.example.com.", dns.TypeTXT); got != nil { - t.Errorf("expected nil for missing name, got %v", got) - } -} - -func TestStore_LookupNameExistsWrongTypeReturnsNil(t *testing.T) { - s := newStore() - s.Add(mustRR(t, `foo.example.com. 60 IN A 192.0.2.1`)) - - if got := s.Lookup("foo.example.com.", dns.TypeTXT); got != nil { - t.Errorf("expected nil for wrong type (A exists but not TXT), got %v", got) - } - if !s.NameExists("foo.example.com.") { - t.Errorf("NameExists should return true (A record exists)") - } -} - -func TestStore_RemoveRRset(t *testing.T) { - s := newStore() - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "a"`)) - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "b"`)) - s.Add(mustRR(t, `foo.example.com. 60 IN A 192.0.2.1`)) - - s.RemoveRRset("foo.example.com.", dns.TypeTXT) - - if got := s.Lookup("foo.example.com.", dns.TypeTXT); got != nil { - t.Errorf("TXT RRset should be gone, got %v", got) - } - if got := s.Lookup("foo.example.com.", dns.TypeA); len(got) != 1 { - t.Errorf("A record should survive RRset deletion, got %v", got) - } - if !s.NameExists("foo.example.com.") { - t.Errorf("name should still exist (A remains)") - } -} - -func TestStore_RemoveRRsetReapsEmptyName(t *testing.T) { - s := newStore() - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "a"`)) - s.RemoveRRset("foo.example.com.", dns.TypeTXT) - - if s.NameExists("foo.example.com.") { - t.Errorf("name should have been reaped after last RRset removed") - } -} - -func TestStore_RemoveRR(t *testing.T) { - s := newStore() - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "keep"`)) - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "drop"`)) - - s.RemoveRR(mustRR(t, `foo.example.com. 60 IN TXT "drop"`)) - - got := s.Lookup("foo.example.com.", dns.TypeTXT) - if len(got) != 1 { - t.Fatalf("RRset size after RemoveRR = %d, want 1", len(got)) - } - if got[0].(*dns.TXT).Txt[0] != "keep" { - t.Errorf("wrong RR remained: %v", got[0]) - } -} - -func TestStore_RemoveName(t *testing.T) { - s := newStore() - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "a"`)) - s.Add(mustRR(t, `foo.example.com. 60 IN A 192.0.2.1`)) - - s.RemoveName("foo.example.com.") - - if s.NameExists("foo.example.com.") { - t.Errorf("name should be gone after RemoveName") - } -} - -func TestStore_GenerationBumpsOnMutation(t *testing.T) { - s := newStore() - start := s.generation() - - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "a"`)) - if s.generation() != start+1 { - t.Errorf("generation after Add: %d, want %d", s.generation(), start+1) - } - - // Re-adding the same RR is a no-op → generation must NOT bump. - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "a"`)) - if s.generation() != start+1 { - t.Errorf("generation after duplicate Add: %d, want %d (no bump)", s.generation(), start+1) - } - - s.RemoveRRset("foo.example.com.", dns.TypeTXT) - if s.generation() != start+2 { - t.Errorf("generation after RemoveRRset: %d, want %d", s.generation(), start+2) - } - - // Removing again is a no-op → no bump. - s.RemoveRRset("foo.example.com.", dns.TypeTXT) - if s.generation() != start+2 { - t.Errorf("generation after no-op RemoveRRset: %d, want %d", s.generation(), start+2) - } -} - -func TestStore_LookupReturnsCopy(t *testing.T) { - // The returned slice must be a copy so external mutations don't - // affect store state. - s := newStore() - s.Add(mustRR(t, `foo.example.com. 60 IN TXT "original"`)) - - got := s.Lookup("foo.example.com.", dns.TypeTXT) - got[0] = mustRR(t, `foo.example.com. 60 IN TXT "tampered"`) - - // Re-lookup should still see the original. - again := s.Lookup("foo.example.com.", dns.TypeTXT) - if again[0].(*dns.TXT).Txt[0] != "original" { - t.Errorf("Lookup returned shared slice — store corrupted by external mutation: %v", again[0]) - } -} diff --git a/update.go b/update.go index 5b41334..7c6917e 100644 --- a/update.go +++ b/update.go @@ -1,34 +1,36 @@ package rfc2136 import ( + "fmt" "strings" + "time" "github.com/miekg/dns" ) -// handleUpdate implements the RFC 2136 UPDATE opcode. +// handleUpdate implements the RFC 2136 UPDATE opcode against the +// on-disk zone file. // -// Message layout in an UPDATE (RFC 2136 §2.2): +// Sequence per UPDATE message: +// 1. Validate the Zone section (RFC 2136 §2.3): must be exactly one +// SOA-typed record whose name is a zone we manage. +// 2. Acquire the zone file's mutex. +// 3. Load the file's RRs into memory. +// 4. Check each prerequisite (§3.2) against the loaded RRs. First +// failure short-circuits with the spec's rcode. +// 5. Apply each update RR (§3.4.2) to the in-memory slice. +// 6. Bump the SOA serial (CalVer YYYYMMDDNN). +// 7. Atomic write to disk (temp file + rename). +// 8. Optionally `git add && git commit` for audit trail. // -// Question → "Zone" section (exactly one record, type SOA) -// Answer → "Prerequisite" section (zero or more, see §2.4) -// Authority → "Update" section (zero or more, see §2.5) -// Additional → TSIG, OPT, etc. -// -// Processing order: -// 1. Zone-section validation: zone must be one we're authoritative for. -// 2. Prerequisite checks (§3.2). First failure short-circuits with the -// RFC-specified rcode (NXDOMAIN/YXDOMAIN/NXRRSET/YXRRSET/NOTAUTH). -// 3. Apply updates (§3.4.2). All updates either all succeed or all fail -// by acquiring the store lock once for the batch. -// -// TSIG verification happens before this function is called — see -// ServeDNS for the auth gate. +// Steps 3-7 happen under the zone-file mutex. If 8 fails we log but +// don't roll back (the on-disk state is authoritative; lost commits +// can be re-staged via `git add` later). func (p *RFC2136) handleUpdate(w dns.ResponseWriter, r *dns.Msg) (int, error) { resp := new(dns.Msg) resp.SetReply(r) - // 1. Validate zone section. + // 1. Validate the Zone section. if len(r.Question) != 1 { log.Debugf("UPDATE rejected: expected 1 Zone record, got %d", len(r.Question)) return p.updateResp(w, resp, dns.RcodeFormatError) @@ -43,28 +45,76 @@ func (p *RFC2136) handleUpdate(w dns.ResponseWriter, r *dns.Msg) (int, error) { log.Debugf("UPDATE rejected: zone %q not authoritative", zoneQ.Name) return p.updateResp(w, resp, dns.RcodeNotAuth) } + zf, ok := p.zones[zone] + if !ok { + log.Errorf("UPDATE rejected: no zone file handle for %q (setup bug?)", zone) + return p.updateResp(w, resp, dns.RcodeServerFailure) + } - // 2. Verify each prerequisite. Read-locked through the store API. + zf.mu.Lock() + defer zf.mu.Unlock() + + // 3. Load the current zone contents. + rrs, err := zf.loadRRs() + if err != nil { + log.Errorf("UPDATE failed: %v", err) + return p.updateResp(w, resp, dns.RcodeServerFailure) + } + + // 4. Check prerequisites. for _, rr := range r.Answer { - rcode := p.checkPrereq(zone, rr) + rcode := checkPrereq(zone, rrs, rr) if rcode != dns.RcodeSuccess { log.Debugf("UPDATE prereq failed: %s → rcode=%d", rr.String(), rcode) return p.updateResp(w, resp, rcode) } } - // 3. Apply updates. We don't take a single batch lock here — each - // store operation locks internally. RFC 2136 §3.7 allows the - // "atomic" requirement to be relaxed for implementations; with - // short-lived ACME records this is fine in practice. + // 5. Apply updates. Build a fresh RR slice rather than mutating in + // place — that way a partial application can't leave the slice in + // a half-modified state if an early update fails. + updated := rrs + changed := false for _, rr := range r.Ns { - if rcode := p.applyUpdate(zone, rr); rcode != dns.RcodeSuccess { + next, rcode, modified := applyUpdate(zone, p.TTL, updated, rr) + if rcode != dns.RcodeSuccess { return p.updateResp(w, resp, rcode) } + updated = next + if modified { + changed = true + } } - log.Infof("UPDATE applied: zone=%s prereqs=%d updates=%d gen=%d", - zone, len(r.Answer), len(r.Ns), p.store.generation()) + if !changed { + // UPDATE was a valid no-op (e.g. only contained adds for RRs + // that were already present, deduped away). Return NOERROR + // without rewriting the file. + return p.updateResp(w, resp, dns.RcodeSuccess) + } + + // 6. Bump SOA serial. + now := time.Now() + if err := bumpSerial(updated, now); err != nil { + log.Errorf("UPDATE failed: %v", err) + return p.updateResp(w, resp, dns.RcodeServerFailure) + } + + // 7. Atomic write. + if err := zf.writeAtomic(updated, now); err != nil { + log.Errorf("UPDATE write failed: %v", err) + return p.updateResp(w, resp, dns.RcodeServerFailure) + } + + // 8. Auto-commit. Failure to commit isn't fatal to the UPDATE — + // the on-disk state is authoritative — but we log loudly. + msg := summarizeUpdate(zone, r.Ns) + if err := zf.commit(msg); err != nil { + log.Warningf("git auto-commit failed: %v", err) + } + + log.Infof("UPDATE applied: zone=%s prereqs=%d updates=%d msg=%q", + zone, len(r.Answer), len(r.Ns), msg) return p.updateResp(w, resp, dns.RcodeSuccess) } @@ -75,122 +125,152 @@ func (p *RFC2136) updateResp(w dns.ResponseWriter, resp *dns.Msg, rcode int) (in return rcode, nil } -// checkPrereq evaluates one record from the Prerequisite section. -// Returns dns.RcodeSuccess if satisfied, or the appropriate error rcode. -// -// Encoding rules (§3.2.4): -// -// CLASS=ANY TYPE=ANY → name must exist (else NXDOMAIN) -// CLASS=ANY TYPE!=ANY → RRset must exist (else NXRRSET) -// CLASS=NONE TYPE=ANY → name must NOT exist (else YXDOMAIN) -// CLASS=NONE TYPE!=ANY → RRset must NOT exist (else YXRRSET) -// CLASS= ... rdata → RRset must exist with this exact rdata -func (p *RFC2136) checkPrereq(zone string, rr dns.RR) int { +// findZone returns the longest matching configured zone for qname, or +// "" if qname is outside all configured zones. +func (p *RFC2136) findZone(qname string) string { + qname = canon(qname) + var best string + for _, z := range p.Zones { + if qname == z || strings.HasSuffix(qname, "."+z) { + if len(z) > len(best) { + best = z + } + } + } + return best +} + +// checkPrereq evaluates one record from the Prerequisite section +// against the loaded RR slice. Returns dns.RcodeSuccess if satisfied, +// or the spec rcode otherwise (§3.2). +func checkPrereq(zone string, rrs []dns.RR, rr dns.RR) int { hdr := rr.Header() name := canon(hdr.Name) - - // All prereq names must be within the zone. if !inZone(name, zone) { return dns.RcodeNotZone } - switch hdr.Class { case dns.ClassANY: - // "Name/RRset is in use" if hdr.Rrtype == dns.TypeANY { - if !p.store.NameExists(name) && !isApex(name, zone) { + if !nameExistsIn(rrs, name) { return dns.RcodeNameError } return dns.RcodeSuccess } - if rrs := p.store.Lookup(name, hdr.Rrtype); rrs == nil { + if len(lookupIn(rrs, name, hdr.Rrtype)) == 0 { return dns.RcodeNXRrset } return dns.RcodeSuccess case dns.ClassNONE: - // "Name/RRset is NOT in use" if hdr.Rrtype == dns.TypeANY { - if p.store.NameExists(name) { + if nameExistsIn(rrs, name) { return dns.RcodeYXDomain } return dns.RcodeSuccess } - if rrs := p.store.Lookup(name, hdr.Rrtype); rrs != nil { + if len(lookupIn(rrs, name, hdr.Rrtype)) > 0 { return dns.RcodeYXRrset } return dns.RcodeSuccess default: - // CLASS = zone class. Exact rdata match required (§3.2.5). - // Skipped for v1 — Caddy/caddy-dns/rfc2136 doesn't emit these. - // Document the gap; v2 can implement value-prereq if a caller - // actually needs it. - log.Debugf("prereq with rdata-match semantics not yet implemented; treating as satisfied") + // CLASS = zone class with rdata. Exact value-match prereqs + // (§3.2.5). Not used by Caddy/caddy-dns/rfc2136; treating as + // satisfied for now. v2 can implement value-prereq if a real + // caller needs it. + log.Debugf("prereq with rdata-match semantics not implemented; treating as satisfied") return dns.RcodeSuccess } } // applyUpdate handles one record in the Update section per §3.4.2. -// -// Encoding rules: -// -// CLASS= RDLEN>0 → add RR (§3.4.2.2) -// CLASS=ANY TYPE=ANY → delete all RRsets from name (§3.4.2.3) -// CLASS=ANY TYPE!=ANY RDLEN=0 → delete this RRset (§3.4.2.3) -// CLASS=NONE RDLEN>0 → delete the specific RR (§3.4.2.4) -func (p *RFC2136) applyUpdate(zone string, rr dns.RR) int { +// Returns the (possibly mutated) RR slice, an rcode (Success unless +// the update was rejected), and a flag indicating whether the slice +// was actually modified (to avoid no-op file rewrites). +func applyUpdate(zone string, defaultTTL uint32, rrs []dns.RR, rr dns.RR) ([]dns.RR, int, bool) { hdr := rr.Header() name := canon(hdr.Name) - if !inZone(name, zone) { - return dns.RcodeNotZone + return rrs, dns.RcodeNotZone, false } switch hdr.Class { case dns.ClassANY: if hdr.Rrtype == dns.TypeANY { - // Reject deleting the apex (SOA/NS bedrock); the rest of - // the zone is free game. + // Wipe the whole name. Refuse apex wipes — that would + // destroy SOA + NS bedrock. if isApex(name, zone) { - log.Debugf("apex deletion refused: %s", name) - return dns.RcodeRefused + log.Debugf("apex wipe refused: %s", name) + return rrs, dns.RcodeRefused, false } - p.store.RemoveName(name) - return dns.RcodeSuccess + before := len(rrs) + rrs = removeNameFrom(rrs, name) + return rrs, dns.RcodeSuccess, len(rrs) != before } - // Apex SOA/NS protected against type-targeted deletion too. + // Apex SOA/NS removal refused for the same reason. if isApex(name, zone) && (hdr.Rrtype == dns.TypeSOA || hdr.Rrtype == dns.TypeNS) { - log.Debugf("apex %s deletion refused: %s", dns.TypeToString[hdr.Rrtype], name) - return dns.RcodeRefused + log.Debugf("apex %s removal refused", dns.TypeToString[hdr.Rrtype]) + return rrs, dns.RcodeRefused, false } - p.store.RemoveRRset(name, hdr.Rrtype) - return dns.RcodeSuccess + before := len(rrs) + rrs = removeRRsetFrom(rrs, name, hdr.Rrtype) + return rrs, dns.RcodeSuccess, len(rrs) != before case dns.ClassNONE: + // Refuse to delete apex SOA/NS by exact-RR match. if isApex(name, zone) && (hdr.Rrtype == dns.TypeSOA || hdr.Rrtype == dns.TypeNS) { - return dns.RcodeRefused + return rrs, dns.RcodeRefused, false } - p.store.RemoveRR(rr) - return dns.RcodeSuccess + before := len(rrs) + rrs = removeRRFrom(rrs, rr) + return rrs, dns.RcodeSuccess, len(rrs) != before default: - // CLASS = zone class → add. Apply default TTL if missing. - if hdr.Ttl == 0 { - hdr.Ttl = p.TTL - } - // SOA/NS at the apex are synthetic — don't let UPDATE override. + // Apex SOA/NS adds refused — those are managed by the zone-file + // owner, not by dynamic updates. if isApex(name, zone) && (hdr.Rrtype == dns.TypeSOA || hdr.Rrtype == dns.TypeNS) { - log.Debugf("apex %s add refused: synthetic at this plugin", dns.TypeToString[hdr.Rrtype]) - return dns.RcodeRefused + log.Debugf("apex %s add refused", dns.TypeToString[hdr.Rrtype]) + return rrs, dns.RcodeRefused, false } - p.store.Add(rr) - return dns.RcodeSuccess + if hdr.Ttl == 0 { + hdr.Ttl = defaultTTL + } + before := len(rrs) + rrs = addRRTo(rrs, rr) + return rrs, dns.RcodeSuccess, len(rrs) != before } } -// inZone reports whether name is within zone (either the apex itself -// or a sub-name of it). Both arguments must already be canonical. +// summarizeUpdate produces a one-line commit message describing the +// UPDATE for git history. +func summarizeUpdate(zone string, updates []dns.RR) string { + if len(updates) == 1 { + return fmt.Sprintf("rfc2136 %s: %s", zone, oneLineOp(updates[0])) + } + return fmt.Sprintf("rfc2136 %s: %d operations", zone, len(updates)) +} + +// oneLineOp returns a short human-readable description of a single +// update RR for inclusion in commit messages. +func oneLineOp(rr dns.RR) string { + hdr := rr.Header() + name := strings.TrimSuffix(canon(hdr.Name), ".") + ttype := dns.TypeToString[hdr.Rrtype] + switch hdr.Class { + case dns.ClassANY: + if hdr.Rrtype == dns.TypeANY { + return fmt.Sprintf("delete all %s", name) + } + return fmt.Sprintf("delete %s %s", ttype, name) + case dns.ClassNONE: + return fmt.Sprintf("delete-rr %s %s", ttype, name) + default: + return fmt.Sprintf("add %s %s", ttype, name) + } +} + +// inZone reports whether name is within zone. func inZone(name, zone string) bool { return name == zone || strings.HasSuffix(name, "."+zone) } diff --git a/update_test.go b/update_test.go index f20f29f..d340a61 100644 --- a/update_test.go +++ b/update_test.go @@ -1,15 +1,52 @@ package rfc2136 import ( - "context" + "net" + "os" + "path/filepath" + "strings" "testing" "github.com/miekg/dns" ) -// newUpdate builds a minimal UPDATE message for one zone with zero -// prereqs and the given update RRs. Caddy's caddy-dns/rfc2136 module -// produces messages in this same shape. +// captureWriter implements dns.ResponseWriter and stashes the message +// passed to WriteMsg so tests can inspect it after handleUpdate returns. +type captureWriter struct { + msg *dns.Msg +} + +func (cw *captureWriter) WriteMsg(m *dns.Msg) error { cw.msg = m; return nil } +func (cw *captureWriter) Write([]byte) (int, error) { return 0, nil } +func (cw *captureWriter) Close() error { return nil } +func (cw *captureWriter) TsigStatus() error { return nil } +func (cw *captureWriter) TsigTimersOnly(bool) {} +func (cw *captureWriter) Hijack() {} +func (cw *captureWriter) LocalAddr() net.Addr { return &net.IPAddr{IP: net.IPv4(127, 0, 0, 1)} } +func (cw *captureWriter) RemoteAddr() net.Addr { return &net.IPAddr{IP: net.IPv4(127, 0, 0, 1)} } +func (cw *captureWriter) Network() string { return "udp" } + +// newTestPluginWithZone builds an RFC2136 backed by a temp zones-dir. +// AutoCommit is disabled by default (tests don't need git side-effects). +func newTestPluginWithZone(t *testing.T, zone string) *RFC2136 { + t.Helper() + dir := withTempZonesDir(t, zone) + zone = dns.Fqdn(zone) + + p := &RFC2136{ + Zones: []string{zone}, + TTL: 60, + ZonesDir: dir, + zones: map[string]*zoneFile{ + zone: openZoneFile(filepath.Join(dir, strings.TrimSuffix(zone, ".")+".zone"), zone), + }, + } + p.zones[zone].AutoCommit = false + return p +} + +// newUpdate builds an UPDATE message for `zone` with zero prereqs and +// the given update RRs. Matches what caddy-dns/rfc2136 sends. func newUpdate(zone string, updates ...dns.RR) *dns.Msg { m := new(dns.Msg) m.SetUpdate(dns.Fqdn(zone)) @@ -17,220 +54,229 @@ func newUpdate(zone string, updates ...dns.RR) *dns.Msg { return m } -// applyUpdateNoAuth bypasses the TSIG gate so we can test handler -// logic directly without setting up dns.Server-level integration in -// unit tests. End-to-end TSIG verification happens in Phase 2 with -// nsupdate against the live custom CoreDNS binary. -func applyUpdateNoAuth(t *testing.T, p *RFC2136, msg *dns.Msg) (rcode int, response *dns.Msg) { +// runUpdate sends msg through the handler with TSIG auth bypassed +// (calling handleUpdate directly instead of ServeDNS). +func runUpdate(t *testing.T, p *RFC2136, msg *dns.Msg) (rcode int) { t.Helper() w := &captureWriter{} rcode, _ = p.handleUpdate(w, msg) - return rcode, w.msg + return rcode } -func TestUpdate_AddSingleTXT(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) +// readZoneRecords loads the zone file and returns the RRs for inspection. +func readZoneRecords(t *testing.T, p *RFC2136, zone string) []dns.RR { + t.Helper() + zf := p.zones[dns.Fqdn(zone)] + rrs, err := zf.loadRRs() + if err != nil { + t.Fatalf("loadRRs: %v", err) + } + return rrs +} - upd := newUpdate("auth.example.com.", +func TestUpdate_AddSingleTXT_PersistsToFile(t *testing.T) { + p := newTestPluginWithZone(t, "auth.example.com") + + upd := newUpdate("auth.example.com", mustRR(t, `token-1.auth.example.com. 60 IN TXT "validation-1"`), ) - - rcode, _ := applyUpdateNoAuth(t, p, upd) - if rcode != dns.RcodeSuccess { + if rcode := runUpdate(t, p, upd); rcode != dns.RcodeSuccess { t.Fatalf("rcode = %d, want NOERROR", rcode) } - // Verify via a query through ServeDNS. - req := new(dns.Msg) - req.SetQuestion("token-1.auth.example.com.", dns.TypeTXT) - w := &captureWriter{} - p.ServeDNS(context.Background(), w, req) - - if len(w.msg.Answer) != 1 { - t.Fatalf("Answer len = %d, want 1", len(w.msg.Answer)) + // Verify by re-reading the file. + rrs := readZoneRecords(t, p, "auth.example.com") + for _, rr := range rrs { + if txt, ok := rr.(*dns.TXT); ok && txt.Hdr.Name == "token-1.auth.example.com." { + if txt.Txt[0] == "validation-1" { + return // success + } + } } - if w.msg.Answer[0].(*dns.TXT).Txt[0] != "validation-1" { - t.Errorf("TXT mismatch: %v", w.msg.Answer[0]) + t.Errorf("added TXT not found in re-read zone file") +} + +func TestUpdate_AddBumpsSerial(t *testing.T) { + p := newTestPluginWithZone(t, "auth.example.com") + + before := readZoneRecords(t, p, "auth.example.com") + var beforeSerial uint32 + for _, rr := range before { + if soa, ok := rr.(*dns.SOA); ok { + beforeSerial = soa.Serial + break + } + } + + upd := newUpdate("auth.example.com", + mustRR(t, `foo.auth.example.com. 60 IN TXT "x"`), + ) + runUpdate(t, p, upd) + + after := readZoneRecords(t, p, "auth.example.com") + var afterSerial uint32 + for _, rr := range after { + if soa, ok := rr.(*dns.SOA); ok { + afterSerial = soa.Serial + break + } + } + if afterSerial <= beforeSerial { + t.Errorf("Serial did not advance: before=%d after=%d", beforeSerial, afterSerial) } } -func TestUpdate_DeleteRRset(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - p.store.Add(mustRR(t, `token-1.auth.example.com. 60 IN TXT "old-token"`)) +func TestUpdate_DeleteRRset_RemovesAllOfType(t *testing.T) { + p := newTestPluginWithZone(t, "auth.example.com") - // CLASS=ANY, RDLEN=0 → delete this specific RRset. - del := &dns.ANY{Hdr: dns.RR_Header{ - Name: "token-1.auth.example.com.", - Rrtype: dns.TypeTXT, - Class: dns.ClassANY, - Ttl: 0, - }} - upd := newUpdate("auth.example.com.", del) + // First add two TXTs at the same name. + runUpdate(t, p, newUpdate("auth.example.com", + mustRR(t, `foo.auth.example.com. 60 IN TXT "a"`), + mustRR(t, `foo.auth.example.com. 60 IN TXT "b"`), + )) - rcode, _ := applyUpdateNoAuth(t, p, upd) - if rcode != dns.RcodeSuccess { - t.Fatalf("rcode = %d, want NOERROR", rcode) - } - - if got := p.store.Lookup("token-1.auth.example.com.", dns.TypeTXT); got != nil { - t.Errorf("RRset should be gone, still got %v", got) - } -} - -func TestUpdate_DeleteAllRRsetsAtName(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - p.store.Add(mustRR(t, `foo.auth.example.com. 60 IN TXT "t"`)) - p.store.Add(mustRR(t, `foo.auth.example.com. 60 IN A 192.0.2.1`)) - - // CLASS=ANY, TYPE=ANY, RDLEN=0 → wipe the name. + // Now delete the whole TXT RRset. del := &dns.ANY{Hdr: dns.RR_Header{ Name: "foo.auth.example.com.", - Rrtype: dns.TypeANY, + Rrtype: dns.TypeTXT, Class: dns.ClassANY, }} - rcode, _ := applyUpdateNoAuth(t, p, newUpdate("auth.example.com.", del)) - if rcode != dns.RcodeSuccess { - t.Fatalf("rcode = %d", rcode) + if rcode := runUpdate(t, p, newUpdate("auth.example.com", del)); rcode != dns.RcodeSuccess { + t.Fatalf("delete rcode = %d", rcode) } - if p.store.NameExists("foo.auth.example.com.") { - t.Errorf("name should be wiped") + + for _, rr := range readZoneRecords(t, p, "auth.example.com") { + if rr.Header().Rrtype == dns.TypeTXT && rr.Header().Name == "foo.auth.example.com." { + t.Errorf("TXT %s should be gone, still present: %s", rr.Header().Name, rr.String()) + } } } -func TestUpdate_OutsideZone_NOTZONE(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) +func TestUpdate_OutOfZone_Refused(t *testing.T) { + p := newTestPluginWithZone(t, "auth.example.com") - // Update tries to write into a different zone. - upd := newUpdate("auth.example.com.", - mustRR(t, `evil.different.tld. 60 IN TXT "nope"`), + upd := newUpdate("auth.example.com", + mustRR(t, `evil.other.tld. 60 IN TXT "nope"`), ) - rcode, _ := applyUpdateNoAuth(t, p, upd) - if rcode != dns.RcodeNotZone { + if rcode := runUpdate(t, p, upd); rcode != dns.RcodeNotZone { t.Errorf("rcode = %d, want NOTZONE (%d)", rcode, dns.RcodeNotZone) } } -func TestUpdate_ZoneSectionNotSOA_FORMERR(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) +func TestUpdate_UnknownZone_NOTAUTH(t *testing.T) { + p := newTestPluginWithZone(t, "auth.example.com") - // Hand-built broken UPDATE: zone section type is TXT, not SOA. + // Build UPDATE for a zone we don't manage. m := new(dns.Msg) - m.SetUpdate("auth.example.com.") - m.Question[0].Qtype = dns.TypeTXT // <-- wrong; must be SOA per RFC 2136 + m.SetUpdate("other.zone.") + m.Ns = []dns.RR{mustRR(t, `x.other.zone. 60 IN TXT "y"`)} - rcode, _ := applyUpdateNoAuth(t, p, m) - if rcode != dns.RcodeFormatError { - t.Errorf("rcode = %d, want FORMERR (%d)", rcode, dns.RcodeFormatError) - } -} - -func TestUpdate_UnauthorisedZone_NOTAUTH(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - - m := new(dns.Msg) - m.SetUpdate("not-our-zone.com.") // we're not authoritative for this - m.Ns = []dns.RR{mustRR(t, `x.not-our-zone.com. 60 IN TXT "hi"`)} - - rcode, _ := applyUpdateNoAuth(t, p, m) - if rcode != dns.RcodeNotAuth { + if rcode := runUpdate(t, p, m); rcode != dns.RcodeNotAuth { t.Errorf("rcode = %d, want NOTAUTH (%d)", rcode, dns.RcodeNotAuth) } } -func TestUpdate_PrereqNameExists_NXDOMAIN(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) +func TestUpdate_ApexSOADeletion_Refused(t *testing.T) { + p := newTestPluginWithZone(t, "auth.example.com") - // Prereq: name must exist (CLASS=ANY, TYPE=ANY). It doesn't. - prereq := &dns.ANY{Hdr: dns.RR_Header{ - Name: "ghost.auth.example.com.", + // CLASS=ANY type=SOA at apex → would wipe the SOA. + del := &dns.ANY{Hdr: dns.RR_Header{ + Name: "auth.example.com.", + Rrtype: dns.TypeSOA, + Class: dns.ClassANY, + }} + if rcode := runUpdate(t, p, newUpdate("auth.example.com", del)); rcode != dns.RcodeRefused { + t.Errorf("rcode = %d, want REFUSED", rcode) + } + + // SOA must still be in the file. + hasSOA := false + for _, rr := range readZoneRecords(t, p, "auth.example.com") { + if _, ok := rr.(*dns.SOA); ok { + hasSOA = true + break + } + } + if !hasSOA { + t.Errorf("SOA was deleted despite REFUSED rcode") + } +} + +func TestUpdate_ApexWipe_Refused(t *testing.T) { + p := newTestPluginWithZone(t, "auth.example.com") + + del := &dns.ANY{Hdr: dns.RR_Header{ + Name: "auth.example.com.", Rrtype: dns.TypeANY, Class: dns.ClassANY, }} - m := new(dns.Msg) - m.SetUpdate("auth.example.com.") - m.Answer = []dns.RR{prereq} - m.Ns = []dns.RR{mustRR(t, `x.auth.example.com. 60 IN TXT "y"`)} - - rcode, _ := applyUpdateNoAuth(t, p, m) - if rcode != dns.RcodeNameError { - t.Errorf("rcode = %d, want NXDOMAIN (%d)", rcode, dns.RcodeNameError) + if rcode := runUpdate(t, p, newUpdate("auth.example.com", del)); rcode != dns.RcodeRefused { + t.Errorf("rcode = %d, want REFUSED", rcode) } } func TestUpdate_PrereqRRsetMustNotExist_YXRRSET(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - p.store.Add(mustRR(t, `existing.auth.example.com. 60 IN TXT "present"`)) + p := newTestPluginWithZone(t, "auth.example.com") + + // Seed a TXT then send an UPDATE whose prereq says "no TXT here". + runUpdate(t, p, newUpdate("auth.example.com", + mustRR(t, `foo.auth.example.com. 60 IN TXT "present"`), + )) - // Prereq: TXT RRset must NOT exist at this name (CLASS=NONE, TYPE=TXT). prereq := &dns.ANY{Hdr: dns.RR_Header{ - Name: "existing.auth.example.com.", + Name: "foo.auth.example.com.", Rrtype: dns.TypeTXT, Class: dns.ClassNONE, }} m := new(dns.Msg) m.SetUpdate("auth.example.com.") m.Answer = []dns.RR{prereq} + m.Ns = []dns.RR{mustRR(t, `foo.auth.example.com. 60 IN TXT "should-not-be-added"`)} - rcode, _ := applyUpdateNoAuth(t, p, m) - if rcode != dns.RcodeYXRrset { - t.Errorf("rcode = %d, want YXRRSET (%d)", rcode, dns.RcodeYXRrset) + if rcode := runUpdate(t, p, m); rcode != dns.RcodeYXRrset { + t.Errorf("rcode = %d, want YXRRSET", rcode) } } -func TestUpdate_ApexSOA_RefusedForAdd(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) +func TestUpdate_NoOpAdd_DoesntRewriteFile(t *testing.T) { + p := newTestPluginWithZone(t, "auth.example.com") - // Attempting to add an SOA at the apex must be refused — we serve - // SOA synthetically. - soa := mustRR(t, `auth.example.com. 60 IN SOA ns.example.com. admin.auth.example.com. 1 3600 600 604800 60`) - rcode, _ := applyUpdateNoAuth(t, p, newUpdate("auth.example.com.", soa)) - if rcode != dns.RcodeRefused { - t.Errorf("rcode = %d, want REFUSED (%d) for SOA-at-apex add", rcode, dns.RcodeRefused) - } -} - -func TestUpdate_ApexDeletion_Refused(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - - // CLASS=ANY, TYPE=ANY at the apex → would wipe the zone. Refuse. - del := &dns.ANY{Hdr: dns.RR_Header{ - Name: "auth.example.com.", - Rrtype: dns.TypeANY, - Class: dns.ClassANY, - }} - rcode, _ := applyUpdateNoAuth(t, p, newUpdate("auth.example.com.", del)) - if rcode != dns.RcodeRefused { - t.Errorf("rcode = %d, want REFUSED for apex wipe", rcode) - } -} - -func TestUpdate_DefaultTTL_Applied(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - p.TTL = 120 // configure non-default - - // Build an UPDATE add with TTL=0 → plugin should fill in p.TTL. - rr := mustRR(t, `foo.auth.example.com. 0 IN TXT "x"`) - rcode, _ := applyUpdateNoAuth(t, p, newUpdate("auth.example.com.", rr)) - if rcode != dns.RcodeSuccess { - t.Fatalf("rcode = %d", rcode) - } - - got := p.store.Lookup("foo.auth.example.com.", dns.TypeTXT) - if got[0].Header().Ttl != 120 { - t.Errorf("TTL = %d, want default 120", got[0].Header().Ttl) - } -} - -func TestUpdate_GenerationBumps(t *testing.T) { - p := newTestPlugin("auth.example.com.", "ns.example.com.", nil) - start := p.store.generation() - - upd := newUpdate("auth.example.com.", + // Add a record once. + runUpdate(t, p, newUpdate("auth.example.com", mustRR(t, `foo.auth.example.com. 60 IN TXT "x"`), - ) - applyUpdateNoAuth(t, p, upd) + )) - if p.store.generation() <= start { - t.Errorf("generation did not bump: was %d, still %d", start, p.store.generation()) + path := p.zones["auth.example.com."].Path + beforeStat, _ := os.Stat(path) + + // Same UPDATE again — should be a no-op (dedupe). + runUpdate(t, p, newUpdate("auth.example.com", + mustRR(t, `foo.auth.example.com. 60 IN TXT "x"`), + )) + + afterStat, _ := os.Stat(path) + if afterStat.ModTime().After(beforeStat.ModTime()) { + t.Errorf("file was rewritten despite no-op update (mtime advanced)") + } +} + +func TestFindZone_LongestSuffixWins(t *testing.T) { + p := &RFC2136{Zones: []string{"example.com.", "auth.example.com."}} + if got := p.findZone("foo.auth.example.com."); got != "auth.example.com." { + t.Errorf("findZone returned %q, expected longest match", got) + } +} + +func TestFindZone_OutsideAllZones(t *testing.T) { + p := &RFC2136{Zones: []string{"auth.example.com."}} + if got := p.findZone("other.tld."); got != "" { + t.Errorf("findZone returned %q, want empty", got) + } +} + +func TestFindZone_CaseInsensitive(t *testing.T) { + p := &RFC2136{Zones: []string{"auth.example.com."}} + if got := p.findZone("Foo.AUTH.example.COM."); got != "auth.example.com." { + t.Errorf("case-insensitive findZone returned %q", got) } } diff --git a/zonefile.go b/zonefile.go new file mode 100644 index 0000000..cd35aed --- /dev/null +++ b/zonefile.go @@ -0,0 +1,325 @@ +package rfc2136 + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "time" + + "github.com/miekg/dns" +) + +// zoneFile is a file-backed authority for a single DNS zone. Replaces +// the Phase-1.3 in-memory recordStore. +// +// On every UPDATE, the file is read fully into memory as parsed RRs, +// the requested adds/deletes are applied to that slice, the SOA serial +// is bumped (CalVer YYYYMMDDNN style), and the file is rewritten via +// an atomic temp-file rename. CoreDNS's `auto` plugin notices the +// mtime change within its reload interval (~30s) and re-serves the +// zone. HE eventually pulls on its SOA refresh. +// +// Concurrency: per-zone mutex serializes RFC 2136 UPDATEs against +// each other and against the plugin's own reads. It does NOT protect +// against external editors (e.g. a human running an editor while the +// plugin is mid-write); that's the operator's responsibility, and +// the typical mitigation is to do manual edits when no UPDATEs are +// in flight (or just accept the rare race — the worst case is one +// lost manual edit, easily restored from git). +type zoneFile struct { + mu sync.Mutex + + // Path is the absolute path to the zone file on disk. + Path string + + // Origin is the canonical (lowercase, trailing dot) zone apex. + Origin string + + // AutoCommit, when true, runs `git add && git commit ...` + // after every successful write. Defaults to true (per the chosen + // architecture: every dynamic update should leave a git trail). + AutoCommit bool + + // GitAuthorName and GitAuthorEmail are passed to `git commit` + // via -c user.name and -c user.email so the commits are + // attributable without depending on the system git config. + GitAuthorName string + GitAuthorEmail string +} + +// canon normalises a DNS name to the store's internal form: lowercase +// with trailing dot. miekg/dns sometimes hands us names without the +// trailing dot; passing through this once at the boundary keeps every +// lookup, every comparison consistent. +func canon(name string) string { + return strings.ToLower(dns.Fqdn(name)) +} + +// openZoneFile prepares a zoneFile handle. Does NOT read or parse the +// file; that happens lazily in each operation (so the file's content +// is always fresh and we never serve a stale snapshot). +func openZoneFile(path, origin string) *zoneFile { + return &zoneFile{ + Path: path, + Origin: canon(origin), + AutoCommit: true, + GitAuthorName: "coredns-rfc2136", + GitAuthorEmail: "rfc2136@coredns", + } +} + +// loadRRs reads the zone file and parses it into an RR slice via +// miekg/dns's zone parser. The parser handles $ORIGIN, $TTL, multi-line +// SOA, comments, includes, etc. +func (z *zoneFile) loadRRs() ([]dns.RR, error) { + f, err := os.Open(z.Path) + if err != nil { + return nil, fmt.Errorf("open %s: %w", z.Path, err) + } + defer f.Close() + + parser := dns.NewZoneParser(f, z.Origin, z.Path) + parser.SetDefaultTTL(3600) + + var rrs []dns.RR + for rr, ok := parser.Next(); ok; rr, ok = parser.Next() { + rrs = append(rrs, rr) + } + if err := parser.Err(); err != nil { + return nil, fmt.Errorf("parse %s: %w", z.Path, err) + } + if len(rrs) == 0 { + return nil, fmt.Errorf("%s: zero RRs parsed", z.Path) + } + return rrs, nil +} + +// Lookup returns RRs in `rrs` matching (name, rtype). Both name and +// the RR header names are canonicalised for the comparison. Pass-by- +// slice rather than holding state means we can let the caller batch +// multiple operations against one snapshot of the file. +func lookupIn(rrs []dns.RR, name string, rtype uint16) []dns.RR { + name = canon(name) + var out []dns.RR + for _, rr := range rrs { + hdr := rr.Header() + if canon(hdr.Name) == name && hdr.Rrtype == rtype { + out = append(out, rr) + } + } + return out +} + +// nameExistsIn reports whether any RR's owner equals name (canonical). +func nameExistsIn(rrs []dns.RR, name string) bool { + name = canon(name) + for _, rr := range rrs { + if canon(rr.Header().Name) == name { + return true + } + } + return false +} + +// removeRRsetFrom returns rrs minus every RR matching (name, rtype). +func removeRRsetFrom(rrs []dns.RR, name string, rtype uint16) []dns.RR { + name = canon(name) + out := rrs[:0:0] + for _, rr := range rrs { + hdr := rr.Header() + if canon(hdr.Name) == name && hdr.Rrtype == rtype { + continue + } + out = append(out, rr) + } + return out +} + +// removeNameFrom returns rrs minus every RR with the given owner name. +func removeNameFrom(rrs []dns.RR, name string) []dns.RR { + name = canon(name) + out := rrs[:0:0] + for _, rr := range rrs { + if canon(rr.Header().Name) == name { + continue + } + out = append(out, rr) + } + return out +} + +// removeRRFrom returns rrs minus the single RR matching the given one +// by owner + type + rdata. String() comparison covers rdata exactness. +func removeRRFrom(rrs []dns.RR, target dns.RR) []dns.RR { + targetStr := target.String() + out := rrs[:0:0] + matched := false + for _, rr := range rrs { + if !matched && rr.String() == targetStr { + matched = true + continue + } + out = append(out, rr) + } + return out +} + +// addRRTo appends rr to rrs unless an identical RR already exists +// (de-dupe semantics per RFC 2136 §3.4.2.2). +func addRRTo(rrs []dns.RR, rr dns.RR) []dns.RR { + target := rr.String() + for _, existing := range rrs { + if existing.String() == target { + return rrs + } + } + return append(rrs, rr) +} + +// bumpSerial advances the SOA's serial in CalVer (YYYYMMDDNN) form. +// Behaviour: +// - If today is later than the existing serial's date, jump to +// today with NN=01. +// - Otherwise (same day, or serial-date is in the future), bump NN. +// - Caps at NN=99; returns an error if exceeded. +// +// The SOA is found by type (there should be exactly one); mutated in +// place. The returned slice is the same slice with the SOA's serial +// updated. +func bumpSerial(rrs []dns.RR, now time.Time) error { + var soa *dns.SOA + for _, rr := range rrs { + if s, ok := rr.(*dns.SOA); ok { + soa = s + break + } + } + if soa == nil { + return fmt.Errorf("zone has no SOA record") + } + + today := now.UTC().Format("20060102") + cur := fmt.Sprintf("%d", soa.Serial) + + if len(cur) == 10 && cur[:8] == today { + nn := atoi(cur[8:10]) + if nn >= 99 { + return fmt.Errorf("serial counter exhausted for %s (NN=99)", today) + } + soa.Serial = uint32(parseUint(today)*100 + uint64(nn+1)) + return nil + } + + // Either the date is older, or the format isn't CalVer at all — + // either way, today + 01 is the next valid serial. + soa.Serial = uint32(parseUint(today)*100 + 1) + return nil +} + +// atoi is a tiny helper that ignores errors — only called on a +// substring we already validated is two digits. +func atoi(s string) int { + n := 0 + for _, c := range s { + n = n*10 + int(c-'0') + } + return n +} + +// parseUint parses an all-digits string into a uint64. Used because +// strconv.ParseUint adds error-handling overhead we don't need on +// internally-controlled inputs. +func parseUint(s string) uint64 { + var n uint64 + for _, c := range s { + n = n*10 + uint64(c-'0') + } + return n +} + +// writeAtomic serializes rrs to a temp file in the same directory as +// z.Path, then renames over the destination. POSIX guarantees atomic +// rename on local filesystems, so a partial write can never leave a +// corrupt zone file on disk. +// +// Format: one RR per line, tab-separated owner/TTL/class/type/rdata. +// Comments and multi-line SOA formatting from the original file are +// NOT preserved (v1 limitation; sophisticated comment preservation can +// land in v2). A short header line is emitted with the write timestamp +// and the plugin name, so it's obvious in `git log` what touched the +// file. +func (z *zoneFile) writeAtomic(rrs []dns.RR, now time.Time) error { + dir := filepath.Dir(z.Path) + tmp, err := os.CreateTemp(dir, ".rfc2136-*.zone") + if err != nil { + return fmt.Errorf("create temp: %w", err) + } + tmpPath := tmp.Name() + // Best-effort cleanup if we fail before the rename. + defer func() { + if tmpPath != "" { + _ = os.Remove(tmpPath) + } + }() + + header := fmt.Sprintf("; Auto-written by coredns-rfc2136 on %s\n; Zone: %s\n$ORIGIN %s\n", + now.UTC().Format(time.RFC3339), z.Origin, z.Origin) + if _, err := tmp.WriteString(header); err != nil { + _ = tmp.Close() + return fmt.Errorf("write header: %w", err) + } + + for _, rr := range rrs { + if _, err := tmp.WriteString(rr.String() + "\n"); err != nil { + _ = tmp.Close() + return fmt.Errorf("write rr: %w", err) + } + } + + if err := tmp.Sync(); err != nil { + _ = tmp.Close() + return fmt.Errorf("sync: %w", err) + } + if err := tmp.Close(); err != nil { + return fmt.Errorf("close: %w", err) + } + if err := os.Rename(tmpPath, z.Path); err != nil { + return fmt.Errorf("rename %s -> %s: %w", tmpPath, z.Path, err) + } + tmpPath = "" // suppress cleanup; rename consumed it + return nil +} + +// commit stages and commits the zone file via git. Runs from the +// repository directory inferred from the zone file's parent. Returns +// nil silently if AutoCommit is false. Returns an error if the commit +// fails; the caller decides whether to roll back the file write. +func (z *zoneFile) commit(message string) error { + if !z.AutoCommit { + return nil + } + // We run git from the directory containing the zone file. git will + // walk upward to find the .git dir. + dir := filepath.Dir(z.Path) + // `git add` first; if file is already in the index, no harm done. + add := exec.Command("git", + "-C", dir, + "add", "--", z.Path, + ) + if out, err := add.CombinedOutput(); err != nil { + return fmt.Errorf("git add failed: %w: %s", err, strings.TrimSpace(string(out))) + } + commit := exec.Command("git", + "-C", dir, + "-c", "user.name="+z.GitAuthorName, + "-c", "user.email="+z.GitAuthorEmail, + "commit", "-q", "-m", message, "--", z.Path, + ) + if out, err := commit.CombinedOutput(); err != nil { + return fmt.Errorf("git commit failed: %w: %s", err, strings.TrimSpace(string(out))) + } + return nil +} diff --git a/zonefile_test.go b/zonefile_test.go new file mode 100644 index 0000000..22a12af --- /dev/null +++ b/zonefile_test.go @@ -0,0 +1,261 @@ +package rfc2136 + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/miekg/dns" +) + +// mustRR is a test helper that parses an RR string or fails the test. +// Used widely in zonefile + update tests. +func mustRR(t *testing.T, s string) dns.RR { + t.Helper() + rr, err := dns.NewRR(s) + if err != nil { + t.Fatalf("failed to parse RR %q: %v", s, err) + } + return rr +} + +func TestZoneFile_LoadRRs(t *testing.T) { + dir := withTempZonesDir(t, "auth.example.com") + zf := openZoneFile(filepath.Join(dir, "auth.example.com.zone"), "auth.example.com.") + + rrs, err := zf.loadRRs() + if err != nil { + t.Fatalf("loadRRs: %v", err) + } + if len(rrs) < 2 { + t.Fatalf("expected >=2 RRs (SOA + NS), got %d", len(rrs)) + } + // First RR should be the SOA. + if _, ok := rrs[0].(*dns.SOA); !ok { + t.Errorf("first RR is %T, want SOA", rrs[0]) + } +} + +func TestZoneFile_LoadRRs_MissingFile(t *testing.T) { + zf := openZoneFile("/nope/missing.zone", "missing.") + if _, err := zf.loadRRs(); err == nil { + t.Errorf("expected error loading missing file, got nil") + } +} + +func TestLookupIn_CaseInsensitive(t *testing.T) { + rrs := []dns.RR{ + mustRR(t, `FOO.example.com. 60 IN TXT "hello"`), + } + if got := lookupIn(rrs, "foo.EXAMPLE.com.", dns.TypeTXT); len(got) != 1 { + t.Errorf("case-insensitive lookup failed: %d results", len(got)) + } +} + +func TestLookupIn_WrongTypeReturnsEmpty(t *testing.T) { + rrs := []dns.RR{mustRR(t, `foo.example.com. 60 IN A 192.0.2.1`)} + if got := lookupIn(rrs, "foo.example.com.", dns.TypeTXT); len(got) != 0 { + t.Errorf("wrong-type lookup returned %d results, want 0", len(got)) + } +} + +func TestNameExistsIn(t *testing.T) { + rrs := []dns.RR{mustRR(t, `foo.example.com. 60 IN A 192.0.2.1`)} + if !nameExistsIn(rrs, "foo.example.com.") { + t.Errorf("nameExistsIn returned false for present name") + } + if nameExistsIn(rrs, "bar.example.com.") { + t.Errorf("nameExistsIn returned true for absent name") + } +} + +func TestRemoveRRsetFrom(t *testing.T) { + rrs := []dns.RR{ + mustRR(t, `foo.example.com. 60 IN TXT "a"`), + mustRR(t, `foo.example.com. 60 IN TXT "b"`), + mustRR(t, `foo.example.com. 60 IN A 192.0.2.1`), + mustRR(t, `bar.example.com. 60 IN A 192.0.2.2`), + } + out := removeRRsetFrom(rrs, "foo.example.com.", dns.TypeTXT) + if len(out) != 2 { + t.Errorf("after removing TXT rrset, got %d records, want 2", len(out)) + } + // The remaining records: foo's A + bar's A + for _, rr := range out { + if rr.Header().Rrtype == dns.TypeTXT { + t.Errorf("a TXT slipped through removal: %s", rr.String()) + } + } +} + +func TestRemoveRRFrom_ExactMatchOnly(t *testing.T) { + rrs := []dns.RR{ + mustRR(t, `foo.example.com. 60 IN TXT "keep"`), + mustRR(t, `foo.example.com. 60 IN TXT "drop"`), + } + out := removeRRFrom(rrs, mustRR(t, `foo.example.com. 60 IN TXT "drop"`)) + if len(out) != 1 || out[0].(*dns.TXT).Txt[0] != "keep" { + t.Errorf("removeRRFrom wrong result: %v", out) + } +} + +func TestRemoveNameFrom(t *testing.T) { + rrs := []dns.RR{ + mustRR(t, `foo.example.com. 60 IN TXT "x"`), + mustRR(t, `foo.example.com. 60 IN A 192.0.2.1`), + mustRR(t, `bar.example.com. 60 IN A 192.0.2.2`), + } + out := removeNameFrom(rrs, "foo.example.com.") + if len(out) != 1 || out[0].Header().Name != "bar.example.com." { + t.Errorf("removeNameFrom left %v, want only bar", out) + } +} + +func TestAddRRTo_Dedupes(t *testing.T) { + rrs := []dns.RR{mustRR(t, `foo.example.com. 60 IN TXT "same"`)} + out := addRRTo(rrs, mustRR(t, `foo.example.com. 60 IN TXT "same"`)) + if len(out) != 1 { + t.Errorf("identical RR was duplicated: %v", out) + } +} + +func TestBumpSerial_SameDay_NNIncrement(t *testing.T) { + rrs := []dns.RR{ + &dns.SOA{Hdr: dns.RR_Header{Name: "example.com.", Rrtype: dns.TypeSOA}, + Serial: 2026052105}, + } + // Use a 'now' that matches today's serial date. + now := time.Date(2026, 5, 21, 12, 0, 0, 0, time.UTC) + if err := bumpSerial(rrs, now); err != nil { + t.Fatalf("bumpSerial: %v", err) + } + if got := rrs[0].(*dns.SOA).Serial; got != 2026052106 { + t.Errorf("Serial = %d, want 2026052106", got) + } +} + +func TestBumpSerial_NewDay_DateAdvancesNN01(t *testing.T) { + rrs := []dns.RR{ + &dns.SOA{Hdr: dns.RR_Header{Name: "example.com.", Rrtype: dns.TypeSOA}, + Serial: 2026052099}, // exhausted yesterday + } + now := time.Date(2026, 5, 21, 1, 0, 0, 0, time.UTC) + if err := bumpSerial(rrs, now); err != nil { + t.Fatalf("bumpSerial: %v", err) + } + if got := rrs[0].(*dns.SOA).Serial; got != 2026052101 { + t.Errorf("Serial = %d, want 2026052101 (today, NN=01)", got) + } +} + +func TestBumpSerial_NNExhausted_ReturnsError(t *testing.T) { + rrs := []dns.RR{ + &dns.SOA{Hdr: dns.RR_Header{Name: "example.com.", Rrtype: dns.TypeSOA}, + Serial: 2026052199}, // today, NN=99 + } + now := time.Date(2026, 5, 21, 1, 0, 0, 0, time.UTC) + if err := bumpSerial(rrs, now); err == nil { + t.Errorf("expected error on NN=99, got nil") + } +} + +func TestBumpSerial_NonCalVerFormat_ResetsToToday(t *testing.T) { + rrs := []dns.RR{ + &dns.SOA{Hdr: dns.RR_Header{Name: "example.com.", Rrtype: dns.TypeSOA}, + Serial: 12345}, // random unix-ish serial, not CalVer + } + now := time.Date(2026, 5, 21, 1, 0, 0, 0, time.UTC) + if err := bumpSerial(rrs, now); err != nil { + t.Fatalf("bumpSerial: %v", err) + } + if got := rrs[0].(*dns.SOA).Serial; got != 2026052101 { + t.Errorf("Serial = %d, want 2026052101", got) + } +} + +func TestBumpSerial_NoSOA_ReturnsError(t *testing.T) { + rrs := []dns.RR{mustRR(t, `foo.example.com. 60 IN A 192.0.2.1`)} + now := time.Now() + if err := bumpSerial(rrs, now); err == nil { + t.Errorf("expected error when no SOA in zone, got nil") + } +} + +func TestZoneFile_WriteAtomic_RoundTrip(t *testing.T) { + dir := withTempZonesDir(t, "auth.example.com") + path := filepath.Join(dir, "auth.example.com.zone") + zf := openZoneFile(path, "auth.example.com.") + zf.AutoCommit = false // not testing commit here + + rrs, err := zf.loadRRs() + if err != nil { + t.Fatalf("initial load: %v", err) + } + originalCount := len(rrs) + + // Add a record + write. + rrs = addRRTo(rrs, mustRR(t, `new.auth.example.com. 60 IN TXT "added"`)) + if err := zf.writeAtomic(rrs, time.Now()); err != nil { + t.Fatalf("writeAtomic: %v", err) + } + + // Re-load and verify. + after, err := zf.loadRRs() + if err != nil { + t.Fatalf("re-load: %v", err) + } + if len(after) != originalCount+1 { + t.Errorf("RR count = %d, want %d", len(after), originalCount+1) + } + found := false + for _, rr := range after { + if txt, ok := rr.(*dns.TXT); ok && txt.Hdr.Name == "new.auth.example.com." { + found = true + break + } + } + if !found { + t.Errorf("added TXT not in reloaded zone") + } +} + +func TestZoneFile_WriteAtomic_LeavesNoTempFile(t *testing.T) { + dir := withTempZonesDir(t, "auth.example.com") + path := filepath.Join(dir, "auth.example.com.zone") + zf := openZoneFile(path, "auth.example.com.") + zf.AutoCommit = false + + rrs, _ := zf.loadRRs() + _ = zf.writeAtomic(rrs, time.Now()) + + // No .rfc2136-*.zone temp files should remain. + matches, _ := filepath.Glob(filepath.Join(dir, ".rfc2136-*.zone")) + if len(matches) != 0 { + t.Errorf("temp files leaked: %v", matches) + } +} + +func TestZoneFile_WriteAtomic_FileEndsWithNewline(t *testing.T) { + dir := withTempZonesDir(t, "auth.example.com") + path := filepath.Join(dir, "auth.example.com.zone") + zf := openZoneFile(path, "auth.example.com.") + zf.AutoCommit = false + + rrs, _ := zf.loadRRs() + _ = zf.writeAtomic(rrs, time.Now()) + + data, _ := os.ReadFile(path) + if !strings.HasSuffix(string(data), "\n") { + t.Errorf("written file should end with newline; tail: %q", + data[max(0, len(data)-20):]) + } +} + +func max(a, b int) int { + if a > b { + return a + } + return b +}