From f1d47c8ff7d9cf29d72d761ed9638d4afe09599c Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 19 Sep 2026 22:33:48 +1000 Subject: [PATCH] Inspect zone file and journal before seeding a zone Fixes #19 --- internal/bind/seed.go | 20 +- internal/bind/zonestate.go | 221 +++++++++++++++++++ internal/bind/zonestate_test.go | 244 +++++++++++++++++++++ internal/controller/bindzone_controller.go | 15 +- 4 files changed, 491 insertions(+), 9 deletions(-) create mode 100644 internal/bind/zonestate.go create mode 100644 internal/bind/zonestate_test.go diff --git a/internal/bind/seed.go b/internal/bind/seed.go index e168a64..c197337 100644 --- a/internal/bind/seed.go +++ b/internal/bind/seed.go @@ -26,13 +26,11 @@ func (e *Executor) ZoneExists(ctx context.Context, namespace, pod, zone, view st return err == nil } -// WriteSeedZone writes a minimal loadable zone file (SOA + apex NS + glue) to -// path, creating parent directories. The apex NS is the in-zone name ns1, and a -// glue A record pointing at primaryIP is included so BIND's check-integrity -// accepts the zone (an in-zone NS without an address record is a load error). -// It is only safe to call when creating a zone, as it overwrites any existing -// file. This is a placeholder that is replaced once real records are loaded. -func (e *Executor) WriteSeedZone(ctx context.Context, namespace, pod, zone, path, primaryIP string, serial int64) error { +// renderSeedZone renders a minimal loadable zone (SOA + apex NS + glue). The +// apex NS is the in-zone name ns1, and a glue A record pointing at primaryIP is +// included so BIND's check-integrity accepts the zone (an in-zone NS without an +// address record is a load error). +func renderSeedZone(zone, primaryIP string, serial int64) string { origin := dot(zone) ns := "ns1." + origin // Short refresh/retry so a secondary that misses a NOTIFY (e.g. its pod IP @@ -41,7 +39,7 @@ func (e *Executor) WriteSeedZone(ctx context.Context, namespace, pod, zone, path // negative-cache TTL: keep it low so a stale-secondary NXDOMAIN does not // stick in downstream resolvers for long. NOTIFY (also-notify on the // primary) remains the fast path; these are the fallback. - content := fmt.Sprintf(`$TTL 3600 + return fmt.Sprintf(`$TTL 3600 @ IN SOA %s hostmaster.%s ( %d ; serial 300 ; refresh @@ -51,7 +49,13 @@ func (e *Executor) WriteSeedZone(ctx context.Context, namespace, pod, zone, path @ IN NS %s ns1 IN A %s `, ns, origin, serial, ns, primaryIP) +} +// WriteSeedZone writes a seed zone file to path, creating parent directories. +// It overwrites any existing file, so callers must clear it with PlanSeed +// first. This is a placeholder that is replaced once real records are loaded. +func (e *Executor) WriteSeedZone(ctx context.Context, namespace, pod, zone, path, primaryIP string, serial int64) error { + content := renderSeedZone(zone, primaryIP, serial) cmd := []string{"sh", "-c", fmt.Sprintf("mkdir -p \"$(dirname '%s')\" && cat > '%s'", path, path)} if out, err := e.Exec(ctx, namespace, pod, cmd, content); err != nil { return fmt.Errorf("seed zone %s: %w (out: %s)", zone, err, out) diff --git a/internal/bind/zonestate.go b/internal/bind/zonestate.go new file mode 100644 index 0000000..89808ba --- /dev/null +++ b/internal/bind/zonestate.go @@ -0,0 +1,221 @@ +package bind + +import ( + "context" + "encoding/hex" + "fmt" + "strconv" + "strings" +) + +// JournalPath returns the BIND journal that accompanies a zone database file. +func JournalPath(zonePath string) string { return zonePath + ".jnl" } + +// ZoneDiskState is what the primary pod's filesystem holds for one zone. +// A journal is only replayable onto a zone file whose SOA serial lies within +// [JournalBegin, JournalEnd]; outside that range BIND fails the load with +// "out of range" and the zone never comes up. +type ZoneDiskState struct { + ZoneFile bool + ZoneSerial int64 + ZoneSerialOK bool + Journal bool + JournalBegin int64 + JournalEnd int64 + JournalOK bool +} + +// SeedPlan is the decision taken before writing a skeleton zone file. +type SeedPlan struct { + WriteSeed bool + Serial int64 + QuarantineZoneFile bool + QuarantineJournal bool + QuarantineSuffix string +} + +// PlanSeed decides how to make a zone loadable without discarding live data. +// Nothing is ever deleted: unusable files are renamed aside so they stay +// recoverable on the PVC. +func PlanSeed(st ZoneDiskState) SeedPlan { + suffix := fmt.Sprintf(".orphaned-%d", highestSerial(st)) + + if !st.ZoneFile { + p := SeedPlan{WriteSeed: true, Serial: nextSerial(st), QuarantineSuffix: suffix} + // A journal without its zone file can only refuse to replay. + p.QuarantineJournal = st.Journal + return p + } + + if !st.Journal || !st.JournalOK || !st.ZoneSerialOK { + return SeedPlan{} + } + + switch { + case serialLT(st.ZoneSerial, st.JournalBegin): + // The file has regressed behind the journal: it is the skeleton a + // previous reconcile clobbered it with. Keep both aside and reseed + // above the journal so secondaries still see a serial increase. + return SeedPlan{ + WriteSeed: true, + Serial: nextSerial(st), + QuarantineZoneFile: true, + QuarantineJournal: true, + QuarantineSuffix: suffix, + } + case serialLT(st.JournalEnd, st.ZoneSerial): + return SeedPlan{QuarantineJournal: true, QuarantineSuffix: suffix} + default: + return SeedPlan{} + } +} + +func highestSerial(st ZoneDiskState) int64 { + var h int64 + if st.ZoneFile && st.ZoneSerialOK { + h = st.ZoneSerial + } + if st.Journal && st.JournalOK && serialLT(h, st.JournalEnd) { + h = st.JournalEnd + } + return h +} + +func nextSerial(st ZoneDiskState) int64 { + next := int64(uint32(highestSerial(st)) + 1) + if next == 0 { + return 1 + } + return next +} + +// serialLT compares DNS serials in RFC 1982 sequence space. +func serialLT(a, b int64) bool { + if a == b { + return false + } + return uint32(b)-uint32(a) < 1<<31 +} + +// ParseZoneSerial extracts the SOA serial from the head of a zone file, in +// both the operator's seed layout and BIND's own multi-line dump layout. +func ParseZoneSerial(content string) (int64, bool) { + var tokens []string + for _, line := range strings.Split(content, "\n") { + if i := strings.IndexByte(line, ';'); i >= 0 { + line = line[:i] + } + line = strings.ReplaceAll(line, "(", " ( ") + line = strings.ReplaceAll(line, ")", " ) ") + tokens = append(tokens, strings.Fields(line)...) + } + for i, tok := range tokens { + if !strings.EqualFold(tok, "SOA") { + continue + } + rest := tokens[i+1:] + if len(rest) < 3 { + return 0, false + } + rest = rest[2:] // MNAME, RNAME + if rest[0] == "(" { + rest = rest[1:] + } + if len(rest) == 0 { + return 0, false + } + serial, err := strconv.ParseUint(rest[0], 10, 32) + if err != nil { + return 0, false + } + return int64(serial), true + } + return 0, false +} + +const journalMagic = ";BIND LOG V9" + +// parseJournalHeader reads the begin and end serials from a BIND journal +// header: a 16-byte format magic followed by two {serial,offset} big-endian +// pairs. +func parseJournalHeader(b []byte) (begin, end int64, ok bool) { + if len(b) < 28 || !strings.HasPrefix(string(b[:16]), journalMagic) { + return 0, 0, false + } + return int64(beUint32(b[16:20])), int64(beUint32(b[24:28])), true +} + +func beUint32(b []byte) uint32 { + return uint32(b[0])<<24 | uint32(b[1])<<16 | uint32(b[2])<<8 | uint32(b[3]) +} + +// zoneStateProbe reads only the head of the zone file: the SOA is the first +// record in both layouts the operator has to read. +func zoneStateProbe(path string) string { + jnl := JournalPath(path) + return strings.Join([]string{ + fmt.Sprintf("if [ -f '%s' ]; then printf 'zonefile=1\\nhead<<\\n'; head -c 4096 '%s'; printf '\\n>>head\\n'; else printf 'zonefile=0\\n'; fi", path, path), + fmt.Sprintf("if [ -f '%s' ]; then printf 'journal=1\\njnl=%%s\\n' \"$(od -An -v -tx1 -N32 '%s' | tr -d ' \\n')\"; else printf 'journal=0\\n'; fi", jnl, jnl), + }, "\n") +} + +func parseZoneDiskState(out string) ZoneDiskState { + var st ZoneDiskState + var head []string + inHead := false + for _, line := range strings.Split(out, "\n") { + switch { + case inHead && line == ">>head": + inHead = false + case inHead: + head = append(head, line) + case line == "head<<": + inHead = true + case line == "zonefile=1": + st.ZoneFile = true + case line == "journal=1": + st.Journal = true + case strings.HasPrefix(line, "jnl="): + if raw, err := hex.DecodeString(strings.TrimPrefix(line, "jnl=")); err == nil { + st.JournalBegin, st.JournalEnd, st.JournalOK = parseJournalHeader(raw) + } + } + } + if st.ZoneFile { + st.ZoneSerial, st.ZoneSerialOK = ParseZoneSerial(strings.Join(head, "\n")) + } + return st +} + +// ZoneDiskState inspects the zone database file and journal on the pod. +func (e *Executor) ZoneDiskState(ctx context.Context, namespace, pod, path string) (ZoneDiskState, error) { + out, err := e.Exec(ctx, namespace, pod, []string{"sh", "-c", zoneStateProbe(path)}, "") + if err != nil { + return ZoneDiskState{}, fmt.Errorf("inspect zone files %s: %w (out: %s)", path, err, out) + } + return parseZoneDiskState(out), nil +} + +// Quarantine renames the files the plan marks unusable, leaving them on the +// PVC under a suffixed name. +func (e *Executor) Quarantine(ctx context.Context, namespace, pod, path string, plan SeedPlan) error { + var cmds []string + if plan.QuarantineZoneFile { + cmds = append(cmds, moveAside(path, plan.QuarantineSuffix)) + } + if plan.QuarantineJournal { + cmds = append(cmds, moveAside(JournalPath(path), plan.QuarantineSuffix)) + } + if len(cmds) == 0 { + return nil + } + cmd := []string{"sh", "-c", strings.Join(cmds, "\n")} + if out, err := e.Exec(ctx, namespace, pod, cmd, ""); err != nil { + return fmt.Errorf("quarantine zone files %s: %w (out: %s)", path, err, out) + } + return nil +} + +func moveAside(path, suffix string) string { + return fmt.Sprintf("if [ -f '%s' ]; then mv -- '%s' '%s%s'; fi", path, path, path, suffix) +} diff --git a/internal/bind/zonestate_test.go b/internal/bind/zonestate_test.go new file mode 100644 index 0000000..9ffcba2 --- /dev/null +++ b/internal/bind/zonestate_test.go @@ -0,0 +1,244 @@ +package bind + +import ( + "encoding/hex" + "strings" + "testing" +) + +func journalHeader(magic string, begin, end uint32) []byte { + b := make([]byte, 32) + copy(b, magic) + put := func(off int, v uint32) { + b[off] = byte(v >> 24) + b[off+1] = byte(v >> 16) + b[off+2] = byte(v >> 8) + b[off+3] = byte(v) + } + put(16, begin) + put(24, end) + return b +} + +func TestParseZoneSerial(t *testing.T) { + bindDump := `$ORIGIN . +$TTL 3600 ; 1 hour +k8s.syd1.au.unkin.net IN SOA ns1.k8s.syd1.au.unkin.net. hostmaster.k8s.syd1.au.unkin.net. ( + 16 ; serial + 300 ; refresh (5 minutes) + 60 ; retry (1 minute) + 1209600 ; expire (2 weeks) + 60 ; minimum (1 minute) + ) +` + cases := []struct { + name string + content string + want int64 + ok bool + }{ + {"bind dump", bindDump, 16, true}, + {"seed", renderSeedZone("example.com", "10.0.0.1", 42), 42, true}, + {"single line", "@ IN SOA ns1.example.com. hostmaster.example.com. 7 300 60 1209600 60\n", 7, true}, + {"glued paren", "@ IN SOA ns. host. (9 300 60 1209600 60)\n", 9, true}, + {"no soa", "$TTL 3600\nwww IN A 192.0.2.1\n", 0, false}, + {"truncated", "@ IN SOA ns.\n", 0, false}, + {"non numeric serial", "@ IN SOA ns. host. ( abc 300 )\n", 0, false}, + } + for _, c := range cases { + got, ok := ParseZoneSerial(c.content) + if got != c.want || ok != c.ok { + t.Errorf("%s: ParseZoneSerial = (%d,%v) want (%d,%v)", c.name, got, ok, c.want, c.ok) + } + } +} + +func TestParseJournalHeader(t *testing.T) { + begin, end, ok := parseJournalHeader(journalHeader(";BIND LOG V9.2\n", 10, 16)) + if !ok || begin != 10 || end != 16 { + t.Errorf("V9.2 header = (%d,%d,%v) want (10,16,true)", begin, end, ok) + } + if _, _, ok := parseJournalHeader(journalHeader("not a journal\n", 10, 16)); ok { + t.Error("bad magic should not parse") + } + if _, _, ok := parseJournalHeader([]byte(";BIND LOG V9.2\n")); ok { + t.Error("truncated header should not parse") + } +} + +func TestParseZoneDiskState(t *testing.T) { + out := strings.Join([]string{ + "zonefile=1", + "head<<", + "@ IN SOA ns. host. ( 5 300 60 1209600 60 )", + ">>head", + "journal=1", + "jnl=" + hex.EncodeToString(journalHeader(";BIND LOG V9.2\n", 3, 8)), + "", + }, "\n") + st := parseZoneDiskState(out) + if !st.ZoneFile || !st.ZoneSerialOK || st.ZoneSerial != 5 { + t.Errorf("zone file state wrong: %+v", st) + } + if !st.Journal || !st.JournalOK || st.JournalBegin != 3 || st.JournalEnd != 8 { + t.Errorf("journal state wrong: %+v", st) + } + + empty := parseZoneDiskState("zonefile=0\njournal=0\n") + if empty.ZoneFile || empty.Journal { + t.Errorf("empty state wrong: %+v", empty) + } +} + +// applyPlan models what the pod filesystem looks like after the plan runs, so +// a second PlanSeed can be checked for idempotence. +func applyPlan(st ZoneDiskState, p SeedPlan) ZoneDiskState { + if p.QuarantineJournal { + st.Journal, st.JournalBegin, st.JournalEnd, st.JournalOK = false, 0, 0, false + } + if p.QuarantineZoneFile { + st.ZoneFile, st.ZoneSerial, st.ZoneSerialOK = false, 0, false + } + if p.WriteSeed { + st.ZoneFile, st.ZoneSerial, st.ZoneSerialOK = true, p.Serial, true + } + return st +} + +func TestPlanSeedFreshInstall(t *testing.T) { + p := PlanSeed(ZoneDiskState{}) + if !p.WriteSeed || p.Serial != 1 { + t.Fatalf("fresh install should seed at serial 1, got %+v", p) + } + if p.QuarantineZoneFile || p.QuarantineJournal { + t.Errorf("fresh install should quarantine nothing, got %+v", p) + } +} + +func TestPlanSeedOrphanJournal(t *testing.T) { + st := ZoneDiskState{Journal: true, JournalBegin: 10, JournalEnd: 16, JournalOK: true} + p := PlanSeed(st) + if !p.WriteSeed { + t.Fatalf("orphan journal should still seed, got %+v", p) + } + if !p.QuarantineJournal || p.QuarantineZoneFile { + t.Errorf("only the journal should be quarantined, got %+v", p) + } + if p.Serial <= 16 { + t.Errorf("seed serial %d must exceed the journal end serial 16", p.Serial) + } + if p.QuarantineSuffix != ".orphaned-16" { + t.Errorf("quarantine suffix should be deterministic, got %q", p.QuarantineSuffix) + } +} + +func TestPlanSeedFileRegressedBehindJournal(t *testing.T) { + // The production failure: a skeleton at serial 1 left next to a journal at + // serial 16, which BIND refuses to replay ("out of range"). + st := ZoneDiskState{ + ZoneFile: true, ZoneSerial: 1, ZoneSerialOK: true, + Journal: true, JournalBegin: 10, JournalEnd: 16, JournalOK: true, + } + p := PlanSeed(st) + if !p.WriteSeed || p.Serial <= 16 { + t.Fatalf("reseed must land above the journal end serial, got %+v", p) + } + if !p.QuarantineJournal || !p.QuarantineZoneFile { + t.Errorf("the unloadable pair should both be moved aside, got %+v", p) + } + + after := applyPlan(st, p) + if after.Journal { + t.Error("journal should be gone after quarantine") + } + if next := PlanSeed(after); next != (SeedPlan{}) { + t.Errorf("second reconcile should be a no-op, got %+v", next) + } + if after.ZoneSerial != p.Serial { + t.Errorf("second reconcile changed the serial: %d want %d", after.ZoneSerial, p.Serial) + } +} + +func TestPlanSeedLeavesHealthyZoneAlone(t *testing.T) { + cases := []struct { + name string + st ZoneDiskState + }{ + {"file and covering journal", ZoneDiskState{ + ZoneFile: true, ZoneSerial: 16, ZoneSerialOK: true, + Journal: true, JournalBegin: 10, JournalEnd: 20, JournalOK: true, + }}, + {"file at journal end", ZoneDiskState{ + ZoneFile: true, ZoneSerial: 20, ZoneSerialOK: true, + Journal: true, JournalBegin: 10, JournalEnd: 20, JournalOK: true, + }}, + {"file without journal", ZoneDiskState{ZoneFile: true, ZoneSerial: 16, ZoneSerialOK: true}}, + {"unreadable journal header", ZoneDiskState{ + ZoneFile: true, ZoneSerial: 16, ZoneSerialOK: true, Journal: true, + }}, + {"unparsable zone file", ZoneDiskState{ZoneFile: true}}, + } + for _, c := range cases { + if p := PlanSeed(c.st); p != (SeedPlan{}) { + t.Errorf("%s: live data must not be touched, got %+v", c.name, p) + } + } +} + +func TestPlanSeedStaleJournalBehindFile(t *testing.T) { + st := ZoneDiskState{ + ZoneFile: true, ZoneSerial: 30, ZoneSerialOK: true, + Journal: true, JournalBegin: 10, JournalEnd: 16, JournalOK: true, + } + p := PlanSeed(st) + if p.WriteSeed || p.QuarantineZoneFile { + t.Fatalf("a file ahead of its journal is live data, got %+v", p) + } + if !p.QuarantineJournal { + t.Errorf("the unreplayable journal should be moved aside, got %+v", p) + } + if next := PlanSeed(applyPlan(st, p)); next != (SeedPlan{}) { + t.Errorf("second reconcile should be a no-op, got %+v", next) + } +} + +func TestPlanSeedFreshInstallIdempotent(t *testing.T) { + st := ZoneDiskState{} + p := PlanSeed(st) + after := applyPlan(st, p) + if next := PlanSeed(after); next != (SeedPlan{}) { + t.Fatalf("second reconcile of a fresh zone should be a no-op, got %+v", next) + } + if after.ZoneSerial != 1 { + t.Errorf("serial reset on second reconcile: %d", after.ZoneSerial) + } +} + +func TestPlanSeedSerialWrap(t *testing.T) { + st := ZoneDiskState{Journal: true, JournalBegin: 1 << 31, JournalEnd: 1<<32 - 1, JournalOK: true} + if got := PlanSeed(st).Serial; got != 1 { + t.Errorf("serial after wrap = %d want 1", got) + } +} + +func TestSeedZoneRoundTripsThroughParser(t *testing.T) { + content := renderSeedZone("200.18.198.in-addr.arpa", "198.18.200.8", 17) + got, ok := ParseZoneSerial(content) + if !ok || got != 17 { + t.Fatalf("seed zone serial = (%d,%v) want (17,true)", got, ok) + } +} + +func TestQuarantinePathsAreSuffixed(t *testing.T) { + path := ZoneFilePath("example.com") + if JournalPath(path) != path+".jnl" { + t.Fatalf("journal path = %q", JournalPath(path)) + } + cmd := moveAside(JournalPath(path), ".orphaned-16") + if !strings.Contains(cmd, "'"+path+".jnl.orphaned-16'") { + t.Errorf("quarantine command should rename, not delete: %s", cmd) + } + if strings.Contains(cmd, "rm ") { + t.Errorf("quarantine must never delete: %s", cmd) + } +} diff --git a/internal/controller/bindzone_controller.go b/internal/controller/bindzone_controller.go index b06a61b..44812c2 100644 --- a/internal/controller/bindzone_controller.go +++ b/internal/controller/bindzone_controller.go @@ -105,9 +105,22 @@ func (r *BindZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c if primaryIP == "" { return r.setPhase(ctx, &zone, "Pending", "PrimaryNoIP", "waiting for primary pod IP") } - if err := r.Exec.WriteSeedZone(ctx, zone.Namespace, primaryPod, zone.Spec.ZoneName, bind.ZoneFilePath(zone.Spec.ZoneName), primaryIP, 1); err != nil { + // The zone is absent from named's memory, but its database file and + // journal may still be on the PVC from a previous incarnation. + path := bind.ZoneFilePath(zone.Spec.ZoneName) + state, err := r.Exec.ZoneDiskState(ctx, zone.Namespace, primaryPod, path) + if err != nil { return r.setPhase(ctx, &zone, "Error", "SeedFailed", err.Error()) } + plan := bind.PlanSeed(state) + if err := r.Exec.Quarantine(ctx, zone.Namespace, primaryPod, path, plan); err != nil { + return r.setPhase(ctx, &zone, "Error", "SeedFailed", err.Error()) + } + if plan.WriteSeed { + if err := r.Exec.WriteSeedZone(ctx, zone.Namespace, primaryPod, zone.Spec.ZoneName, path, primaryIP, plan.Serial); err != nil { + return r.setPhase(ctx, &zone, "Error", "SeedFailed", err.Error()) + } + } } if err := r.Exec.AddZone(ctx, zone.Namespace, primaryPod, zone.Spec.ZoneName, zone.Spec.ViewRef, zoneConfig); err != nil { return r.setPhase(ctx, &zone, "Error", "AddZoneFailed", err.Error())