From 65086e314a2a442c8b503670217466825246ccaf Mon Sep 17 00:00:00 2001 From: adam Date: Sun, 18 Oct 2026 15:29:12 +0100 Subject: [PATCH] Linked zones: PTR records follow each view, deleting an override stays deleted Auto-PTR (and --ptr) changed PTR records in every linked view, whatever view the address record was in. ptrSync now compares each touched A/AAAA set before and after, per view, and changes only that view's reverse zone. PTR records in linked reverse zones are local (fill too); a shared PTR set is made local in each view before one view changes it. Deleting a linked zone's local records no longer brings the shared ones back into the view: the name and type are gone from that view and the other views keep theirs, local to each. Share still restores. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 15 ++ docs/cli.md | 3 +- docs/reverse.md | 21 +- docs/views.md | 4 +- internal/engine/expand.go | 455 ++++++++++++++++++-------------- internal/engine/linked.go | 46 +++- internal/engine/linked_test.go | 33 ++- internal/engine/reverse.go | 15 +- internal/engine/reverse_test.go | 99 +++++++ 9 files changed, 453 insertions(+), 238 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 121ed5c..6e06f5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,12 @@ All notable changes to Jarl are documented here. The format follows ### Changed +- Deleting a linked zone's local records (an override) removes that name + and type from that view only. It used to bring the shared records back + into the view, so a delete looked undone. The other views keep their + records, local to each now; **Share** one view's version (`jarl record + update ID --shared`) to have it everywhere again. + - A linked zone's Compare views tab starts with a view it is linked to, not the first other view. - The New token form only offers templates within what the token's user @@ -27,6 +33,15 @@ All notable changes to Jarl are documented here. The format follows ### Fixed +- Auto-PTR in linked zones changed PTR records in every linked view, not + just where the address record is. A record local to one view got PTR + records in the other views too; deleting or changing the address in + one view deleted or moved the PTR records of views that still had it. + PTR records now follow each view's own address records, are local to + their view in a linked reverse zone, and each zone's Reverse DNS + setting counts for its own view. **Fill** also adds local PTR records + now, not shared ones. The same applies to **Update PTR** (`--ptr`). + - Saving the settings of a zone whose primary name server is outside it (`ns1.adbraun.lol.` for `wg.adbraun.lol`) changed the name server to `ns1.adbraun.lol.wg.adbraun.lol.`. The form showed the name without its diff --git a/docs/cli.md b/docs/cli.md index 9e1e264..20936ad 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -318,7 +318,8 @@ host name (a bare `nas` is refused, since it would mean keeps the PTR record in the reverse zone in step (when that zone is managed by Jarl); `jarl record update ID --ptr` with nothing else adds the PTR record if it is missing, e.g. after creating the reverse zone. In linked zones, the -reverse zones of every linked view count. Every record type BIND supports can be used. Jarl knows the +reverse zone of each linked view that has the record counts +([reverse.md](reverse.md#keeping-ptr-records-in-step)). Every record type BIND supports can be used. Jarl knows the syntax of the common ones and checks them strictly, and the rest are validated by parsing. diff --git a/docs/reverse.md b/docs/reverse.md index af07430..2b85a99 100644 --- a/docs/reverse.md +++ b/docs/reverse.md @@ -147,9 +147,24 @@ jarl zone update adbraun.lol --view internal --auto-ptr # off: --auto-p (Zone settings → **Reverse DNS** in the web UI.) Then adding, changing, disabling, enabling or deleting an A or AAAA record in the zone does the -same to its PTR record, in the reverse zones of the zone's view and of the -views it is linked with ([views.md](views.md#linked-zones)). A whole set -replaced at once (`jarl record set`, a batch) works too. +same to its PTR record, in the reverse zone of each view that has the +record. A whole set replaced at once (`jarl record set`, a batch) works +too. + +**Linked zones** ([views.md](views.md#linked-zones)). PTR records follow +each view's own address records: + +* A shared record gets a PTR record in every linked view that has it; a + record local to one view gets one in that view only. +* An address that goes from one view (deleted, changed, or overridden by + a local record) loses its PTR record there only. Views that still have + the address keep theirs. +* Each zone's own **Reverse DNS** setting counts: a linked zone without + it doesn't get PTR records from its partner's changes. +* In a linked reverse zone, these PTR records (and those **Fill** adds) + are local to their view. A PTR record that is shared becomes local in + each view the first time one of them changes it, so that a change in + one view doesn't reach the others. * **One name per address.** When an address's PTR record names another host, it stays: a second A record for the same address doesn't add a diff --git a/docs/views.md b/docs/views.md index be305f9..90ce466 100644 --- a/docs/views.md +++ b/docs/views.md @@ -183,7 +183,9 @@ own and its own zone options. the web UI). A local record overrides the shared records of the same name and type in that view: `www A 10.0.10.5` local to internal, while the external view keeps the shared `www A 203.0.113.20`. Delete the local - records and the view gets the shared ones back. + records and the name and type are gone from that view only: the other + views keep their records, each local to its view now. To give the view + the shared version back instead, **Share** it from a view that has it. * A new record joins the set of the same name and type: local if that set is local in its view, shared otherwise. * **Sharing a version** (`jarl record update ID --shared`, **Share** in the diff --git a/internal/engine/expand.go b/internal/engine/expand.go index 9d8bcfd..115bfb4 100644 --- a/internal/engine/expand.go +++ b/internal/engine/expand.go @@ -32,6 +32,12 @@ type expander struct { // exposures classifies the views as public or private (nil in tests: // every view private). exposures func(*model.State) []validate.Exposure + // ptrForced are the address sets whose PTR records the request being + // expanded asked to update (ptrForce). + ptrForced map[rrKey]bool + // sharing are the local sets the request deletes so that the shared + // version reaches them (dropOverrides). + sharing map[rrKey]bool } func (x *expander) ids(n int) ([]int64, error) { @@ -136,13 +142,20 @@ func dateSerial(now time.Time) uint32 { } func (x *expander) expand(req Request) ([]model.Op, error) { + x.ptrForced, x.sharing = nil, nil ops, err := x.expandOne(req) if err != nil { return nil, err } switch req.Op { - case OpRecordCreate, OpRecordUpdate, OpRecordDelete, OpRRSetReplace, OpRecordDisable, OpRecordEnable, OpZoneFillPTR: - // Changes to linked zones reach the other zones of the link. + case OpRecordCreate, OpRecordUpdate, OpRecordDelete, OpRRSetReplace, OpRecordDisable, OpRecordEnable: + // Changes to linked zones reach the other zones of the link, then + // PTR records follow the addresses in each view. + if ops, err = x.linkSync(ops); err != nil { + return nil, err + } + return x.ptrSync(ops) + case OpZoneFillPTR: return x.linkSync(ops) } return ops, nil @@ -757,65 +770,227 @@ func addrOf(p *rr.Parsed) (netip.Addr, bool) { return netip.Addr{}, false } -// ptrOps returns operations creating the PTR record for an address record. -// -// auto is set when the zone's auto_ptr asked rather than the request: a -// missing reverse zone isn't worth a warning then (most addresses have -// none), and an address whose PTR record names another host keeps it. -func (x *expander) ptrCreate(z *model.Zone, owner string, p *rr.Parsed, auto bool) ([]model.Op, error) { - addr, ok := addrOf(p) - if !ok { - return nil, errf(KindBadRequest, "ptr only applies to A and AAAA records") +// PTR records follow address records per view (docs/reverse.md): after a +// request's operations (and those keeping linked zones in step), ptrSync +// compares each touched A or AAAA set before and after, in every view it +// is in, and changes the PTR records of that view's reverse zone to match. +// So a record local to one view of a linked zone changes PTR records in +// that view only, and an address that goes from one view keeps its PTR +// record in the views that still have it. + +// ptrForce marks a name and type whose PTR records the request asked to +// update (ptr: true), in its zone and the zones linked with it. +func (x *expander) ptrForce(z *model.Zone, name, typ string) { + if x.ptrForced == nil { + x.ptrForced = map[rrKey]bool{} } - var ops []model.Op - found := false - for _, vid := range ptrViews(x.st, z) { - rz, ptrName := reverseZoneFor(x.st, vid, addr) - if rz == nil { - continue - } - found = true - more, err := x.ptrCreateIn(rz, ptrName, owner, auto) - if err != nil { - return nil, err - } - ops = append(ops, more...) + x.ptrForced[rrKey{z.ID, name, typ}] = true + for _, o := range x.st.LinkedWith(z) { + x.ptrForced[rrKey{o.ID, name, typ}] = true } - if !found && !auto { - x.warnings = append(x.warnings, fmt.Sprintf("no reverse zone for %s in this view, so no PTR record was added (create one: jarl zone create %s/%d)", addr, addr, addr.BitLen())) - } - return ops, nil } -// ptrViews are the views whose reverse zones get the PTR records of zone -// z's address records: z's own, and, for a linked zone, the other views -// of the link (the record is in all of them). -func ptrViews(st *model.State, z *model.Zone) []int64 { - out := []int64{z.ViewID} - for _, o := range st.LinkedWith(z) { - if !slices.Contains(out, o.ViewID) { - out = append(out, o.ViewID) +// Whether a set has an address: none, only in disabled records, enabled. +const ( + addrNone = iota + addrDisabled + addrEnabled +) + +// addrStates returns the addresses of a zone's A or AAAA set. +func addrStates(st *model.State, k rrKey) map[netip.Addr]int { + out := map[netip.Addr]int{} + for _, r := range st.RRset(k.zone, k.name, k.typ) { + a, err := netip.ParseAddr(r.Data) + if err != nil { + continue } + s := addrDisabled + if !r.Disabled { + s = addrEnabled + } + out[a.Unmap()] = max(out[a.Unmap()], s) } return out } +// ptrSync returns ops followed by the PTR changes they call for: for the +// zone's auto_ptr, or because the request asked (ptrForce). +func (x *expander) ptrSync(ops []model.Op) ([]model.Op, error) { + forced := x.ptrForced + x.ptrForced = nil + work := x.st.Clone() + c := &applyCtx{p: x.p, now: x.now} + for _, op := range ops { + if err := applyOp(work, op, c); err != nil { + return nil, err + } + } + keys := affectedKeys(x.st, work, ops) + for _, k := range sortedForced(forced) { + if !slices.Contains(keys, k) { + keys = append(keys, k) + } + } + var extra []model.Op + for _, k := range keys { + z := work.Zones[k.zone] + if z == nil || (k.typ != "A" && k.typ != "AAAA") || strings.HasPrefix(k.name, "*.") { + continue + } + force := forced[k] + if !force && !z.AutoPTR { + continue + } + before, after := addrStates(x.st, k), addrStates(work, k) + var addrs []netip.Addr + for _, m := range []map[netip.Addr]int{before, after} { + for a := range m { + if !slices.Contains(addrs, a) { + addrs = append(addrs, a) + } + } + } + slices.SortFunc(addrs, func(a, b netip.Addr) int { return a.Compare(b) }) + for _, a := range addrs { + was, is := before[a], after[a] + if !force && (was == is || was == addrNone && is == addrDisabled) { + continue + } + rz, ptrName := reverseZoneFor(work, z.ViewID, a) + if rz == nil { + if force && is == addrEnabled { + x.warnings = append(x.warnings, fmt.Sprintf("no reverse zone for %s in %s, so no PTR record was added there (create one: jarl zone create %s/%d%s)", + a, viewOrServer(work, z.ViewID), a, a.BitLen(), viewFlag(work, z.ViewID))) + } + continue + } + more, err := x.ptrChange(work, rz, ptrName, k.name, is, !force) + if err != nil { + return nil, err + } + for _, op := range more { + if err := applyOp(work, op, c); err != nil { + return nil, err + } + } + extra = append(extra, more...) + } + } + if len(extra) == 0 { + return ops, nil + } + // The PTR records are local (see ptrChange), so this only keeps a + // linked reverse zone consistent. + extra, err := x.linkSync(extra) + if err != nil { + return nil, err + } + return append(ops, extra...), nil +} + +func sortedForced(m map[rrKey]bool) []rrKey { + var out []rrKey + for k := range m { + out = append(out, k) + } + slices.SortFunc(out, func(a, b rrKey) int { + if a.zone != b.zone { + return int(a.zone - b.zone) + } + if a.name != b.name { + return strings.Compare(a.name, b.name) + } + return strings.Compare(a.typ, b.typ) + }) + return out +} + +func viewFlag(st *model.State, viewID int64) string { + if v := st.Views[viewID]; v != nil { + return " --view " + v.Name + } + return "" +} + +// ptrChange returns the operations that give reverse zone rz's PTR +// records at ptrName for host owner the state of its address: none +// (deleted), disabled, or enabled (added when missing). +// +// In a linked reverse zone the PTR records are local to each view, since +// each view's follow its own address records: a shared set at ptrName +// is made local in every view of the link first, as each view has it. +func (x *expander) ptrChange(st *model.State, rz *model.Zone, ptrName, owner string, state int, auto bool) ([]model.Op, error) { + nwarn := len(x.warnings) + ops, err := x.ptrChangeIn(st, rz, ptrName, owner, state, auto) + if err != nil || len(ops) == 0 || rz.LinkGroup == 0 { + return ops, err + } + var local []model.Op + for _, m := range st.LinkMembers(rz.LinkGroup) { + for _, r := range st.RRset(m.ID, ptrName, "PTR") { + if r.SharedID != 0 { + local = append(local, newOp(OpRecordUpdate, r.ID, r.Version, RecordInput{SharedID: model.Ptr(int64(0))}, + fmt.Sprintf("linked: %s PTR %s becomes local to %s (PTR records follow each view's own address records)", r.Name, r.Data, zoneDesc(st, m)))) + } + } + } + if len(local) == 0 { + return ops, nil + } + // Again on the local records (their versions changed). + scratch := st.Clone() + c := &applyCtx{p: x.p, now: x.now} + for _, op := range local { + if err := applyOp(scratch, op, c); err != nil { + return nil, err + } + } + x.warnings = x.warnings[:nwarn] + ops, err = x.ptrChangeIn(scratch, rz, ptrName, owner, state, auto) + if err != nil { + return nil, err + } + return append(local, ops...), nil +} + +func (x *expander) ptrChangeIn(st *model.State, rz *model.Zone, ptrName, owner string, state int, auto bool) ([]model.Op, error) { + var ops []model.Op + switch state { + case addrNone: + for _, r := range st.RRset(rz.ID, ptrName, "PTR") { + if strings.EqualFold(r.Data, owner) { + ops = append(ops, newOp(OpRecordDelete, r.ID, r.Version, nil, fmt.Sprintf("delete %s PTR %s", r.Name, r.Data))) + } + } + case addrDisabled: + for _, r := range st.RRset(rz.ID, ptrName, "PTR") { + if strings.EqualFold(r.Data, owner) && !r.Disabled { + ops = append(ops, newOp(OpRecordUpdate, r.ID, r.Version, RecordInput{Disabled: model.Ptr(true)}, fmt.Sprintf("disable %s PTR %s", r.Name, r.Data))) + } + } + default: + return x.ptrCreateIn(st, rz, ptrName, owner, auto) + } + return ops, nil +} + // ptrCreateIn returns operations creating the PTR record ptrName -> owner // in reverse zone rz (none when it is there; enabling it when disabled). -func (x *expander) ptrCreateIn(rz *model.Zone, ptrName, owner string, auto bool) ([]model.Op, error) { +// auto is set when the zone's auto_ptr asked rather than the request: an +// address whose PTR record names another host keeps it then. +func (x *expander) ptrCreateIn(st *model.State, rz *model.Zone, ptrName, owner string, auto bool) ([]model.Op, error) { var other string - for _, r := range x.st.ZoneRecords(rz.ID) { - if r.Name == ptrName && r.Type == "PTR" { - if strings.EqualFold(r.Data, owner) { - if r.Disabled { - in := RecordInput{Disabled: model.Ptr(false)} - return []model.Op{newOp(OpRecordUpdate, r.ID, r.Version, in, fmt.Sprintf("enable %s PTR %s", r.Name, r.Data))}, nil - } - return nil, nil // already there - } - if !r.Disabled { - other = r.Data + for _, r := range st.RRset(rz.ID, ptrName, "PTR") { + if strings.EqualFold(r.Data, owner) { + if r.Disabled { + in := RecordInput{Disabled: model.Ptr(false)} + return []model.Op{newOp(OpRecordUpdate, r.ID, r.Version, in, fmt.Sprintf("enable %s PTR %s", r.Name, r.Data))}, nil } + return nil, nil // already there + } + if !r.Disabled { + other = r.Data } } if other != "" { @@ -830,38 +1005,12 @@ func (x *expander) ptrCreateIn(rz *model.Zone, ptrName, owner string, auto bool) return nil, err } in := RecordInput{ZoneID: rz.ID, Name: model.Ptr(ptrName), Type: model.Ptr("PTR"), Data: model.Ptr(owner)} + if rz.LinkGroup != 0 { + in.SharedID = model.Ptr(int64(0)) + } return []model.Op{newOp(OpRecordCreate, ids[0], 0, in, fmt.Sprintf("add %s IN PTR %s", ptrName, owner))}, nil } -// ptrSetDisabled returns operations disabling or enabling the PTR record -// of an address record. -func (x *expander) ptrSetDisabled(z *model.Zone, owner string, p *rr.Parsed, disabled bool) []model.Op { - addr, ok := addrOf(p) - if !ok { - return nil - } - var ops []model.Op - for _, vid := range ptrViews(x.st, z) { - rz, ptrName := reverseZoneFor(x.st, vid, addr) - if rz == nil { - continue - } - ops = append(ops, x.ptrSetDisabledIn(rz, ptrName, owner, disabled)...) - } - return ops -} - -func (x *expander) ptrSetDisabledIn(rz *model.Zone, ptrName, owner string, disabled bool) []model.Op { - var ops []model.Op - for _, r := range x.st.ZoneRecords(rz.ID) { - if r.Name == ptrName && r.Type == "PTR" && strings.EqualFold(r.Data, owner) && r.Disabled != disabled { - in := RecordInput{Disabled: model.Ptr(disabled)} - ops = append(ops, newOp(OpRecordUpdate, r.ID, r.Version, in, fmt.Sprintf("%s %s PTR %s", enableWord(disabled), r.Name, r.Data))) - } - } - return ops -} - func enableWord(disabled bool) string { if disabled { return "disable" @@ -869,23 +1018,6 @@ func enableWord(disabled bool) string { return "enable" } -func (x *expander) ptrDelete(z *model.Zone, owner string, p *rr.Parsed) []model.Op { - addr, ok := addrOf(p) - if !ok { - return nil - } - var ops []model.Op - for _, vid := range ptrViews(x.st, z) { - rz, ptrName := reverseZoneFor(x.st, vid, addr) - if rz == nil { - continue - } - in := RecordInput{ZoneID: rz.ID, Name: model.Ptr(ptrName), Type: model.Ptr("PTR"), Data: model.Ptr(owner), MissingOK: true} - ops = append(ops, newOp(OpRecordDelete, 0, 0, in, fmt.Sprintf("delete %s IN PTR %s (if present)", ptrName, owner))) - } - return ops -} - // parseWarnings keeps what parsing noticed (e.g. a name read as absolute). // The stored operation holds the canonical data, so applying it doesn't // notice again. @@ -912,7 +1044,12 @@ func (x *expander) recordCreate(req Request) ([]model.Op, error) { return nil, err } x.parseWarnings(owner, p) - ptr, auto := in.PTR, !in.PTR && autoPTR(z, owner, p) + if in.PTR { + if _, ok := addrOf(p); !ok { + return nil, errf(KindBadRequest, "ptr only applies to A and AAAA records") + } + x.ptrForce(z, owner, p.Type) + } in.ZoneID, in.Zone, in.View, in.PTR = z.ID, "", "", false in.Name, in.Type, in.Data = &owner, &p.Type, &p.Data ids, err := x.ids(1) @@ -940,15 +1077,7 @@ func (x *expander) recordCreate(req Request) ([]model.Op, error) { } // local on a zone that isn't linked: its records are local anyway. in.Local = nil - ops := []model.Op{newOp(OpRecordCreate, ids[0], 0, in, summary)} - if ptr || auto && (in.Disabled == nil || !*in.Disabled) { - more, err := x.ptrCreate(z, owner, p, auto) - if err != nil { - return nil, err - } - ops = append(ops, more...) - } - return ops, nil + return []model.Op{newOp(OpRecordCreate, ids[0], 0, in, summary)}, nil } func (x *expander) recordUpdate(req Request) ([]model.Op, error) { @@ -1069,67 +1198,22 @@ func (x *expander) recordUpdate(req Request) ([]model.Op, error) { if string(mustJSON(in)) != "{}" || !ptr { ops = append(ops, newOp(OpRecordUpdate, r.ID, base, in, summary)) } - disabled := r.Disabled - if in.Disabled != nil { - disabled = *in.Disabled - } - moved := p.Data != r.Data || owner != r.Name || p.Type != r.Type - if !ptr && z.AutoPTR { - // The zone keeps PTR records in step (docs/reverse.md): as if - // asked, for whichever of the old and new record is an address. - old, err := rr.Parse(z.Name, r.Name, r.Type, r.Data) - oldAuto := err == nil && autoPTR(z, r.Name, old) - newAuto := autoPTR(z, owner, p) - switch { - case moved: - if oldAuto { - ops = append(ops, x.ptrDelete(z, r.Name, old)...) - } - if newAuto && !disabled { - more, err := x.ptrCreate(z, owner, p, true) - if err != nil { - return nil, err - } - ops = append(ops, more...) - } - case toggled && newAuto && disabled: - ops = append(ops, x.ptrSetDisabled(z, owner, p, true)...) - case toggled && newAuto: - // Enables the PTR record, or adds it when it is missing. - more, err := x.ptrCreate(z, owner, p, true) - if err != nil { - return nil, err - } - ops = append(ops, more...) - } - return ops, nil - } - if ptr && moved { + if ptr { + // The PTR records of the old and the new record, whichever is an + // address (ptrSync). + _, isAddr := addrOf(p) if old, err := rr.Parse(z.Name, r.Name, r.Type, r.Data); err == nil { - ops = append(ops, x.ptrDelete(z, r.Name, old)...) - } - if !disabled { - more, err := x.ptrCreate(z, owner, p, false) - if err != nil { - return nil, err + if _, ok := addrOf(old); ok { + isAddr = true + x.ptrForce(z, r.Name, r.Type) } - ops = append(ops, more...) } - } - if ptr && !moved && toggled { - if _, ok := addrOf(p); !ok { + if !isAddr { return nil, errf(KindBadRequest, "ptr only applies to A and AAAA records") } - ops = append(ops, x.ptrSetDisabled(z, owner, p, disabled)...) - } - if ptr && !moved && !toggled && !disabled { - // Nothing else changes: make sure the PTR record is there (the - // reverse zone may be newer than the record). - more, err := x.ptrCreate(z, owner, p, false) - if err != nil { - return nil, err + if _, ok := addrOf(p); ok { + x.ptrForce(z, owner, p.Type) } - ops = append(ops, more...) } return ops, nil } @@ -1152,9 +1236,8 @@ func (x *expander) recordDelete(req Request) ([]model.Op, error) { base = r.Version } ops := []model.Op{newOp(OpRecordDelete, r.ID, base, nil, fmt.Sprintf("delete %s %s %s", r.Name, r.Type, r.Data))} - z := x.st.Zones[r.ZoneID] - if p, err := rr.Parse(z.Name, r.Name, r.Type, r.Data); err == nil && (in.PTR || autoPTR(z, r.Name, p)) { - ops = append(ops, x.ptrDelete(z, r.Name, p)...) + if in.PTR { + x.ptrForce(x.st.Zones[r.ZoneID], r.Name, r.Type) } return ops, nil } @@ -1190,25 +1273,12 @@ func (x *expander) recordDelete(req Request) ([]model.Op, error) { if in.MissingOK { what += " (if present)" } - ptr := in.PTR + if in.PTR && parsed != nil { + x.ptrForce(z, owner, typ) + } in.ZoneID, in.Zone, in.View, in.PTR = z.ID, "", "", false in.Name, in.Type = &owner, &typ - ops := []model.Op{newOp(OpRecordDelete, 0, 0, in, "delete "+what)} - switch { - case ptr && parsed != nil: - ops = append(ops, x.ptrDelete(z, owner, parsed)...) - case z.AutoPTR: - // Every matching record's PTR record goes too. - for _, r := range x.st.RRset(z.ID, owner, typ) { - if parsed != nil && r.Data != parsed.Data { - continue - } - if p, err := rr.Parse(z.Name, r.Name, r.Type, r.Data); err == nil && autoPTR(z, r.Name, p) { - ops = append(ops, x.ptrDelete(z, r.Name, p)...) - } - } - } - return ops, nil + return []model.Op{newOp(OpRecordDelete, 0, 0, in, "delete "+what)}, nil } // recordSetDisabled expands record.disable and record.enable: the record @@ -1342,32 +1412,7 @@ func (x *expander) rrsetReplace(req Request) ([]model.Op, error) { if len(canon) == 0 { summary = fmt.Sprintf("delete all %s records of %s%s", typ, owner, where) } - ops := append(pre, newOp(OpRRSetReplace, 0, 0, in, summary)) - if z.AutoPTR && (typ == "A" || typ == "AAAA") && !strings.HasPrefix(owner, "*.") { - // PTR records follow the addresses that go and come. - old := map[string]bool{} - for _, r := range x.st.RRset(z.ID, owner, typ) { - old[r.Data] = true - if !slices.Contains(canon, r.Data) { - if p, err := rr.Parse(z.Name, r.Name, r.Type, r.Data); err == nil { - ops = append(ops, x.ptrDelete(z, r.Name, p)...) - } - } - } - for _, d := range canon { - if old[d] { - continue - } - if _, p, err := parseRecord(z, owner, typ, d); err == nil { - more, err := x.ptrCreate(z, owner, p, true) - if err != nil { - return nil, err - } - ops = append(ops, more...) - } - } - } - return ops, nil + return append(pre, newOp(OpRRSetReplace, 0, 0, in, summary)), nil } func (x *expander) viewCreate(req Request) ([]model.Op, error) { diff --git a/internal/engine/linked.go b/internal/engine/linked.go index 4f99c2c..a9d8a5c 100644 --- a/internal/engine/linked.go +++ b/internal/engine/linked.go @@ -106,8 +106,8 @@ func affectedKeys(pre, post *model.State, ops []model.Op) []rrKey { // // - z overrides it (local records): z's shared copies go; the others // keep theirs. -// - z's override was just removed: z gets the shared records back from -// a zone that has them. +// - z's override was just removed: z goes without the name and type, +// and the other zones' shared copies become local to each. // - otherwise z's shared records are the truth: every zone that doesn't // override the name and type gets the same (created, updated or // deleted to match). @@ -123,24 +123,40 @@ func (x *expander) syncKey(st *model.State, z *model.Zone, name, typ string) ([] } truth := st.SharedRecords(z.ID, name, typ) members := st.LinkedWith(z) + if len(truth) == 0 && x.st.HasLocal(z.ID, name, typ) && x.sharing[rrKey{z.ID, name, typ}] { + // Another view's version was shared (dropOverrides): z gets it. + for _, o := range members { + if src := st.SharedRecords(o.ID, name, typ); len(src) > 0 && !st.HasLocal(o.ID, name, typ) { + for _, r := range src { + op, err := x.copyRecord(st, z, r, "linked: "+name+" "+typ+" is shared again in "+where(z)+": add "+r.Data) + if err != nil { + return nil, err + } + ops = append(ops, op) + } + break + } + } + return ops, nil + } if len(truth) == 0 && x.st.HasLocal(z.ID, name, typ) { - // The override was removed: back to the shared records. + // z's local records are gone, and with them the name and type from + // z: the other views keep theirs, each its own now (a shared set + // would be in z too). Share sets one version for all again. for _, o := range members { if st.HasLocal(o.ID, name, typ) { continue } - src := st.SharedRecords(o.ID, name, typ) - if len(src) == 0 { - continue + var more []model.Op + for _, r := range st.SharedRecords(o.ID, name, typ) { + more = append(more, newOp(OpRecordUpdate, r.ID, r.Version, RecordInput{SharedID: model.Ptr(int64(0))}, + fmt.Sprintf("linked: %s %s %s stays in %s, local to it now (%s has none)", name, typ, r.Data, where(o), where(z)))) } - for _, r := range src { - op, err := x.copyRecord(st, z, r, "linked: "+name+" "+typ+" is shared again in "+where(z)+": add "+r.Data) - if err != nil { - return nil, err - } - ops = append(ops, op) + if len(more) > 0 && !x.p.Can(recordAction(st, o, name, typ, true)) { + return nil, errf(KindForbidden, "deleting %s %s from %s makes it local in %s, which you may not change; ask someone who may to delete it, or share %s's version (jarl record update ID --shared)", + dnsname.Display(name), typ, where(z), where(o), where(o)) } - break + ops = append(ops, more...) } return ops, nil } @@ -468,6 +484,10 @@ func (x *expander) dropOverrides(z *model.Zone, name, typ string) []model.Op { for _, o := range x.st.LinkedWith(z) { for _, r := range x.st.RRset(o.ID, name, typ) { if r.SharedID == 0 { + if x.sharing == nil { + x.sharing = map[rrKey]bool{} + } + x.sharing[rrKey{o.ID, name, typ}] = true ops = append(ops, newOp(OpRecordDelete, r.ID, r.Version, nil, fmt.Sprintf("linked: %s %s %s in %s is replaced by the shared version", r.Name, r.Type, r.Data, zoneDesc(x.st, o)))) } } diff --git a/internal/engine/linked_test.go b/internal/engine/linked_test.go index 0f059ee..b27fbd3 100644 --- a/internal/engine/linked_test.go +++ b/internal/engine/linked_test.go @@ -130,10 +130,15 @@ func TestLinkedZones(t *testing.T) { if !slices.Contains(env.view("external"), "www A 192.0.2.11 S") || slices.Contains(env.view("internal"), "www A 192.0.2.11 S") { t.Errorf("shared set with an override elsewhere: internal %q external %q", env.view("internal"), env.view("external")) } - // Removing the override brings the shared records back. + // Removing the override removes www A from internal only: external + // keeps its record, local now. Sharing it brings it back to internal. env.submit(env.admin, NewRequest(OpRRSetReplace, 0, RRSetInput{Zone: "example.com", View: "internal", Name: "www", Type: "A", Data: []string{}})) - if !slices.Contains(env.view("internal"), "www A 192.0.2.11 S") { - t.Errorf("override removed: internal %q", env.view("internal")) + if slices.ContainsFunc(env.view("internal"), func(s string) bool { return strings.HasPrefix(s, "www A ") }) || !slices.Contains(env.view("external"), "www A 192.0.2.11 L") { + t.Errorf("override removed: internal %q external %q", env.view("internal"), env.view("external")) + } + env.submit(env.admin, NewRequest(OpRecordUpdate, env.recordID("external", "www", "A"), RecordInput{Local: model.Ptr(false)})) + if !slices.Contains(env.view("internal"), "www A 192.0.2.11 S") || !slices.Contains(env.view("external"), "www A 192.0.2.11 S") { + t.Errorf("shared again: internal %q external %q", env.view("internal"), env.view("external")) } // Make a shared set local, then share another view's version. @@ -386,3 +391,25 @@ func TestCompareReport(t *testing.T) { t.Errorf("mail A: %+v", rep) } } + +// A shared record made local in one view, then deleted there, is gone from +// that view only; the other view keeps it, local now. +func TestDeleteLocalOverride(t *testing.T) { + env := linkedEnv(t) + env.submit(env.admin, NewRequest(OpZoneCreate, 0, ZoneInput{Name: model.Ptr("example.com"), View: model.Ptr("external"), LinkWith: "internal"})) + env.submit(env.admin, recIn("internal", "a", "A", "192.0.2.50")) + if !slices.Contains(env.view("external"), "a A 192.0.2.50 S") { + t.Fatalf("not shared: %q", env.view("external")) + } + env.submit(env.admin, NewRequest(OpRecordUpdate, env.recordID("internal", "a", "A"), RecordInput{Local: model.Ptr(true)})) + env.submit(env.admin, NewRequest(OpRecordDelete, env.recordID("internal", "a", "A"), nil)) + if slices.ContainsFunc(env.view("internal"), func(s string) bool { return strings.HasPrefix(s, "a A ") }) { + t.Errorf("internal still has a: %q", env.view("internal")) + } + if !slices.Contains(env.view("external"), "a A 192.0.2.50 L") { + t.Errorf("external: %q", env.view("external")) + } + if probs := env.e.Validate(env.state()); hasCode(probs, "linked_out_of_sync") || hasCode(probs, "linked_mixed") { + t.Errorf("validation: %+v", probs) + } +} diff --git a/internal/engine/reverse.go b/internal/engine/reverse.go index 506bd8f..778fc94 100644 --- a/internal/engine/reverse.go +++ b/internal/engine/reverse.go @@ -8,7 +8,6 @@ import ( "jarl/internal/dnsname" "jarl/internal/model" - "jarl/internal/rr" "jarl/internal/validate" ) @@ -227,9 +226,11 @@ func (x *expander) zoneFillPTR(req Request) ([]model.Op, error) { } var ops []model.Op for i, f := range fills { + // Local to the view in a linked zone, like the PTR records that + // follow address records (ptrChange). ri := RecordInput{ZoneID: z.ID, Name: model.Ptr(f.Name), Type: model.Ptr("PTR"), Data: model.Ptr(f.Target)} if z.LinkGroup != 0 { - ri.SharedID = model.Ptr(ids[i]) + ri.SharedID = model.Ptr(int64(0)) } ops = append(ops, newOp(OpRecordCreate, ids[i], 0, ri, fmt.Sprintf("add %s IN PTR %s", f.Name, f.Target))) } @@ -531,16 +532,6 @@ func (x *expander) reverseDismiss(req Request, dismiss bool) ([]model.Op, error) var cgnat = netip.MustParsePrefix("100.64.0.0/10") -// autoPTR reports whether a record's PTR is kept in step by its zone's -// auto_ptr setting. -func autoPTR(z *model.Zone, owner string, p *rr.Parsed) bool { - if z == nil || !z.AutoPTR || strings.HasPrefix(owner, "*.") { - return false - } - _, ok := addrOf(p) - return ok -} - func viewName(st *model.State, id int64) string { if v := st.Views[id]; v != nil { return v.Name diff --git a/internal/engine/reverse_test.go b/internal/engine/reverse_test.go index 237cca0..eab3fc4 100644 --- a/internal/engine/reverse_test.go +++ b/internal/engine/reverse_test.go @@ -303,3 +303,102 @@ func TestReverseFromOtherView(t *testing.T) { t.Errorf("warnings %q", res.Warnings) } } + +// ptrsMarked is ptrs with S (shared) or L (local) after each record. +func (env *testEnv) ptrsMarked(view, zone string) []string { + env.t.Helper() + s := env.state() + z := s.FindZone(zone, s.ViewByName(view).ID) + var out []string + for _, r := range s.ZoneRecords(z.ID) { + if r.Type == "PTR" { + mark := " L" + if r.SharedID != 0 { + mark = " S" + } + out = append(out, strings.TrimSuffix(r.Name, ".10.0.10.in-addr.arpa.")+" "+strings.TrimSuffix(r.Data, ".example.com.")+mark) + } + } + return out +} + +// Auto-PTR with linked forward and reverse zones: PTR records follow each +// view's own address records, and are local to their view. +func TestAutoPTRLinked(t *testing.T) { + env := linkedEnv(t) + env.submit(env.admin, NewRequest(OpZoneCreate, 0, ZoneInput{Name: model.Ptr("example.com"), View: model.Ptr("external"), LinkWith: "internal"})) + env.submit(env.admin, NewRequest(OpZoneCreate, 0, ZoneInput{Name: model.Ptr("10.0.10.0/24"), View: model.Ptr("internal")})) + env.submit(env.admin, NewRequest(OpZoneCreate, 0, ZoneInput{Name: model.Ptr("10.0.10.0/24"), View: model.Ptr("external"), LinkWith: "internal"})) + st := env.state() + for _, v := range []string{"internal", "external"} { + env.submit(env.admin, NewRequest(OpZoneUpdate, st.FindZone("example.com.", st.ViewByName(v).ID).ID, ZoneInput{AutoPTR: model.Ptr(true)})) + } + rz := "10.0.10.in-addr.arpa." + local := func(view, name, data string) Request { + r := recIn(view, name, "A", data) + var in RecordInput + _ = decode(r.Data, &in) + in.Local = model.Ptr(true) + return NewRequest(OpRecordCreate, 0, in) + } + + // A local record: a local PTR record in its own view only. + env.submit(env.admin, local("internal", "a", "10.0.10.1")) + want(t, "local a internal", env.ptrsMarked("internal", rz), "1 a L") + want(t, "local a external", env.ptrsMarked("external", rz)) + + // The other view adds the same name (shared, as it has no local + // one): its own PTR record; deleting it in one view leaves the other. + env.submit(env.admin, recIn("external", "a", "A", "10.0.10.1")) + want(t, "both internal", env.ptrsMarked("internal", rz), "1 a L") + want(t, "both external", env.ptrsMarked("external", rz), "1 a L") + env.submit(env.admin, NewRequest(OpRecordDelete, env.recordID("external", "a", "A"), nil)) + want(t, "deleted in external: internal", env.ptrsMarked("internal", rz), "1 a L") + want(t, "deleted in external: external", env.ptrsMarked("external", rz)) + + // Both local; one changes its address, the other keeps its PTR. + env.submit(env.admin, local("external", "a", "10.0.10.1")) + env.submit(env.admin, NewRequest(OpRecordUpdate, env.recordID("internal", "a", "A"), RecordInput{Data: model.Ptr("10.0.10.2")})) + want(t, "moved internal", env.ptrsMarked("internal", rz), "2 a L") + want(t, "moved external", env.ptrsMarked("external", rz), "1 a L") + // Disabling in one view disables there only. + env.submit(env.admin, NewRequest(OpRecordUpdate, env.recordID("external", "a", "A"), RecordInput{Disabled: model.Ptr(true)})) + want(t, "disabled external", env.ptrs("external", rz), "1.10.0.10.in-addr.arpa. a.example.com. disabled") + want(t, "disabled: internal", env.ptrsMarked("internal", rz), "2 a L") + env.submit(env.admin, NewRequest(OpRecordDelete, env.recordID("external", "a", "A"), nil)) + env.submit(env.admin, NewRequest(OpRecordDelete, env.recordID("internal", "a", "A"), nil)) + want(t, "gone internal", env.ptrsMarked("internal", rz)) + want(t, "gone external", env.ptrsMarked("external", rz)) + + // A shared record: a local PTR in each view; deleting it removes both. + env.submit(env.admin, recIn("internal", "b", "A", "10.0.10.3")) + want(t, "shared internal", env.ptrsMarked("internal", rz), "3 b L") + want(t, "shared external", env.ptrsMarked("external", rz), "3 b L") + // Overridden in one view with another address: that view's PTR moves. + env.submit(env.admin, NewRequest(OpRecordUpdate, env.recordID("external", "b", "A"), RecordInput{Local: model.Ptr(true), Data: model.Ptr("10.0.10.4")})) + want(t, "override internal", env.ptrsMarked("internal", rz), "3 b L") + want(t, "override external", env.ptrsMarked("external", rz), "4 b L") + // Sharing internal's version: external gets it back, and its PTR. + env.submit(env.admin, NewRequest(OpRecordUpdate, env.recordID("internal", "b", "A"), RecordInput{Local: model.Ptr(false)})) + want(t, "shared again internal", env.ptrsMarked("internal", rz), "3 b L") + want(t, "shared again external", env.ptrsMarked("external", rz), "3 b L") + // The override deleted: external goes without b (and its PTR); + // internal keeps it, local now. + env.submit(env.admin, NewRequest(OpRecordUpdate, env.recordID("external", "b", "A"), RecordInput{Local: model.Ptr(true), Data: model.Ptr("10.0.10.4")})) + env.submit(env.admin, NewRequest(OpRecordDelete, env.recordID("external", "b", "A"), nil)) + want(t, "override gone internal", env.ptrsMarked("internal", rz), "3 b L") + want(t, "override gone external", env.ptrsMarked("external", rz)) + env.submit(env.admin, NewRequest(OpRecordDelete, env.recordID("internal", "b", "A"), nil)) + want(t, "b gone internal", env.ptrsMarked("internal", rz)) + + // A shared PTR record from before (shared by hand) becomes local in + // each view when one view's address goes. + env.submit(env.admin, recIn("internal", "c", "A", "10.0.10.5")) + env.submit(env.admin, NewRequest(OpRecordDelete, 0, RecordInput{Zone: rz, View: "internal", Name: model.Ptr("5"), Type: model.Ptr("PTR")})) + env.submit(env.admin, NewRequest(OpRecordDelete, 0, RecordInput{Zone: rz, View: "external", Name: model.Ptr("5"), Type: model.Ptr("PTR")})) + env.submit(env.admin, NewRequest(OpRecordCreate, 0, RecordInput{Zone: rz, View: "internal", Name: model.Ptr("5"), Type: model.Ptr("PTR"), Data: model.Ptr("c.example.com."), Local: model.Ptr(false)})) + want(t, "shared ptr external", env.ptrsMarked("external", rz), "5 c S") + env.submit(env.admin, NewRequest(OpRecordUpdate, env.recordID("external", "c", "A"), RecordInput{Local: model.Ptr(true), Data: model.Ptr("10.0.10.6")})) + want(t, "unshared internal", env.ptrsMarked("internal", rz), "5 c L") + want(t, "unshared external", env.ptrsMarked("external", rz), "6 c L") +}