diff --git a/internal/catalogue/amqp_is_not_a_provision_test.go b/internal/catalogue/amqp_is_not_a_provision_test.go new file mode 100644 index 0000000..c6d08b0 --- /dev/null +++ b/internal/catalogue/amqp_is_not_a_provision_test.go @@ -0,0 +1,48 @@ +package catalogue + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// **The word does not come back through a manifest** (novox/hq ADR 0131). A module that wants +// messaging wants the mesh's bus, reached through the sdk and named by the `mesh-broker` seat. Naming +// the old wire protocol asks for the one server being retired, so both directions are refused at the +// parser — this is judged from the manifest alone, no store needed. + +func TestAManifestProvidingAmqpIsRefused(t *testing.T) { + raw := []byte(`{"module":"old-broker","version":"1","provides":[{"name":"amqp","scope":"mesh"}]}`) + _, err := ParseManifest(raw) + if err == nil || !strings.Contains(err.Error(), `provides "amqp", which is not a provision`) { + t.Fatalf("a module providing amqp was not refused, or not for the reason: %v", err) + } +} + +func TestAManifestRequiringAmqpIsRefused(t *testing.T) { + raw := []byte(`{"module":"forwarder","version":"1","requires":["amqp"]}`) + _, err := ParseManifest(raw) + if err == nil || !strings.Contains(err.Error(), `requires "amqp", which is not a provision`) { + t.Fatalf("a module requiring amqp was not refused, or not for the reason: %v", err) + } +} + +// And the catalogue as checked out beside this repository names it nowhere — the three modules that +// did are removed under design 28 task 5.4, not converted. +func TestNoCatalogueManifestNamesAmqp(t *testing.T) { + modules, err := filepath.Glob("../../../mesh-catalog/modules/*/module.json") + if err != nil || len(modules) == 0 { + t.Skip("the catalogue is not checked out beside this repository") + } + for _, path := range modules { + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(raw), `"amqp"`) { + t.Errorf("%s names amqp, which is not a provision (novox/hq ADR 0131)", + filepath.Base(filepath.Dir(path))) + } + } +} diff --git a/internal/catalogue/broker_seat_test.go b/internal/catalogue/broker_seat_test.go index d10c5a1..f9ca20a 100644 --- a/internal/catalogue/broker_seat_test.go +++ b/internal/catalogue/broker_seat_test.go @@ -46,41 +46,9 @@ func TestTheSeatRefusesADifferentBusToo(t *testing.T) { } } -// **The old broker claims the seat until the seat is handed over, and stands beside the new one -// while it waits** (novox/hq ADR 0131, superseding the record this test used to pin). Whoever is on -// record holds it; the other eligible claimant is neither refused nor holding. This is the shape the -// handover needs: both brokers assigned, one bus, no moment with nobody in the seat. -func TestTheOldBrokerStandsBesideTheNewOneUntilTheHandover(t *testing.T) { - was := Seats() - t.Cleanup(func() { UseSeats(was) }) - UseSeats([]Seat{{Name: "mesh-broker", Scope: ScopeMesh, Delivers: "mesh-bus", Decision: "test"}}) - - lavinmq := catalogueManifest(t, "lavinmq") - if !lavinmq.ClaimsSeat("mesh-broker") { - t.Skip("the old broker no longer claims the seat: design 28 task 5.4 has removed it") - } - nats := catalogueManifest(t, "nats") - onRecord := World{Holdings: []Held{{Claim: "mesh-broker", Scope: ScopeMesh, - Node: "anchor", Module: "nats"}}} - - // The same machine runs both. Without the record this is two holders and refused; with it, the - // recorded one holds and the other is silent. - anchor := workstation() - anchor.Name = "anchor" - got, err := Resolve(shelf(lavinmq, nats), []string{"lavinmq", "nats"}, anchor, onRecord) - if err != nil { - t.Fatalf("the old broker beside the recorded holder was refused: %v", err) - } - var holders []string - for _, h := range got.Claims { - if h.Claim == "mesh-broker" { - holders = append(holders, h.Module) - } - } - if len(holders) != 1 || holders[0] != "nats" { - t.Fatalf("the seat is held by %v, not by the holder on record alone", holders) - } -} +// The old broker is gone from the catalogue (novox/hq ADR 0131, design 28 task 5.4), so it is no +// longer a fixture here. That two eligible holders stand beside each other with one on record is +// pinned in holdings_test.go against manifests this package owns. // **A seat and the interface it delivers are different names, and renaming one must not rename // the other** (novox/hq ADR 0118). This nearly went wrong: the seats were renamed to the `mesh-*` diff --git a/internal/catalogue/foundation_manifests_test.go b/internal/catalogue/foundation_manifests_test.go index 5565993..01e0c5d 100644 --- a/internal/catalogue/foundation_manifests_test.go +++ b/internal/catalogue/foundation_manifests_test.go @@ -28,8 +28,10 @@ func TestTheStoreAndTheBrokerSayWhatTheMeshGuards(t *testing.T) { if got := catalogueManifest(t, "postgres").Guards; !reflect.DeepEqual(got, []int{5432}) { t.Errorf("postgres guards %v; the store's port must be refused from outside", got) } - if got := catalogueManifest(t, "lavinmq").Guards; !reflect.DeepEqual(got, []int{15672}) { - t.Errorf("lavinmq guards %v; the management port must be refused from outside", got) + // The bus's monitoring port, not its client port: a node reaches the bus, nobody outside + // reads its state (novox/hq ADR 0131 — the broker that guarded 15672 has left the catalogue). + if got := catalogueManifest(t, "nats").Guards; !reflect.DeepEqual(got, []int{8222}) { + t.Errorf("nats guards %v; the monitoring port must be refused from outside", got) } } diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 64a67bc..0a87d74 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -1027,6 +1027,28 @@ func ParseManifest(raw []byte) (Manifest, error) { problems = append(problems, fmt.Sprintf( "%q is not a usable slug: lower-case letters, digits, dashes and dots", m.Slug)) } + // **`amqp` is not a provision, and not a requirement** (novox/hq ADR 0131). A module that wants + // messaging wants the mesh's bus — it emits and consumes through the sdk, which the mesh hands the + // bus with the module's own credential — and the bus is whatever holds `mesh-broker`, spoken in + // whatever that holder speaks. Naming the old wire protocol asks for a specific server, and the + // only one that could answer is the one being retired. Refused here so the word cannot come back + // through a manifest. + for _, offer := range m.Provides { + if offer.Name == "amqp" { + problems = append(problems, fmt.Sprintf( + "%s provides %q, which is not a provision: the mesh's bus is whatever holds "+ + "mesh-broker, and a module provides mesh-bus to be it (novox/hq ADR 0131)", + m.Module, offer.Name)) + } + } + for _, r := range m.Requires { + if r == "amqp" { + problems = append(problems, fmt.Sprintf( + "%s requires %q, which is not a provision: a module reaches the mesh's bus through "+ + "the sdk, and depends on the mesh-broker seat, not on a protocol (novox/hq ADR 0131)", + m.Module, r)) + } + } for _, offer := range m.Provides { p := offer.Name if !name.MatchString(p) { diff --git a/internal/catalogue/seats_declared_test.go b/internal/catalogue/seats_declared_test.go index 8cdd2c3..b9a0329 100644 --- a/internal/catalogue/seats_declared_test.go +++ b/internal/catalogue/seats_declared_test.go @@ -121,9 +121,9 @@ func TestTheParserDoesNotJudgeWhatOnlyTheStoreKnows(t *testing.T) { t.Cleanup(func() { UseSeats(was) }) // A store whose bus seat delivers something this module does provide. - UseSeats([]Seat{{Name: "mesh-broker", Scope: ScopeMesh, Delivers: "amqp", Decision: "test"}}) - raw := []byte(`{"module":"lavinmq","version":"1",` + - `"provides":[{"name":"amqp","scope":"mesh"}],` + + UseSeats([]Seat{{Name: "mesh-broker", Scope: ScopeMesh, Delivers: "mesh-bus", Decision: "test"}}) + raw := []byte(`{"module":"a-bus","version":"1",` + + `"provides":[{"name":"mesh-bus","scope":"mesh"}],` + `"claims":[{"name":"mesh-broker","scope":"mesh"}]}`) m, err := ParseManifest(raw) @@ -135,12 +135,12 @@ func TestTheParserDoesNotJudgeWhatOnlyTheStoreKnows(t *testing.T) { } // And with the store saying the seat delivers something else, registration is what refuses it. - UseSeats([]Seat{{Name: "mesh-broker", Scope: ScopeMesh, Delivers: "mesh-bus", Decision: "test"}}) + UseSeats([]Seat{{Name: "mesh-broker", Scope: ScopeMesh, Delivers: "other-bus", Decision: "test"}}) if _, err := ParseManifest(raw); err != nil { t.Fatalf("the parser judged it the second time: %v", err) } got := strings.Join(CatalogueProblems(Shelf{m.Module: m}), "; ") - if !strings.Contains(got, `does not provide "mesh-bus"`) { + if !strings.Contains(got, `does not provide "other-bus"`) { t.Fatalf("registration did not refuse a holder that cannot answer for the seat: %q", got) } } diff --git a/internal/inventory/migrations/0040-the-broker-seat-delivers-the-bus.sql b/internal/inventory/migrations/0040-the-broker-seat-delivers-the-bus.sql new file mode 100644 index 0000000..51e591c --- /dev/null +++ b/internal/inventory/migrations/0040-the-broker-seat-delivers-the-bus.sql @@ -0,0 +1,12 @@ +-- The bus seat's holder answers for the mesh's bus, not for a wire protocol (novox/hq ADR 0131). +-- +-- The row said `amqp`, which is the protocol the old broker spoke, and so only that broker could hold +-- the seat that names the mesh's bus — while the module that will carry the bus could not. The seat +-- delivers `mesh-bus`; whichever module provides that may hold it, and today that is one module. +-- +-- Safe under the current holder: the control plane composes its own bus address through the seat by +-- name, and the overview derives holders by name. Only registration and provision-to-seat resolution +-- read this column. So the row changes, the current holder keeps holding by derivation, the next one +-- can register its claim, and the handover (0039) moves the seat when both are running. What must +-- not happen in between is re-registering the current holder — registration would now refuse it. +update seat set delivers = 'mesh-bus' where name = 'mesh-broker' and delivers = 'amqp';