From e5007a7daa3d9af7570d52a6137d89cf5b81bcc1 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 27 Sep 2026 17:33:52 +0200 Subject: [PATCH] The user list is composed before anything moves onto the bus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by reading the live mesh's own notes before touching it, which is where this was heading next. Composing the bus's user list was gated on the controller already being on the new bus. That cannot work: the server needs its user list *before* anything moves onto it. Step 2 of the whole change is exactly that — the server stands in the mesh carrying nothing, on its own ports, while every node stays where it is. Under the old gating that step was impossible: the module would come up, find no accounts file, and its entrypoint would wait for one the controller had decided not to write. So the only question is whether this machine runs the module that asked for the file. A mesh that never moves has written a user list nothing reads, costing a few hundred bytes on one node. The reverse cost a step that could not be taken. Pinned by a test over the records of a mesh mid-change: everything running, nothing on the new bus, and a user list that contains the controller — because a file without it is a bus its own writer cannot connect to. --- cmd/mesh-controller/plan.go | 20 +++++++++++------- internal/broker/users_test.go | 38 +++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index 017f5b8..8877ccf 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -1160,13 +1160,19 @@ func portsOn( func composeBusUsers(ctx context.Context, inv *inventory.Inventory, onThisNode []catalogue.Manifest) (string, error) { - _, onNATS, err := broker.OnNATS() - if err != nil || !onNATS { - return "", err - } - // Only for the machine holding the bus. Asked of what this push resolves to rather than of the - // seat's holder mesh-wide: the file is a resource of that module, so the question is whether it - // is here. + // **Not gated on which bus the controller is on, and that was a bug.** It read "compose this only + // once the mesh is on the new bus" — which cannot work, because the server needs its user list + // *before* anything moves onto it. Step 2 of the change is exactly that: the server stands in the + // mesh carrying nothing, on its own ports, while every node is still on the old bus (novox/hq + // ADR 0116). Under the old gating that step could not happen: the module would come up, find no + // accounts file, and its entrypoint would wait for one the controller had decided not to write. + // + // So the question is only whether this machine runs the module that asked for the file. A mesh + // that never moves has written a user list nothing reads, which costs a few hundred bytes on one + // node; the reverse cost a step that cannot be taken. + // + // Asked of what this push resolves to rather than of the seat's holder mesh-wide: the file is a + // resource of that module, so the question is whether it is here. holdsTheBus := false for _, m := range onThisNode { if m.BusUsers != "" && m.ClaimsSeat("mesh-broker") { diff --git a/internal/broker/users_test.go b/internal/broker/users_test.go index 8f40e1b..0e21eaf 100644 --- a/internal/broker/users_test.go +++ b/internal/broker/users_test.go @@ -201,3 +201,41 @@ func TestTheAccountsFileRefusesAUserWithNoPassword(t *testing.T) { t.Fatal("a user with no password hash was written") } } + +// **A user list is composed for a bus the mesh has not moved onto yet**, and that is the whole of +// step 2 (novox/hq ADR 0116): the server stands in the mesh carrying nothing, on its own ports, while +// every node is still on the bus it was on. +// +// Pinned because the first version of the composing step got it backwards — it wrote the list only +// once the controller was already on the new bus, which is a step that cannot be taken: the module +// comes up, finds no accounts file, and waits for one the controller had decided not to write. +func TestAUserListIsComposedBeforeAnythingMovesOntoTheBus(t *testing.T) { + // Exactly the records of a mesh mid-change: everything running, nothing on the new bus. + users, err := Users(Records{ + Nodes: []string{"anchor"}, + Assigned: map[string][]Declared{"anchor": {{Module: "nats"}}}, + }) + if err != nil { + t.Fatal(err) + } + hashes := map[string]string{} + for _, u := range users { + hashes[u.Username()] = "$2a$11$" + strings.Repeat("x", 22) + } + filled, missing := WithPasswords(users, hashes) + if len(missing) != 0 { + t.Fatalf("users with no credential: %v", missing) + } + accounts, err := ComposeAccounts(filled) + if err != nil { + t.Fatal(err) + } + // The controller's own user above all: a file without it is a bus its writer cannot connect to, + // which is what the server would be left holding the moment it starts. + if !strings.Contains(accounts, `user: "controller"`) { + t.Fatalf("the composed list does not contain the controller:\n%s", accounts) + } + if !strings.Contains(accounts, `user: "node.anchor"`) { + t.Errorf("the composed list does not contain the machine running the bus") + } +}