From 53d3cd7ce9ec687f93dfc739295a0dab2c3e0606 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 5 Oct 2026 21:57:04 +0200 Subject: [PATCH] Delete node-dns-resolver and make resolver config need the uplink Nothing has claimed node-dns-resolver since the mesh moved to one resolver (hq ADR 0194); seeding never removes a row, so a migration deletes it. resolv.conf stays the mesh's only while the network manager is told to keep off it, which the node-uplink holder does (ADR 0117). A seat's Needs makes that a dependency checked at assignment by the ADR 0207 mechanism (hq ADR 0220). The two-claimants test keeps its intent with a synthetic module now that resolved-split-dns leaves the catalogue. --- internal/broker/seattools_test.go | 16 +-- internal/catalogue/resolver_manifests_test.go | 22 ++-- .../catalogue/resolver_needs_uplink_test.go | 121 ++++++++++++++++++ internal/catalogue/seat_dependencies.go | 22 +++- internal/catalogue/seats.go | 24 +++- internal/catalogue/seats_test.go | 12 +- ...060-the-per-node-resolver-seat-is-gone.sql | 17 +++ 7 files changed, 202 insertions(+), 32 deletions(-) create mode 100644 internal/catalogue/resolver_needs_uplink_test.go create mode 100644 internal/inventory/migrations/0060-the-per-node-resolver-seat-is-gone.sql diff --git a/internal/broker/seattools_test.go b/internal/broker/seattools_test.go index 3daff1a..ed24e1f 100644 --- a/internal/broker/seattools_test.go +++ b/internal/broker/seattools_test.go @@ -5,16 +5,16 @@ import "testing" // A node-scoped seat's tool carries the node (novox/hq ADR 0132, design 33 §4): two nodes holding one // node-scoped seat derive two addresses, and a user of the seat may publish any node's. func TestTwoNodesHoldingOneNodeSeatDeriveTwoToolAddresses(t *testing.T) { - seat := Seat{Name: "node-dns-resolver", Scope: "node", Serves: []string{"lookup"}} - one, _ := PermissionsFor(Principal{Kind: KindModule, Node: "one", Module: "dnsmasq", Holds: []Seat{seat}, PasswordHash: "x"}) - two, _ := PermissionsFor(Principal{Kind: KindModule, Node: "two", Module: "dnsmasq", Holds: []Seat{seat}, PasswordHash: "x"}) - has(t, one.Subscribe, "mesh.seat.node-dns-resolver.tool.lookup.one") - has(t, two.Subscribe, "mesh.seat.node-dns-resolver.tool.lookup.two") - hasNot(t, one.Subscribe, "mesh.seat.node-dns-resolver.tool.lookup") - hasNot(t, one.Subscribe, "mesh.seat.node-dns-resolver.tool.lookup.two") + seat := Seat{Name: "node-hosts-file", Scope: "node", Serves: []string{"entries"}} + one, _ := PermissionsFor(Principal{Kind: KindModule, Node: "one", Module: "hosts", Holds: []Seat{seat}, PasswordHash: "x"}) + two, _ := PermissionsFor(Principal{Kind: KindModule, Node: "two", Module: "hosts", Holds: []Seat{seat}, PasswordHash: "x"}) + has(t, one.Subscribe, "mesh.seat.node-hosts-file.tool.entries.one") + has(t, two.Subscribe, "mesh.seat.node-hosts-file.tool.entries.two") + hasNot(t, one.Subscribe, "mesh.seat.node-hosts-file.tool.entries") + hasNot(t, one.Subscribe, "mesh.seat.node-hosts-file.tool.entries.two") user, _ := PermissionsFor(Principal{Kind: KindModule, Node: "three", Module: "asker", Uses: []Seat{seat}, PasswordHash: "x"}) - has(t, user.Publish, "mesh.seat.node-dns-resolver.tool.lookup.*") + has(t, user.Publish, "mesh.seat.node-hosts-file.tool.entries.*") } // A mesh-scoped seat's tool stays flat: nothing about it changes. diff --git a/internal/catalogue/resolver_manifests_test.go b/internal/catalogue/resolver_manifests_test.go index d47431c..350bf8f 100644 --- a/internal/catalogue/resolver_manifests_test.go +++ b/internal/catalogue/resolver_manifests_test.go @@ -15,7 +15,7 @@ import ( // same arrangement, and to the two things a resolver here must never do — read resolv.conf for // its upstreams, or take an address systemd-resolved holds. -// resolverShelf is the three resolver modules and the container runtime beside something that +// resolverShelf is the two resolver modules and the container runtime beside something that // answers `mesh-addressing`. // The networking module that really does is composed in the controller and cannot be imported // here, so a stand-in offers the same word; what is under test is the manifests, not the network. @@ -24,7 +24,7 @@ func resolverShelf(t *testing.T) map[string]Manifest { shelf := map[string]Manifest{ "net": {Module: "net", Version: "1", Provides: []Offer{{Name: "mesh-addressing"}}}, } - for _, name := range []string{"dnsmasq", "resolv-conf", "resolved-split-dns", "docker"} { + for _, name := range []string{"dnsmasq", "resolv-conf", "docker"} { shelf[name] = catalogueManifest(t, name) } return shelf @@ -104,13 +104,6 @@ func TestTheResolverForwardsToFixedUpstreamsAndNeverReadsResolvConf(t *testing.T if !strings.Contains(resolv, "\noptions timeout:1 attempts:1") { t.Errorf("the fallback is not reached after one short attempt:\n%s", resolv) } - // The split-DNS alternative points at the same resolver, or a machine that keeps - // systemd-resolved in charge would route the mesh's suffix to nothing. - for _, r := range catalogueManifest(t, "resolved-split-dns").Resources { - if content, _ := r["content"].(string); content != "" && !strings.Contains(content, "DNS=${bound:wildcard-resolution:address}\n") { - t.Errorf("resolved-split-dns does not point at the resolver's address:\n%s", content) - } - } } // The resolver and what points the machine at it compose on one machine, and what arrives is the @@ -220,11 +213,18 @@ func TestTheResolverAndWhatAsksItComposeOnOneMachine(t *testing.T) { // Two modules deciding what a machine asks are refused on one machine, as before — the claim // exists so they never take turns overwriting each other. +// +// **The second is made up.** The catalogue's only other claimant, a systemd-resolved split-DNS +// module, was retired with the choice against a stub on every node (novox/hq ADR 0196, ADR 0220); +// what is under test is the claim, so any second module claiming it will do. func TestTwoThingsDecidingWhatAMachineAsksAreRefused(t *testing.T) { - _, err := Resolve(resolverShelf(t), []string{"dnsmasq", "resolv-conf", "resolved-split-dns"}, + shelf := resolverShelf(t) + shelf["other-resolver-config"] = Manifest{Module: "other-resolver-config", Version: "1", + Claims: []Claim{{Name: "node-resolver-config", Scope: ScopeNode}}} + _, err := Resolve(shelf, []string{"dnsmasq", "resolv-conf", "other-resolver-config"}, Node{Name: "anchor", At: "anchor.internal"}, World{}) if err == nil { - t.Fatal("resolv-conf and resolved-split-dns were both assigned to one machine") + t.Fatal("resolv-conf and a second module deciding what the machine asks were both assigned to one machine") } if !strings.Contains(err.Error(), "node-resolver-config") { t.Fatalf("the refusal does not say what was claimed: %v", err) diff --git a/internal/catalogue/resolver_needs_uplink_test.go b/internal/catalogue/resolver_needs_uplink_test.go new file mode 100644 index 0000000..903b8e2 --- /dev/null +++ b/internal/catalogue/resolver_needs_uplink_test.go @@ -0,0 +1,121 @@ +package catalogue + +import ( + "errors" + "reflect" + "strings" + "testing" +) + +// Defends novox/hq ADR 0220: what a machine asks for names needs the uplink held beside it. +// +// resolv.conf is the mesh's only while the program managing the machine's network is told to leave +// it alone, and the uplink's holder is what tells it (ADR 0117). Without one, the first connectivity +// change rewrites the file — so the dependency is checked at assignment, by the same mechanism as a +// service's on the service manager (ADR 0207), derived from the claim and never stated in a manifest. + +// resolverAndUplinks is a resolver-config holder and two uplink holders, with no resources of their +// own so that nothing but the seats is judged. +func resolverAndUplinks() map[string]Manifest { + return shelf( + mod("resolv-conf", nil, nil, nil, Claim{Name: "node-resolver-config"}), + mod("networkmanager", nil, nil, nil, Claim{Name: "node-uplink"}), + mod("systemd-networkd", nil, nil, nil, Claim{Name: "node-uplink"}), + ) +} + +func TestTheResolverConfigSeatNeedsTheUplink(t *testing.T) { + s, known := SeatNamed("node-resolver-config") + if !known { + t.Fatal("node-resolver-config is not in the mesh's set") + } + if !reflect.DeepEqual(s.Needs, []string{"node-uplink"}) { + t.Errorf("node-resolver-config needs %v, want [node-uplink] (ADR 0220)", s.Needs) + } + // Derived from the claim: a module claiming the seat depends on the uplink with nothing written. + got := DependsOn(mod("anything", nil, nil, nil, Claim{Name: "node-resolver-config"})) + if !reflect.DeepEqual(got, []string{"node-uplink"}) { + t.Errorf("a module claiming node-resolver-config depends on %v, want [node-uplink]", got) + } + // And the uplink's holders need nothing of the kind: the dependency runs one way. + if got := DependsOn(mod("networkmanager", nil, nil, nil, Claim{Name: "node-uplink"})); len(got) != 0 { + t.Errorf("an uplink holder depends on %v; it needs no seat beside it", got) + } +} + +// What the store loads has no column for it, so the compiled value survives a load. +func TestTheNeedSurvivesTheStoresRows(t *testing.T) { + defer UseSeats(DefaultSeats()) + var rows []Seat + for _, s := range DefaultSeats() { + rows = append(rows, Seat{Name: s.Name, Scope: s.Scope, Delivers: s.Delivers, Decision: s.Decision}) + } + UseSeats(rows) + if s, _ := SeatNamed("node-resolver-config"); !reflect.DeepEqual(s.Needs, []string{"node-uplink"}) { + t.Errorf("after loading the store's rows node-resolver-config needs %v", s.Needs) + } +} + +func TestAssigningTheResolverConfigWithoutAnUplinkIsRefused(t *testing.T) { + cat := resolverAndUplinks() + _, err := AssignRefusal(cat, "laptop", nil, []string{"resolv-conf"}) + var refusal *Refusal + if !errors.As(err, &refusal) { + t.Fatalf("resolv-conf was assigned to a machine nothing manages the network of: %v", err) + } + for _, want := range []string{"resolv-conf on laptop depends on node-uplink", "networkmanager", "systemd-networkd"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the refusal does not say %q:\n%s", want, err) + } + } + // Beside a holder, or together with one in one act, it is let through. + if _, err := AssignRefusal(cat, "laptop", []string{"networkmanager"}, []string{"resolv-conf"}); err != nil { + t.Errorf("resolv-conf beside networkmanager was refused: %v", err) + } + if _, err := AssignRefusal(cat, "anchor", nil, []string{"systemd-networkd", "resolv-conf"}); err != nil { + t.Errorf("resolv-conf assigned with systemd-networkd was refused: %v", err) + } + // And a composition without one is refused once the switch is on. + if _, err := Resolve(cat, []string{"resolv-conf"}, workstation(), World{}); err == nil || + !strings.Contains(err.Error(), "node-uplink") { + t.Errorf("a node with resolv-conf and no uplink composed: %v", err) + } +} + +func TestUnassigningTheUplinkUnderTheResolverConfigIsRefused(t *testing.T) { + cat := resolverAndUplinks() + err := UnassignRefusal(cat, "laptop", []string{"networkmanager", "resolv-conf"}, []string{"networkmanager"}) + if err == nil || !strings.Contains(err.Error(), "resolv-conf") || !strings.Contains(err.Error(), "node-uplink") { + t.Errorf("taking the uplink from under resolv-conf gave %v", err) + } +} + +// The catalogue as it is: the module that writes resolv.conf depends on the uplink, and every manager +// the mesh knows can meet it — so the refusal always has a remedy to name. +func TestTheCataloguesResolverConfigHasUplinkHoldersToName(t *testing.T) { + cat := map[string]Manifest{} + for _, m := range theCatalogue(t) { + cat[m.Module] = m + } + deps := DependsOn(cat["resolv-conf"]) + found := false + for _, d := range deps { + found = found || d == "node-uplink" + } + if !found { + t.Errorf("the catalogue's resolv-conf depends on %v, not on node-uplink", deps) + } + holders := PossibleHolders(cat, "node-uplink") + for _, want := range []string{"dhcpcd", "networkmanager", "systemd-networkd"} { + in := false + for _, h := range holders { + in = in || h == want + } + if !in { + t.Errorf("%s does not hold node-uplink in the catalogue; holders: %v", want, holders) + } + } + if got := PossibleHolders(cat, "node-resolver-config"); !reflect.DeepEqual(got, []string{"resolv-conf"}) { + t.Errorf("node-resolver-config can be held by %v; resolv-conf alone since ADR 0220", got) + } +} diff --git a/internal/catalogue/seat_dependencies.go b/internal/catalogue/seat_dependencies.go index 8044993..0409ff2 100644 --- a/internal/catalogue/seat_dependencies.go +++ b/internal/catalogue/seat_dependencies.go @@ -6,8 +6,9 @@ import ( "strings" ) -// A module depends on the node seats that apply its resources (novox/hq ADR 0207), and on the -// seats it contributes to (novox/hq ADR 0210). +// A module depends on the node seats that apply its resources (novox/hq ADR 0207), on the +// seats it contributes to (novox/hq ADR 0210), and on the seats a seat it holds needs beside it +// (novox/hq ADR 0220). // // Some of what a module declares is applied through software on the machine that is itself a // module: a service through the service manager, a package through the package manager, a container @@ -93,6 +94,9 @@ func DependsOn(m Manifest) []string { for _, seat := range contributedTo(m) { seen[seat] = true } + for _, seat := range neededBesideClaims(m) { + seen[seat] = true + } out := make([]string, 0, len(seen)) for s := range seen { out = append(out, s) @@ -124,6 +128,20 @@ func contributedTo(m Manifest) []string { return out } +// neededBesideClaims is every seat a seat the module claims at node scope needs held on the same +// node (novox/hq ADR 0220): the holder of node-resolver-config is only right while node-uplink's +// holder keeps the network manager off resolv.conf. Derived from the claim, as a resource's seat is +// derived from its type, so a module that claims the seat cannot leave the dependency out. +func neededBesideClaims(m Manifest) []string { + var out []string + for _, name := range nodeSeatsClaimed(m) { + if s, known := SeatNamed(name); known { + out = append(out, s.Needs...) + } + } + return out +} + // claimsSeat is whether a module claims a node seat, by its current name or one it used to have // (ADR 0122), so a rename leaves the dependency met. func claimsSeat(m Manifest, seat string) bool { diff --git a/internal/catalogue/seats.go b/internal/catalogue/seats.go index 7764096..38e5066 100644 --- a/internal/catalogue/seats.go +++ b/internal/catalogue/seats.go @@ -45,6 +45,13 @@ type Seat struct { // ${contribution::}. Compiled, never stored: like the protocol, it is the mesh's // definition of the role, and the store's rows carry no column for it. Receives []Receivable + // Needs is every node seat this seat's holder needs held on its own node (novox/hq ADR 0220): a + // role whose holder is only right while another role is filled beside it. A module claiming this + // seat depends on each, exactly as a module declaring a service depends on the service manager + // (ADR 0207) — derived from the claim, never written in a manifest, and judged over the node's + // whole set of assignments. Compiled, never stored, like Receives: it is the mesh's definition of + // the role, and the store's rows carry no column for it. + Needs []string // Decision is the record that made it a seat. Decision string } @@ -137,10 +144,6 @@ var defaultSeats = append([]Seat{ // place, and every node and container asks it first. Delivers what a machine's resolver // configuration requires, so that requirement resolves to the holder wherever it is placed. {Name: "mesh-dns-resolver", Scope: ScopeMesh, Delivers: "wildcard-resolution", Decision: "novox/hq ADR 0194"}, - // **Retired by ADR 0194, kept while a manifest still claims it** — the same reason as - // mesh-build-machine above: a machine still holds it until the mesh's resolver replaces it, and - // removing the row first would make that machine unresolvable. Deleted once nothing claims it. - {Name: "node-dns-resolver", Scope: ScopeNode, Decision: "novox/hq ADR 0121"}, // **A machine's /etc/hosts is one module's** (novox/hq ADR 0199): its holder writes the machine's // own lines and keeps every other line as the operator's, changed through these three verbs on that // machine alone. The controller holds none of it. @@ -245,7 +248,14 @@ var defaultSeats = append([]Seat{ // Deferred (novox/hq ADR 0121): renaming to mesh-private-network is a scope + server/client // model change, not a rename, so it stays until that is built. {Name: "the-private-network", Scope: ScopeNode, Decision: "novox/hq ADR 0110"}, - {Name: "node-resolver-config", Scope: ScopeNode, Decision: "novox/hq ADR 0121"}, + // What a machine asks for names (novox/hq ADR 0121, ADR 0196): its holder writes resolv.conf. + // **And it needs the uplink held beside it** (novox/hq ADR 0220): resolv.conf stays the mesh's + // only while the program managing the machine's network is told to keep its hands off it, and + // that is what the uplink's holder says (ADR 0117). Without one, the first connectivity change + // rewrites the file and every surface of the mesh still reads green — so it is refused at + // assignment instead. + {Name: "node-resolver-config", Scope: ScopeNode, Decision: "novox/hq ADR 0121, ADR 0220", + Needs: []string{"node-uplink"}}, // The program that manages the machine's own network. It delivers nothing: its holder only // keeps the manager and the mesh from contradicting each other — the resolver file left to the // mesh, the private network's interface left alone — and never declares a link, an address or @@ -304,9 +314,11 @@ func UseSeats(s []Seat) { row.Accepts, row.Emits, row.Serves = d.Accepts, d.Emits, d.Serves } } - // What a seat receives is never stored (novox/hq ADR 0212), so it is always the compiled one. + // What a seat receives and what its holder needs are never stored (novox/hq ADR 0212, ADR + // 0220), so they are always the compiled ones. if d, known := byName[row.Name]; known { row.Receives = d.Receives + row.Needs = d.Needs } merged = append(merged, row) } diff --git a/internal/catalogue/seats_test.go b/internal/catalogue/seats_test.go index ffca17b..ad52627 100644 --- a/internal/catalogue/seats_test.go +++ b/internal/catalogue/seats_test.go @@ -46,14 +46,16 @@ func TestTheSeatsAreAClosedSetAndEachNamesItsDecision(t *testing.T) { delivered[s.Delivers] = s.Name } } - // Thirty-eight with node-backup (novox/hq ADR 0214); thirty-seven with node-message-bus (novox/hq ADR 0215); thirty-six with mesh-dns-resolver (novox/hq ADR 0194) and node-hosts-file (ADR 0199); thirty-four + // Thirty-seven since the retired node-dns-resolver went (novox/hq ADR 0220); thirty-eight with + // node-backup (novox/hq ADR 0214); thirty-seven with node-message-bus (novox/hq ADR 0215); + // thirty-six with mesh-dns-resolver (novox/hq ADR 0194) and node-hosts-file (ADR 0199); thirty-four // with node-hotkeys (ADR 0212); thirty-three with node-power (ADR 0211); thirty-two since the // graphical session's eleven (ADR 0208); twenty-one with node-package-manager and // node-container-runtime (ADR 0207); nineteen with node-environment and node-login-shell (ADR 0203, - // ADR 0204); seventeen with node-build-agent (ADR 0190). Two fewer once the retired - // mesh-build-machine and node-dns-resolver rows go, when no registered manifest claims either. - if len(Seats()) != 38 { - t.Errorf("the mesh defines %d seats rather than 38; the set is closed, so a change here is "+ + // ADR 0204); seventeen with node-build-agent (ADR 0190). One fewer once the retired + // mesh-build-machine row goes, when no registered manifest claims it. + if len(Seats()) != 37 { + t.Errorf("the mesh defines %d seats rather than 37; the set is closed, so a change here is "+ "a decision (novox/hq ADR 0110): %s", len(Seats()), seatNames()) } } diff --git a/internal/inventory/migrations/0060-the-per-node-resolver-seat-is-gone.sql b/internal/inventory/migrations/0060-the-per-node-resolver-seat-is-gone.sql new file mode 100644 index 0000000..5280847 --- /dev/null +++ b/internal/inventory/migrations/0060-the-per-node-resolver-seat-is-gone.sql @@ -0,0 +1,17 @@ +-- The per-node resolver's seat is gone (novox/hq ADR 0220, retiring what ADR 0194 decided). +-- +-- ADR 0194 retired `node-dns-resolver` when the mesh moved to one resolver, and kept its row while a +-- machine still held it: removing a seat that is claimed makes the claiming machine unresolvable. On +-- the mesh this was written for, nothing has held it since every node's resolver moved to the mesh's +-- one, and no module in the catalogue claims it, so the row goes. +-- +-- **The compiled defaults no longer carry it, and that alone would not remove it.** Seeding adds a +-- seat a release ships and never takes one away (ADR 0122: the table is the live set, and an +-- operator's row is left as it is), so a seat the mesh stops defining stays in the table until +-- something deletes it — this. +-- +-- A holding on record goes with it by cascade; there is none. No alias is kept: nothing was renamed, +-- and a manifest still claiming the old name should be refused at registration, naming it, rather +-- than resolve to anything. +delete from seat_alias where seat = 'node-dns-resolver' or alias = 'node-dns-resolver'; +delete from seat where name = 'node-dns-resolver';