From 63ca0739381ea90672859602d96a3d2b0c2b2071 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 27 Sep 2026 21:59:40 +0200 Subject: [PATCH] The store owns the seat set, so only the control plane may judge a claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A claim on a seat was checked against `SeatNamed` inside `ParseManifest`, and the build machine parses manifests too. It has no store, so there it answered from the set compiled into the binary — a copy of data the control plane owns (ADR 0122). When the two disagreed, that copy won where it mattered. The store's row said the bus seat answers for `amqp`; the binary's said `mesh-bus`; and a holder that provides `amqp` was refused at build time for not providing `mesh-bus`. The seat went unheld, the controller lost the address it composes through that seat, and the control plane crash-looped on a bus that was healthy the whole time. So the two checks that read the set — a seat's scope, and what its holder must provide — move to CatalogueProblems, which runs only in the control plane and only after UseSeats has replaced the set with the store's. The parser keeps what it can judge from the manifest alone, the reserved-namespace rule included. A test pins it: the same manifest, two different values in the store, and the answer follows the store both times. It fails if the check moves back. --- internal/catalogue/seats.go | 22 ++++++------- internal/catalogue/seats_declared.go | 17 +++++++++- internal/catalogue/seats_declared_test.go | 40 ++++++++++++++++++++++- internal/catalogue/seats_test.go | 26 +++++++++------ 4 files changed, 82 insertions(+), 23 deletions(-) diff --git a/internal/catalogue/seats.go b/internal/catalogue/seats.go index 8a70f70..9379370 100644 --- a/internal/catalogue/seats.go +++ b/internal/catalogue/seats.go @@ -184,17 +184,17 @@ func claimProblems(m Manifest) []string { } for _, c := range m.Claims { - if seat, known := SeatNamed(c.Name); known { - if c.At() != seat.Scope { - problems = append(problems, fmt.Sprintf( - "%s claims %s at scope %q, and %s is a %s seat", - m.Module, c.Name, c.At(), c.Name, seat.Scope)) - } - if seat.Delivers != "" && !providesAt(m, seat.Delivers, seat.Scope) { - problems = append(problems, fmt.Sprintf( - "%s claims %s, whose holder answers for %q, and %s does not provide %q at %s scope", - m.Module, c.Name, seat.Delivers, m.Module, seat.Delivers, seat.Scope)) - } + if _, known := SeatNamed(c.Name); known { + // **A seat's scope and what it delivers are not judged here** (novox/hq ADR 0122). + // This function runs wherever a manifest is parsed, and one of those places is the + // build machine, which has no store: there, `SeatNamed` answers from the set the + // binary shipped with, so a build would be refused for disagreeing with a compiled + // copy of data the control plane owns. Exactly that happened — a holder of the bus + // seat was refused for not providing what a stale compiled row said the seat + // delivered, while the store's own row said otherwise. + // + // Both checks moved to CatalogueProblems, which only ever runs in the control plane, + // after UseSeats has replaced the set with the store's. continue } if isSystemSeatName(c.Name) { diff --git a/internal/catalogue/seats_declared.go b/internal/catalogue/seats_declared.go index 5423c96..0188106 100644 --- a/internal/catalogue/seats_declared.go +++ b/internal/catalogue/seats_declared.go @@ -190,7 +190,22 @@ func CatalogueProblems(shelf Shelf) []string { } s, isModuleSeat := declared[c.Name] if !isModuleSeat { - continue // a mesh seat: already judged by claimProblems + // **A mesh seat is judged here and nowhere else** (novox/hq ADR 0122): the set is + // the store's, and this is the only place that runs with the store's set loaded. + // The parser cannot do it — it also runs on the build machine, against whatever + // set that binary was compiled with. + seat, _ := SeatNamed(c.Name) + if c.At() != seat.Scope { + problems = append(problems, fmt.Sprintf( + "%s claims %s at scope %q, and %s is a %s seat", + module, c.Name, c.At(), c.Name, seat.Scope)) + } + if seat.Delivers != "" && !providesAt(m, seat.Delivers, seat.Scope) { + problems = append(problems, fmt.Sprintf( + "%s claims %s, whose holder answers for %q, and %s does not provide %q at %s scope", + module, c.Name, seat.Delivers, module, seat.Delivers, seat.Scope)) + } + continue } if c.At() != s.At() { problems = append(problems, fmt.Sprintf( diff --git a/internal/catalogue/seats_declared_test.go b/internal/catalogue/seats_declared_test.go index 9c64d4b..8cdd2c3 100644 --- a/internal/catalogue/seats_declared_test.go +++ b/internal/catalogue/seats_declared_test.go @@ -91,7 +91,10 @@ func TestAClaimMustMatchTheDeclaredScope(t *testing.T) { // The mesh's own seats still work, and are not shadowed by the derived half. func TestTheMeshsOwnSeatsAreStillClaimable(t *testing.T) { - m := Manifest{Module: "nats", Claims: []Claim{{Name: "mesh-broker", Scope: ScopeMesh}}} + // It delivers the bus, so its holder provides the bus — the rule this check now enforces. + m := Manifest{Module: "nats", + Provides: []Offer{{Name: "mesh-bus", Scope: ScopeMesh}}, + Claims: []Claim{{Name: "mesh-broker", Scope: ScopeMesh}}} if got := problemsFor(t, Shelf{"nats": m}); got != "" { t.Fatalf("a mesh seat was refused by the derived check: %s", got) } @@ -106,3 +109,38 @@ func TestTheProblemsAreStable(t *testing.T) { t.Fatalf("unstable:\n%s\n%s", first, second) } } + +// **A build machine has no store, so it may not judge a seat.** The set is data the control plane +// owns (novox/hq ADR 0122), and `ParseManifest` runs on the build machine too, against whatever set +// that binary was compiled with. When the two disagreed, a valid holder of the bus seat was refused +// mid-rollout — the compiled row said the seat delivered one provision, the store's row said +// another, and the build failed on the copy rather than the truth. The parser judges the manifest; +// the seat set judges the claim, where it is loaded. +func TestTheParserDoesNotJudgeWhatOnlyTheStoreKnows(t *testing.T) { + was := Seats() + 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"}],` + + `"claims":[{"name":"mesh-broker","scope":"mesh"}]}`) + + m, err := ParseManifest(raw) + if err != nil { + t.Fatalf("the parser refused a claim only the seat set can judge: %v", err) + } + if got := CatalogueProblems(Shelf{m.Module: m}); len(got) != 0 { + t.Fatalf("a holder that provides what the store says the seat delivers was refused: %v", got) + } + + // 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"}}) + 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"`) { + t.Fatalf("registration did not refuse a holder that cannot answer for the seat: %q", got) + } +} diff --git a/internal/catalogue/seats_test.go b/internal/catalogue/seats_test.go index 030d043..1b33a72 100644 --- a/internal/catalogue/seats_test.go +++ b/internal/catalogue/seats_test.go @@ -138,26 +138,32 @@ func TestAModuleDefinesAndClaimsItsOwnSeat(t *testing.T) { } } +// **At registration, not in the parser** (novox/hq ADR 0122): a seat's scope is a property of the +// set, the set is the store's, and the parser also runs on a build machine that has no store. func TestASeatClaimedAtAnotherScopeIsRefused(t *testing.T) { - _, err := ParseManifest(claimed(`[{"name":"npm-package-registry","scope":"node"}]`)) - if err == nil { - t.Fatal("a mesh seat was held per node") + m, err := ParseManifest(claimed(`[{"name":"npm-package-registry","scope":"node"}]`)) + if err != nil { + t.Fatalf("the parser judged a scope it reads from data it may not have: %v", err) } - if !strings.Contains(err.Error(), "mesh seat") { - t.Fatalf("the refusal does not say which scope the seat is: %v", err) + got := strings.Join(CatalogueProblems(Shelf{m.Module: m}), "; ") + if !strings.Contains(got, "mesh seat") { + t.Fatalf("the refusal does not say which scope the seat is: %q", got) } } func TestADeliveringSeatIsOnlyHeldByAModuleThatProvides(t *testing.T) { // Holding it makes the module the mesh's answer for the provision. A module that cannot answer // would be the answer anyway, and every consumer would be sent to it. + // And refused at registration, where the seat set is the store's: what a seat delivers is + // data, so a compiled copy of it may not be what refuses a build (novox/hq ADR 0122). raw := []byte(`{"module":"thing","version":"1","claims":[{"name":"git","scope":"mesh"}]}`) - _, err := ParseManifest(raw) - if err == nil { - t.Fatal("a module holding the git seat need not provide git") + m, err := ParseManifest(raw) + if err != nil { + t.Fatalf("the parser judged what a seat delivers: %v", err) } - if !strings.Contains(err.Error(), `does not provide "git"`) { - t.Fatalf("the refusal does not say what is missing: %v", err) + got := strings.Join(CatalogueProblems(Shelf{m.Module: m}), "; ") + if !strings.Contains(got, `does not provide "git"`) { + t.Fatalf("the refusal does not say what is missing: %q", got) } } -- 2.54.0