From 79a9e17df07f5d722cf6a5c69ffa7d7ccb3d846e Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 17 Sep 2026 22:31:36 +0200 Subject: [PATCH] The broker credential resolves the mesh-broker seat, not the hub (issue 059) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An adversarial review of the 055 fix found it encoded the wrong invariants, latent while every mesh keeps its broker on the hub. Now: the address is the overlay name of the node ASSIGNED a module claiming the mesh-broker seat (the hub stands in only while nothing holds the seat — genesis); "on the overlay" is what whereEveryoneIs answers (resolved the networking module), not "has an address"; a portless genesis address defaults to 5671 instead of silently disabling the path; a second `overlay place --hub` is refused rather than last-write-wins; and `overlay place` says that earlier credentials keep their old address. A test now binds the controller's own module.json to its seat, so deleting the claim fails the suite. https://claude.ai/code/session_01D6qtiYU3P9jk3pnAXyAFyx --- cmd/mesh-controller/modules.go | 94 +++++++++++++++++++++++------- cmd/mesh-controller/network.go | 19 ++++++ internal/catalogue/resolve_test.go | 23 ++++++++ 3 files changed, 115 insertions(+), 21 deletions(-) diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index bacdba2..95d6c04 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -467,36 +467,88 @@ func pinCommand(ctx context.Context, args []string, setting bool) error { return nil } +// theBrokerSeat is the mesh-scoped seat the broker module claims (novox/hq ADR 0079: a +// foundation seat is named after the server it guards). +const theBrokerSeat = "mesh-broker" + // brokerReachableAt is the broker's address as the given node can reach it. // -// The genesis address (MESH_BROKER_ADDRESS) is the broker's public endpoint — reachable from the -// control-node itself, but not routed to another node, whose firewall admits only the overlay -// (from:mesh). The foundation, and so the broker, sits on the control-node, which is the overlay -// hub; a node that is on the overlay reaches the broker by the hub's `.internal` name, which the -// firewall admits and every node resolves. A node not yet on the overlay — at genesis, before any -// `overlay place`, which is when the builder's account is issued — keeps the genesis address it -// was given, so nothing about bring-up changes. This is issue 055. +// The genesis address (MESH_BROKER_ADDRESS) is the broker's public endpoint — right for a machine +// that can dial it, wrong across the mesh, where the path is the overlay. A node that is ON the +// overlay is given the overlay name of the node HOLDING the broker — the one assigned a module +// claiming the mesh-broker seat — because that is where the broker is, whatever else the topology +// says. Only when nothing holds the seat yet (genesis raised the broker as plumbing and no module +// has adopted it) does the hub stand in, which is where the foundation is by convention. +// +// "On the overlay" is what `whereEveryoneIs` answers — a machine that RESOLVED the networking +// module — not "has an address", which is true of every placed machine and says nothing about +// whether anything can reach it (novox/hq issue 059). A node not on the overlay — at genesis, +// before any `overlay place`, which is when the builder's account is issued — keeps the genesis +// address, so nothing about bring-up changes. This is issue 055, corrected by 059. func brokerReachableAt(ctx context.Context, inv *inventory.Inventory, known broker.Broker, node string) (string, error) { - overlays, err := inv.Overlays(ctx) + shelf, err := inv.Catalogue(ctx) if err != nil { return "", err } - var hub string - onOverlay := false - for _, o := range overlays { - if o.Hub && o.Address != "" { - hub = o.Name - } - if o.Name == node && o.Address != "" { - onOverlay = true - } - } - if hub == "" || !onOverlay { + onNetwork, err := whereEveryoneIs(ctx, inv, shelf) + if err != nil { + // No catalogue yet is genesis, and at genesis the genesis address is the right one. return known.Address, nil } + if onNetwork[node] == "" { + return known.Address, nil + } + + // Where the broker is: the node whose assigned set includes a module claiming the seat. + holders := map[string]bool{} + for name, m := range shelf { + for _, c := range m.Claims { + if c.Name == theBrokerSeat && c.At() == catalogue.ScopeMesh { + holders[name] = true + } + } + } + brokerAt := "" + if len(holders) > 0 { + overlays, err := inv.Overlays(ctx) + if err != nil { + return "", err + } + for _, o := range overlays { + assigned, err := inv.Assigned(ctx, o.Name) + if err != nil { + continue + } + for _, a := range assigned { + if holders[a] && onNetwork[o.Name] != "" { + brokerAt = onNetwork[o.Name] + } + } + } + } + if brokerAt == "" { + // Nothing holds the seat (or its node is not on the overlay): the hub, where the + // foundation is by convention — and only a hub that is itself on the overlay, or the + // name we hand out routes nowhere. + overlays, err := inv.Overlays(ctx) + if err != nil { + return "", err + } + for _, o := range overlays { + if o.Hub && onNetwork[o.Name] != "" { + brokerAt = onNetwork[o.Name] + } + } + } + if brokerAt == "" { + return known.Address, nil + } + // A portless genesis address is a working configuration (amqps defaults to 5671), and + // silently keeping the public address would disable this whole path — so the port defaults + // rather than the fix dissolving (novox/hq issue 059). _, port, err := net.SplitHostPort(known.Address) if err != nil { - return known.Address, nil + port = "5671" } - return net.JoinHostPort(overlay.InternalName(hub), port), nil + return net.JoinHostPort(brokerAt, port), nil } diff --git a/cmd/mesh-controller/network.go b/cmd/mesh-controller/network.go index 79ddc8c..59325e1 100644 --- a/cmd/mesh-controller/network.go +++ b/cmd/mesh-controller/network.go @@ -93,6 +93,23 @@ func overlayPlace(ctx context.Context, inv *inventory.Inventory, args []string) "and the mesh will not choose between them", node) } + // One hub per mesh, refused rather than last-write-wins: with two flagged, which one the + // graph and the broker address pick is order-dependent — the silent-election fault ADR 0007 + // exists to avoid, one flag over (novox/hq issue 059). Moving the hub is explicit: re-place + // the old one without --hub first. + if *hub { + placed, err := inv.Overlays(ctx) + if err != nil { + return err + } + for _, p := range placed { + if p.Hub && p.Name != node { + return fmt.Errorf("%s is already the hub; a mesh has one. Re-place %s without "+ + "--hub first if the hub is moving", p.Name, p.Name) + } + } + } + // Declared, all three. The address is evidence of reachability and is not the fact, and hub // election by address prefix fails silently (novox/hq ADR 0007). if err := inv.SetPlace(ctx, node, *endpoint, *site, *hub, ""); err != nil { @@ -108,6 +125,8 @@ func overlayPlace(ctx context.Context, inv *inventory.Inventory, args []string) } fmt.Printf("%s is at %s on the overlay\n", node, address) + fmt.Println(" credentials issued for it before this placement keep their old broker address —" + + " `module issue` them again and push (novox/hq issue 059)") switch { case *hub: fmt.Println(" the hub — every node not sharing a site routes through it") diff --git a/internal/catalogue/resolve_test.go b/internal/catalogue/resolve_test.go index 24d7144..54b1e10 100644 --- a/internal/catalogue/resolve_test.go +++ b/internal/catalogue/resolve_test.go @@ -2,6 +2,8 @@ package catalogue import ( "fmt" + "os" + "path/filepath" "strings" "testing" ) @@ -484,3 +486,24 @@ func TestSomethingOrdinaryAnsweredHereNeedsNothing(t *testing.T) { t.Fatalf("a shell answered on this machine produced %d need(s)", len(got.Needs)) } } + +func TestTheControllersOwnManifestClaimsItsSeat(t *testing.T) { + // The seat test above fabricates manifests, so deleting the claim from the real module.json + // would fail nothing (novox/hq issue 059's review). This binds the one manifest this + // repository owns: the controller claims the mesh-scoped seat named after its server + // (ADR 0079), or the one-controller property is convention again. + raw, err := os.ReadFile(filepath.Join("..", "..", "module.json")) + if err != nil { + t.Fatalf("the controller's own manifest is unreadable: %v", err) + } + m, err := ParseManifest(raw) + if err != nil { + t.Fatalf("the controller's own manifest does not parse: %v", err) + } + for _, c := range m.Claims { + if c.Name == "mesh-controller" && c.At() == ScopeMesh { + return + } + } + t.Fatalf("module.json no longer claims the mesh-scoped mesh-controller seat: %+v", m.Claims) +} -- 2.54.0