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()
+	// **Not gated on which bus the controller is on, and that was a bug.** It read "compose this only
-	if err != nil || !onNATS {
+	// once the mesh is on the new bus" — which cannot work, because the server needs its user list
-		return "", err
+	// *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
-	// Only for the machine holding the bus. Asked of what this push resolves to rather than of the
+	// ADR 0116). Under the old gating that step could not happen: the module would come up, find no
-	// seat's holder mesh-wide: the file is a resource of that module, so the question is whether it
+	// accounts file, and its entrypoint would wait for one the controller had decided not to write.
-	// is here.
+	//
+	// 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") {

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")
+	}
+}