From e1367d185ad1b606b2a7f0dc1eb76a83bd84c58f Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 19:01:41 +0200 Subject: [PATCH 1/6] Send the bus's user list alone before a walk's gate, so a new grant is on the bus when the first machine is judged (issue 490) A walk left the machine holding the bus out of a gated send whenever a build waited there for that same gate, so a module's new health tool was refused by the bus on its first machine and the gate could not pass. The grants step sends that machine its declaration with every build kept, before the first machine's send; plans and the change plan say it. --- cmd/mesh-controller/changeplan.go | 24 ++++ cmd/mesh-controller/gate.go | 4 + cmd/mesh-controller/grants_step_test.go | 163 ++++++++++++++++++++++++ cmd/mesh-controller/issue352_test.go | 2 +- cmd/mesh-controller/recorded_kept.go | 42 +++++- cmd/mesh-controller/release.go | 98 ++++++++++++-- cmd/mesh-controller/release_plan.go | 7 +- internal/inventory/plans.go | 12 ++ 8 files changed, 335 insertions(+), 17 deletions(-) create mode 100644 cmd/mesh-controller/grants_step_test.go diff --git a/cmd/mesh-controller/changeplan.go b/cmd/mesh-controller/changeplan.go index 7a983c97..9c84cd19 100644 --- a/cmd/mesh-controller/changeplan.go +++ b/cmd/mesh-controller/changeplan.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "slices" "sort" "strings" @@ -28,6 +29,14 @@ func changePlanOf(repository, base, head string, r mergeReach, entries []invento for _, e := range entries { byName[e.Manifest.Module] = e } + // The machine holding the bus, whose declaration carries every module's grants (novox/hq issue 490). + holder := "" + for _, e := range entries { + if e.Manifest.BusUsers != "" && e.Manifest.ClaimsSeat(catalogue.BrokerSeat) && len(e.On) > 0 { + holder = e.On[0] + } + } + var granting []string machines := map[string]*link.MachinePlan{} machine := func(name string) *link.MachinePlan { if machines[name] == nil { @@ -50,6 +59,10 @@ func changePlanOf(repository, base, head string, r mergeReach, entries []invento machine(on).Receives = append(machine(on).Receives, name) } } + if !waits && holder != "" && !(len(e.On) == 1 && e.On[0] == holder) && len(e.On) > 0 && + !slices.Contains(granting, name) { + granting = append(granting, name) + } switch { case (name == "nats" || catalogue.ProvidesBus(e.Manifest)) && len(e.On) > 0: // Said before the merge (novox/hq issue 336): a new bus build holds every send to the bus's machine @@ -76,6 +89,13 @@ func changePlanOf(repository, base, head string, r mergeReach, entries []invento } } } + if len(granting) > 0 { + // Said before the merge (novox/hq issue 490): what the walk does when the change alters the bus's + // user list. Whether it does is known only once the builds are registered, so it is said as a rule. + p.Steps = append(p.Steps, fmt.Sprintf("%s: if this changes what the bus's user list says (a new tool, "+ + "subject or user), %s is sent that list alone, every build there kept, before %s is judged on its first "+ + "machine", grantsStepWord, holder, strings.Join(granting, ", "))) + } var names []string for name := range machines { names = append(names, name) @@ -88,6 +108,10 @@ func changePlanOf(repository, base, head string, r mergeReach, entries []invento return p } +// grantsStepWord names the grants step (novox/hq issue 490): the bus's user list sent alone to the machine +// holding the bus ahead of a walk's gate. Not the bus step, which replaces the bus itself. +const grantsStepWord = "grants step" + // busUpgradeNeeded is how a change plan says that merging it holds the bus's machine for a person's step. const busUpgradeNeeded = "merging this needs a person's bus upgrade" diff --git a/cmd/mesh-controller/gate.go b/cmd/mesh-controller/gate.go index 36eefc4d..8d3bab69 100644 --- a/cmd/mesh-controller/gate.go +++ b/cmd/mesh-controller/gate.go @@ -1353,6 +1353,10 @@ func gateLine(g *inventory.PlanGate) string { if g.Rollback != "" { line += "; " + g.Rollback } + // Before the send it judges (novox/hq issue 490). + if said := grantsSaid(g.Grants); said != "" { + line += "; " + said + } return line } diff --git a/cmd/mesh-controller/grants_step_test.go b/cmd/mesh-controller/grants_step_test.go new file mode 100644 index 00000000..59458c6d --- /dev/null +++ b/cmd/mesh-controller/grants_step_test.go @@ -0,0 +1,163 @@ +package main + +import ( + "context" + "errors" + "slices" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// The grants reach the bus before the gate (novox/hq issue 490). +// +// On 2026-10-11 a merge gave restic, assigned to every machine, a new health tool. Its walk sent restic to +// its first machine and judged it there; that machine's node-engine asked the new tool and the bus refused +// it, because the grant was in the bus's user list, which only the machine holding the bus carries — and +// that machine runs restic too, so it was left out of the send while restic's new build waited there for +// the very gate that could not pass. A person pushed it by hand, carrying restic's new build there unjudged. + +// aSend is one send the walk made: to which machines, and whether every build there was kept. +type aSend struct { + names []string + keepsAll bool + judged []string +} + +// sendsRecorded replaces the walk's send and the reading of the bus's user list for one test: the machine +// holding the bus is novox, and its list is behind as behind says. +func sendsRecorded(t *testing.T, holder string, behind bool, refuse error) *[]aSend { + t.Helper() + var sends []aSend + wasSend, wasBehind := sendRollout, brokerBehindOf + t.Cleanup(func() { sendRollout, brokerBehindOf = wasSend, wasBehind }) + brokerBehindOf = func(_ context.Context, _ *stores, names []string) (string, bool, error) { + if slices.Contains(names, holder) { + return holder, false, nil + } + return holder, behind, nil + } + sendRollout = func(ctx context.Context, _ *stores, names []string) ([]string, error) { + s := aSend{names: append([]string(nil), names...), keepsAll: everyBuildKept(ctx)} + for n := range scopeOf(ctx).judged { + s.judged = append(s.judged, n) + } + sends = append(sends, s) + if refuse != nil && s.keepsAll { + return nil, refuse + } + return names, nil + } + return &sends +} + +// The restic shape: a new tool on a module on every machine, its first machine ace, the bus on novox. +func TestAWalkSendsTheGrantsToTheBusBeforeTheFirstMachineIsJudged(t *testing.T) { + sends := sendsRecorded(t, "novox", true, nil) + sent, grants, err := sendJudged(t.Context(), nil, "ace") + if err != nil { + t.Fatal(err) + } + if len(*sends) != 2 { + t.Fatalf("the walk made %d send(s), want the grants step and then the first machine: %+v", len(*sends), *sends) + } + first, then := (*sends)[0], (*sends)[1] + if strings.Join(first.names, ",") != "novox" || !first.keepsAll || len(first.judged) != 0 { + t.Errorf("the first send is %+v, want novox alone, every build there kept, judging nothing", first) + } + if strings.Join(then.names, ",") != "ace" || then.keepsAll || strings.Join(then.judged, ",") != "ace" { + t.Errorf("the second send is %+v, want ace, judged", then) + } + if strings.Join(sent, ",") != "ace" { + t.Errorf("the gate's machines are %v, want ace alone: the bus's machine is not judged", sent) + } + if grants == nil || grants.Node != "novox" || grants.At == nil || grants.Failed != "" { + t.Fatalf("the gate keeps %+v as its grants step", grants) + } + line := gateLine(&inventory.PlanGate{Machines: sent, Grants: grants}) + if !strings.Contains(line, "grants step: novox sent the bus's user list first, its builds kept") { + t.Errorf("plans says the gate as %q", line) + } +} + +// Nothing more when there is nothing to carry: the list unchanged, or the first machine holds the bus. +func TestAWalkTakesNoGrantsStepWhenTheListIsAlreadyThere(t *testing.T) { + for _, c := range []struct { + name string + node string + behind bool + }{{"the list unchanged", "ace", false}, {"the first machine holds the bus", "novox", true}} { + t.Run(c.name, func(t *testing.T) { + sends := sendsRecorded(t, "novox", c.behind, nil) + _, grants, err := sendJudged(t.Context(), nil, c.node) + if err != nil { + t.Fatal(err) + } + if len(*sends) != 1 || (*sends)[0].keepsAll || grants != nil { + t.Errorf("the walk made %+v with grants %+v, want the judged send alone", *sends, grants) + } + }) + } +} + +// A grants step that cannot be sent is said on the gate, and the walk goes on as it did before. +func TestAGrantsStepThatFailsIsSaidAndTheSendGoesOn(t *testing.T) { + sends := sendsRecorded(t, "novox", true, errors.New("a bus upgrade waits for a person")) + sent, grants, err := sendJudged(t.Context(), nil, "ace") + if err != nil { + t.Fatal(err) + } + if strings.Join(sent, ",") != "ace" || len(*sends) != 2 { + t.Errorf("the walk made %+v", *sends) + } + if grants == nil || grants.Failed == "" || grants.At != nil { + t.Fatalf("the gate keeps %+v", grants) + } + if line := gateLine(&inventory.PlanGate{Machines: sent, Grants: grants}); !strings.Contains(line, + "grants step to novox FAILED, sent without it: a bus upgrade waits for a person") { + t.Errorf("plans says the gate as %q", line) + } +} + +// The grants step moves no build, so no gate's refusal holds it back: it is never read for moves. +func TestTheGrantsStepIsNotHeldForTheBuildsItKeeps(t *testing.T) { + names, err := ungatedIn(keepingEveryBuild(t.Context()), nil, []string{"novox"}, "") + if err != nil || strings.Join(names, ",") != "novox" { + t.Errorf("the grants step's send reads %v, %v", names, err) + } +} + +// The delivery plan says the step before the merge. +func TestAChangePlanSaysTheGrantsStep(t *testing.T) { + const catalogue_ = "http://forge.internal:20000/novox/mesh-catalog.git" + entry := func(name string, on ...string) inventory.Entry { + e := fromRepo(name, catalogue_, "modules/"+name) + e.Source.BuiltFrom = "old" + e.On = on + return e + } + nats := entry("nats", "novox") + nats.Manifest.BusUsers = "/etc/nats/users.conf" + nats.Manifest.Claims = []catalogue.Claim{{Name: catalogue.BrokerSeat, Scope: catalogue.ScopeMesh}} + restic := entry("restic", "ace", "novox", "shanks") + only := entry("bus-tools", "novox") + entries := []inventory.Entry{nats, restic, only} + rollsOut := func(string) (inventory.Upgrade, bool) { return inventory.Upgrade{RollOut: true}, true } + plan := func(paths ...string) link.ChangePlan { + m := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "head", Paths: paths} + return changePlanOf("novox/mesh-catalog", "main", "head", reachOfMerge(m, entries, nil, nil), entries, rollsOut) + } + + text := planText(plan("modules/restic/module.json")) + if !strings.Contains(text, "grants step: if this changes what the bus's user list says (a new tool, subject or "+ + "user), novox is sent that list alone, every build there kept, before restic is judged on its first machine") { + t.Errorf("the change plan does not say the grants step:\n%s", text) + } + // A module only on the bus's machine goes there with the list in its own send: no step of its own. + if text := planText(plan("modules/bus-tools/module.json")); strings.Contains(text, "grants step") { + t.Errorf("a module on the bus's machine alone reads:\n%s", text) + } +} diff --git a/cmd/mesh-controller/issue352_test.go b/cmd/mesh-controller/issue352_test.go index 951f8514..10afa237 100644 --- a/cmd/mesh-controller/issue352_test.go +++ b/cmd/mesh-controller/issue352_test.go @@ -69,7 +69,7 @@ func TestAPlansFirstSendWaitsForAReleaseJudgingTheModuleThere(t *testing.T) { b := aBacklog(t) ctx := t.Context() advancePlans(ctx, b.open) // the release judges app c2 on anchor - _, _, err := gatedSend(ctx, b.open, "anchor", []inventory.CarriedMove{{Module: "app", Node: "anchor", From: "c2", To: "c3", Build: "build-app-c3"}}) + _, _, _, err := gatedSend(ctx, b.open, "anchor", []inventory.CarriedMove{{Module: "app", Node: "anchor", From: "c2", To: "c3", Build: "build-app-c3"}}) if !errors.Is(err, errWalkedElsewhere) || !strings.Contains(err.Error(), "release-") { t.Fatalf("a newer build of a judged module was sent under the release's gate: %v", err) } diff --git a/cmd/mesh-controller/recorded_kept.go b/cmd/mesh-controller/recorded_kept.go index bbe6f18f..5c81bec8 100644 --- a/cmd/mesh-controller/recorded_kept.go +++ b/cmd/mesh-controller/recorded_kept.go @@ -61,20 +61,52 @@ func keptExcept(ctx context.Context) (map[string]bool, bool) { return skip, on } +type keepEveryBuildKey struct{} + +// keepingEveryBuild is a context whose sends compose every module — recorded or rolling out — at the build +// its machine was last sent (novox/hq issue 490): the grants step, which sends the machine holding the bus +// its new user list and nothing of any module's new code. That code waits there for a gate like any other +// move; the list must not, or the first machine's gate is judged against a bus that refuses what the change +// newly grants. +func keepingEveryBuild(ctx context.Context) context.Context { + return context.WithValue(ctx, keepEveryBuildKey{}, true) +} + +// everyBuildKept is whether this context keeps every module at the build its machine runs. +func everyBuildKept(ctx context.Context) bool { + on, _ := ctx.Value(keepEveryBuildKey{}).(bool) + return on +} + // recordedKept is, for a send under keepingRecorded, every recorded module the machine was last sent a // build of that the mesh's build is not identical to: module → the commit it keeps. Nil when the context // keeps nothing, or when what the machine was last sent is not known (it is then held whole elsewhere — // ADR 0221). +// +// Under keepingEveryBuild, every module the machine was sent is kept, whatever its policy (novox/hq issue +// 490), and a machine whose last send is not known refuses the send: there is nothing to keep it at, and +// composing the mesh's builds would be the very move the grants step must not make. func recordedKept(ctx context.Context, open *stores, node string) (map[string]string, error) { + every := everyBuildKept(ctx) skip, on := keptExcept(ctx) - if !on { + if !on && !every { return nil, nil } + if every { + skip = nil + } inv := open.inventory sent, known, err := inv.SentBuilds(ctx, node) - if err != nil || !known { + if err != nil { return nil, err } + if !known { + if every { + return nil, fmt.Errorf("what %s was last sent is not known, so the bus's user list cannot be sent "+ + "there alone with every build kept (novox/hq issue 490)", node) + } + return nil, nil + } current, err := inv.CurrentBuilds(ctx) if err != nil { return nil, err @@ -83,7 +115,7 @@ func recordedKept(ctx context.Context, open *stores, node string) (map[string]st out := map[string]string{} for m, was := range sent { now, held := current[m] - if !held || now.RollOut || skip[m] || was == "" || sameCommit(was, now.Commit) { + if !held || (now.RollOut && !every) || skip[m] || was == "" || sameCommit(was, now.Commit) { continue } if f == nil { @@ -118,6 +150,10 @@ func keepRecorded(ctx context.Context, open *stores, node string, shelf map[stri if err != nil { return nil, err } + if !found && everyBuildKept(ctx) { + return nil, fmt.Errorf("%s runs %s's build %s, which the build records no longer hold: the bus's user "+ + "list cannot be sent there alone with it kept (novox/hq issue 490)", node, m, short(kept[m])) + } if !found { return nil, fmt.Errorf("%s records rather than rolls out, and %s runs its build %s, which the build "+ "records no longer hold: this send cannot keep it and does not move it — `push %s` sends the new "+ diff --git a/cmd/mesh-controller/release.go b/cmd/mesh-controller/release.go index 56b702c8..87dcc079 100644 --- a/cmd/mesh-controller/release.go +++ b/cmd/mesh-controller/release.go @@ -239,6 +239,11 @@ var machineMoves = func(ctx context.Context, open *stores, f moveFacts, node str // ungatedIn refuses a send whose machines would move a build no gate has seen, outside its scope: the // machines named, and why; the machine holding the bus, added only for its user list, is left instead. func ungatedIn(ctx context.Context, open *stores, names []string, addedHolder string) ([]string, error) { + // The grants step moves no build: every module is composed at the build its machine runs (novox/hq + // issue 490), so there is nothing here for a gate to judge. + if everyBuildKept(ctx) { + return names, nil + } scope := scopeOf(ctx) if scope.person { return names, nil @@ -283,15 +288,18 @@ func ungatedIn(ctx context.Context, open *stores, names []string, addedHolder st // owns are the moves the send exists for — every module of a plan's tier whose first machine this is, in // one send (novox/hq issue 281): a send carries the machine's whole declaration (ADR 0221), so a send per // module was the same declaration sent again and again, each one setting aside the one before. -func gatedSend(ctx context.Context, open *stores, node string, owns []inventory.CarriedMove) ([]inventory.CarriedMove, []string, error) { +// +// And the grants step first, when the send changes what the bus's user list must say (novox/hq issue 490): +// answered for the gate to keep, nil when there was none. +func gatedSend(ctx context.Context, open *stores, node string, owns []inventory.CarriedMove) ([]inventory.CarriedMove, []string, *inventory.GrantsStep, error) { inv := open.inventory f, err := readMoveFacts(ctx, inv) if err != nil { - return nil, nil, err + return nil, nil, nil, err } moves, err := machineMoves(ctx, open, f, node, true) if err != nil { - return nil, nil, err + return nil, nil, nil, err } own := func(module string) bool { return slices.ContainsFunc(owns, func(o inventory.CarriedMove) bool { return o.Module == module }) @@ -301,7 +309,7 @@ func gatedSend(ctx context.Context, open *stores, node string, owns []inventory. continue } if id := f.walkedBy(mv.Module, node, mv.To); id != "" { - return nil, nil, fmt.Errorf("%w: %s's build %s waits on %s, which %s is walking", errWalkedElsewhere, + return nil, nil, nil, fmt.Errorf("%w: %s's build %s waits on %s, which %s is walking", errWalkedElsewhere, mv.Module, short(mv.To), node, id) } } @@ -309,7 +317,7 @@ func gatedSend(ctx context.Context, open *stores, node string, owns []inventory. // plan, or a release, on this machine is not crossed with a newer build; this send waits for its gate. for _, o := range owns { if id := f.walkedBy(o.Module, node, o.To); id != "" { - return nil, nil, fmt.Errorf("%w: %s's build %s waits on %s, which %s is walking", errWalkedElsewhere, + return nil, nil, nil, fmt.Errorf("%w: %s's build %s waits on %s, which %s is walking", errWalkedElsewhere, o.Module, short(o.To), node, id) } } @@ -323,22 +331,22 @@ func gatedSend(ctx context.Context, open *stores, node string, owns []inventory. } } if len(moves) == 0 && len(owns) == 0 { - return nil, nil, nil + return nil, nil, nil, nil } for i := range moves { if moves[i].Build == "" { if moves[i].Build, err = inv.BuildOf(ctx, moves[i].Module, moves[i].To); err != nil { - return nil, nil, err + return nil, nil, nil, err } } } sort.Slice(moves, func(i, j int) bool { return moves[i].Module < moves[j].Module }) sayRecreations(ctx, open, moves) - sent, err := sendRollout(withScope(ctx, sendScope{judged: map[string]bool{node: true}}), open, []string{node}) + sent, grants, err := sendJudged(ctx, open, node) if err != nil { - return nil, nil, err + return nil, nil, nil, err } - return moves, sent, nil + return moves, sent, grants, nil } // passCarried keeps a pass as the verdict of every build the gate judged beside its own module. @@ -674,7 +682,7 @@ func advanceRelease(ctx context.Context, open *stores, p *inventory.Plan) (bool, r.Next++ continue } - moves, sent, err := gatedSend(ctx, open, node, nil) + moves, sent, grants, err := gatedSend(ctx, open, node, nil) if errors.Is(err, errWalkedElsewhere) { note := fmt.Sprintf("waiting before %s: %v", node, err) changed := p.Note != note @@ -689,8 +697,12 @@ func advanceRelease(ctx context.Context, open *stores, p *inventory.Plan) (bool, r.Next++ continue } - r.Gate = &inventory.PlanGate{Machines: sent, Since: &now, Carried: moves, Sent: sentNow(ctx, open.inventory, sent)} + r.Gate = &inventory.PlanGate{Machines: sent, Since: &now, Carried: moves, Sent: sentNow(ctx, open.inventory, sent), + Grants: grants} p.Note = fmt.Sprintf("sent %s %d build(s) that waited for a gate; judging them there", node, len(moves)) + if said := grantsSaid(grants); said != "" { + p.Note += "; " + said + } if said := recreationsSaid(moves); said != "" { p.Note += "; " + said } @@ -916,3 +928,65 @@ func recreationsSaid(moves []inventory.CarriedMove) string { } return strings.Join(said, "; ") } + +// The grants step (novox/hq issue 490). +// +// The bus's user list — every module's grants, composed mesh-wide from the catalogue — travels only in the +// declaration of the machine holding the bus. A send to any other machine that changed it put that machine +// first (issue 249), unless a build waited there for a gate: then ungatedIn left it out, and said so. On +// 2026-10-11 a merge gave restic a new health tool; its walk sent restic to its first machine and judged it +// there, the machine's node-engine asked the tool and the bus refused it ("this host's grant does not name +// …"), because the machine holding the bus also runs restic, whose new build waited for that very gate. The +// gate could not pass; a person pushed the bus's machine by hand, which carried restic's new build there +// unjudged. +// +// So before a gated send, when the user list composed now is not the one the machine holding the bus was +// last sent, that machine is sent its declaration with **every build kept at the one it runs**: the new +// list, and nothing of any module's new code. Said on the gate (`plans`), and, when it cannot be sent, said +// with why and passed over: the send goes on as it did before, and its gate says what it finds. + +// brokerBehindOf is brokerBehind: a variable so a test of the walk needs no store. +var brokerBehindOf = brokerBehind + +// grantsStep sends the machine holding the bus its user list alone, ahead of a gated send to node, when the +// list changed; answers what it did, or nil when there was nothing to send. +func grantsStep(ctx context.Context, open *stores, node string) *inventory.GrantsStep { + holder, behind, err := brokerBehindOf(ctx, open, []string{node}) + if err != nil { + fmt.Printf("grants step before %s: whether the bus's user list changed could not be read: %v\n", node, err) + return &inventory.GrantsStep{Node: "the machine holding the bus", Failed: err.Error()} + } + if holder == "" || holder == node || !behind { + // No bus with a user list, the gate's own machine holds it (its send carries the list first), or the + // list is the one already sent. + return nil + } + if _, err := sendRollout(keepingEveryBuild(withScope(ctx, sendScope{})), open, []string{holder}); err != nil { + fmt.Printf("grants step: the bus's user list could not be sent to %s ahead of %s, which is sent without "+ + "it — the bus may refuse what the send newly grants: %v\n", holder, node, err) + return &inventory.GrantsStep{Node: holder, Failed: err.Error()} + } + now := time.Now().UTC() + fmt.Printf("grants step: %s sent the bus's user list alone, every build there kept, before %s is judged\n", + holder, node) + return &inventory.GrantsStep{Node: holder, At: &now} +} + +// sendJudged is a gated send's sends: the grants step when the user list changed, then the machine judged. +func sendJudged(ctx context.Context, open *stores, node string) ([]string, *inventory.GrantsStep, error) { + grants := grantsStep(ctx, open, node) + sent, err := sendRollout(withScope(ctx, sendScope{judged: map[string]bool{node: true}}), open, []string{node}) + return sent, grants, err +} + +// grantsSaid is a grants step as `plans` and a walk's note say it. +func grantsSaid(g *inventory.GrantsStep) string { + switch { + case g == nil: + return "" + case g.Failed != "": + return fmt.Sprintf(grantsStepWord+" to %s FAILED, sent without it: %s", g.Node, g.Failed) + default: + return fmt.Sprintf(grantsStepWord+": %s sent the bus's user list first, its builds kept", g.Node) + } +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 7adc089c..d3a9b260 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -971,7 +971,7 @@ func firstSend(ctx context.Context, open *stores, p *inventory.Plan, node string } // A gated send (ADR 0236): everything waiting on the machine goes with the tier, and the gate judges // all of it there. - carried, sent, err := gatedSend(ctx, open, node, owns) + carried, sent, grants, err := gatedSend(ctx, open, node, owns) if err != nil { return err } @@ -992,6 +992,7 @@ func firstSend(ctx context.Context, open *stores, p *inventory.Plan, node string s.GatedBy = "" if m == lead { s.Gate.Carried = carried + s.Gate.Grants = grants } else { s.GatedBy = lead } @@ -1001,6 +1002,10 @@ func firstSend(ctx context.Context, open *stores, p *inventory.Plan, node string strings.Join(sent, ", ")) fmt.Printf("%s: tier %d built; sent %d module(s) to %s first in one send (%s), the rest once its gate passes\n", p.ID, p.Tier, len(modules), strings.Join(sent, ", "), strings.Join(modules, ", ")) + // The grants step taken before it (novox/hq issue 490), said with it. + if said := grantsSaid(grants); said != "" { + p.Note += "; " + said + } // What the send recreates, said with it (novox/hq ADR 0245). if said := recreationsSaid(carried); said != "" { p.Note += "; " + said diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index bd03edc5..718b483c 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -305,6 +305,18 @@ type PlanGate struct { // Readings are the judging's readings with their times (novox/hq ADR 0282 decision 6): every reading that // counted a pass, and the first that did not after one that did. At most maxReadings, the newest kept. Readings []GateReading `json:"readings,omitempty"` + // Grants is the grants step taken before this gate's send (novox/hq issue 490): the bus's user list sent + // alone to the machine holding the bus, every build there kept, so what the send newly grants is on the + // bus before the gate judges it. Nil when the list did not change, or the gate's machine holds the bus. + Grants *GrantsStep `json:"grants,omitempty"` +} + +// GrantsStep is the bus's user list sent to the machine holding the bus ahead of a gated send (novox/hq +// issue 490): to which machine, when, and, when it could not be sent, why — the send went on without it. +type GrantsStep struct { + Node string `json:"node"` + At *time.Time `json:"at,omitempty"` + Failed string `json:"failed,omitempty"` } // GateReading is one reading of a first-node gate. From fbeffb608deda67faec5f2223e2e26b5d04d736c Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 19:09:23 +0200 Subject: [PATCH 2/6] Refuse the grants step when its send would change more than the bus's user list (review of #230) Kept builds are not the whole declaration: a new assignment, settings or a provision's logins went out as the mesh holds them now. The step now compares what it would send with the last send's summary and fails, said, when anything but the user list differs; ungatedIn reads the kept builds instead of passing the step unread. --- cmd/mesh-controller/grants_step_store_test.go | 161 ++++++++++++++++++ cmd/mesh-controller/grants_step_test.go | 25 ++- cmd/mesh-controller/release.go | 111 +++++++++++- 3 files changed, 285 insertions(+), 12 deletions(-) create mode 100644 cmd/mesh-controller/grants_step_store_test.go diff --git a/cmd/mesh-controller/grants_step_store_test.go b/cmd/mesh-controller/grants_step_store_test.go new file mode 100644 index 00000000..ecd664ac --- /dev/null +++ b/cmd/mesh-controller/grants_step_store_test.go @@ -0,0 +1,161 @@ +package main + +import ( + "encoding/json" + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// The grants step on a store (novox/hq issue 490): the machine holding the bus is sent its new user list, +// and nothing else — not the new build of a module it runs, not a module newly assigned there. +// +// The restic shape: anchor holds the bus and runs restic at c1, as does laptop; restic's c2 gives it a +// health tool. The grants step's send to anchor composes restic at c1, carries the user list that grants +// laptop's node-engine c2's tool, and records that anchor still runs c1. A module newly assigned to anchor +// makes the step fail, said, rather than install it unjudged. +func TestTheGrantsStepSendsTheUserListAndNothingElse(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + inv := open.inventory + start := time.Now().Add(-time.Hour) + image := "registry.invalid:5000/restic/backup@sha256:" + strings.Repeat("b", 64) + build := func(module, commit string, at time.Time, manifest map[string]any) link.BuildResult { + manifest["module"], manifest["version"] = module, "1" + raw, _ := json.Marshal(manifest) + return link.BuildResult{ID: link.NewBuildID(at), Repository: "novox/mesh-catalog", Path: "modules/" + module, + On: "anchor", Module: module, Commit: commit, Manifest: raw, + Source: &link.SourceOnSeat{Seat: "git", Repository: "novox/mesh-catalog"}} + } + restic := func(health map[string]any, tools ...string) map[string]any { + r := map[string]any{"id": "backup", "type": "container", "name": "restic-backup", "image": image} + if health != nil { + r["health"] = health + } + m := map[string]any{"resources": []any{r}} + if len(tools) > 0 { + m["tools"] = tools + } + return m + } + nats := map[string]any{"bus-users": "/var/lib/nats-module/conf/accounts.conf", + "claims": []any{map[string]any{"name": catalogue.BrokerSeat, "scope": catalogue.ScopeMesh}}} + for _, b := range []link.BuildResult{ + build("nats", "n1", start, nats), + build("restic", "c1", start.Add(time.Second), restic(nil)), + build("wallpaper", "w1", start.Add(2*time.Second), map[string]any{"resources": []any{ + map[string]any{"id": "paper", "type": "container", "name": "wallpaper", "image": image}}}), + } { + if _, _, err := takeIn(ctx, inv, asTheOperator(t, inv, b)); err != nil { + t.Fatal(err) + } + } + for _, a := range [][2]string{{"anchor", "nats"}, {"anchor", "restic"}, {"laptop", "restic"}} { + if _, err := inv.Assign(ctx, a[0], a[1]); err != nil { + t.Fatal(err) + } + } + for _, n := range []string{"anchor", "laptop"} { + if _, err := inv.MintBusPassword(ctx, inventory.BusUser{Username: "node." + n, Kind: inventory.BusNode, Node: n}); err != nil { + t.Fatal(err) + } + } + + // What anchor was last sent: everything at the builds of before the merge, kept as a send keeps it. + sendAnchor := func() { + t.Helper() + plan, settings, err := planFor(keepingRecorded(ctx), open, "anchor") + if err != nil { + t.Fatal(err) + } + gens, err := generators(ctx, open) + if err != nil { + t.Fatal(err) + } + declared, err := declarationWith(keepingRecorded(ctx), open, "anchor", plan, settings, gens, Allocating) + if err != nil { + t.Fatal(err) + } + body, err := declared.Body() + if err != nil { + t.Fatal(err) + } + if _, err := recordSent(ctx, inv, "anchor", body, declared.Builds, 0); err != nil { + t.Fatal(err) + } + } + sendAnchor() + if only, err := grantsOnly(ctx, open, "anchor"); err != nil || only != "" { + t.Fatalf("with nothing changed, the step reads %q, %v", only, err) + } + + // The merge: restic's c2 gives it a health tool. + if _, _, err := takeIn(ctx, inv, build("restic", "c2", start.Add(time.Minute), + restic(map[string]any{"kind": catalogue.HealthTool, "tool": "backup_health"}, "backup_health"))); err != nil { + t.Fatal(err) + } + + // The step's send to anchor: restic as anchor runs it, c1, and the user list granting c2's tool. + kept := keepingEveryBuild(keepingRecorded(ctx)) + plan, settings, err := planFor(kept, open, "anchor") + if err != nil { + t.Fatal(err) + } + for _, m := range plan.Modules { + if m.Module == "restic" && len(m.Tools) > 0 { + t.Fatalf("the grants step composed restic's new build on anchor: tools %v", m.Tools) + } + } + list, _, err := busUserList(ctx, inv, plan.Modules) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(list, "mesh.mod.restic.tool.backup_health.laptop") { + t.Errorf("the user list the step sends does not grant laptop's node-engine restic's new tool:\n%s", list) + } + gens, err := generators(kept, open) + if err != nil { + t.Fatal(err) + } + declared, err := declarationWith(kept, open, "anchor", plan, settings, gens, Allocating) + if err != nil { + t.Fatal(err) + } + if declared.Builds["restic"] != "c1" { + t.Fatalf("the step's send records it carries restic %q; want c1, which anchor runs", declared.Builds["restic"]) + } + if !strings.Contains(declared.BusUsers, "mesh.mod.restic.tool.backup_health.laptop") { + t.Error("the step's declaration does not carry the new grant") + } + // Recorded as a send records it: anchor still runs restic c1, and its gate is still to come there. + if err := inv.RecordSent(ctx, nodeID(t, open, "anchor"), "d-grants", declared.Builds); err != nil { + t.Fatal(err) + } + if sent, _, err := inv.SentBuilds(ctx, "anchor"); err != nil || sent["restic"] != "c1" { + t.Fatalf("after the step anchor reads as sent restic %q (%v); want c1", sent["restic"], err) + } + // The guard: only the user list differs from what anchor was last sent. + if only, err := grantsOnly(ctx, open, "anchor"); err != nil || only != "" { + t.Fatalf("the step reads as changing more than the user list: %q, %v", only, err) + } + // And the gate's refusal still reads the step's send: it moves nothing there. + if names, err := ungatedIn(kept, open, []string{"anchor"}, ""); err != nil || strings.Join(names, ",") != "anchor" { + t.Fatalf("the step's send is refused as a move: %v, %v", names, err) + } + + // A module newly assigned to anchor: the step would install it, unjudged, so it is not sent. + if _, err := inv.Assign(ctx, "anchor", "wallpaper"); err != nil { + t.Fatal(err) + } + only, err := grantsOnly(ctx, open, "anchor") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(only, "wallpaper newly assigned") || !strings.Contains(only, "wallpaper") { + t.Fatalf("a new assignment on the bus's machine reads %q; want the step refused, naming it", only) + } +} diff --git a/cmd/mesh-controller/grants_step_test.go b/cmd/mesh-controller/grants_step_test.go index 59458c6d..fe128009 100644 --- a/cmd/mesh-controller/grants_step_test.go +++ b/cmd/mesh-controller/grants_step_test.go @@ -32,8 +32,9 @@ type aSend struct { func sendsRecorded(t *testing.T, holder string, behind bool, refuse error) *[]aSend { t.Helper() var sends []aSend - wasSend, wasBehind := sendRollout, brokerBehindOf - t.Cleanup(func() { sendRollout, brokerBehindOf = wasSend, wasBehind }) + wasSend, wasBehind, wasOnly := sendRollout, brokerBehindOf, grantsOnlyOf + t.Cleanup(func() { sendRollout, brokerBehindOf, grantsOnlyOf = wasSend, wasBehind, wasOnly }) + grantsOnlyOf = func(context.Context, *stores, string) (string, error) { return "", nil } brokerBehindOf = func(_ context.Context, _ *stores, names []string) (string, bool, error) { if slices.Contains(names, holder) { return holder, false, nil @@ -122,11 +123,21 @@ func TestAGrantsStepThatFailsIsSaidAndTheSendGoesOn(t *testing.T) { } } -// The grants step moves no build, so no gate's refusal holds it back: it is never read for moves. -func TestTheGrantsStepIsNotHeldForTheBuildsItKeeps(t *testing.T) { - names, err := ungatedIn(keepingEveryBuild(t.Context()), nil, []string{"novox"}, "") - if err != nil || strings.Join(names, ",") != "novox" { - t.Errorf("the grants step's send reads %v, %v", names, err) +// A step whose send would change more than the user list is not sent: it fails, said, and the walk goes on. +func TestAGrantsStepThatWouldCarryMoreIsNotSent(t *testing.T) { + sends := sendsRecorded(t, "novox", true, nil) + grantsOnlyOf = func(context.Context, *stores, string) (string, error) { + return "its send would change more than the bus's user list: -postgres.login-n8n", nil + } + sent, grants, err := sendJudged(t.Context(), nil, "ace") + if err != nil { + t.Fatal(err) + } + if len(*sends) != 1 || (*sends)[0].keepsAll || strings.Join(sent, ",") != "ace" { + t.Errorf("the walk made %+v, want ace's judged send alone", *sends) + } + if grants == nil || grants.At != nil || !strings.Contains(grants.Failed, "-postgres.login-n8n") { + t.Fatalf("the gate keeps %+v", grants) } } diff --git a/cmd/mesh-controller/release.go b/cmd/mesh-controller/release.go index 87dcc079..afc2db23 100644 --- a/cmd/mesh-controller/release.go +++ b/cmd/mesh-controller/release.go @@ -2,6 +2,7 @@ package main import ( "context" + "encoding/json" "errors" "flag" "fmt" @@ -239,11 +240,6 @@ var machineMoves = func(ctx context.Context, open *stores, f moveFacts, node str // ungatedIn refuses a send whose machines would move a build no gate has seen, outside its scope: the // machines named, and why; the machine holding the bus, added only for its user list, is left instead. func ungatedIn(ctx context.Context, open *stores, names []string, addedHolder string) ([]string, error) { - // The grants step moves no build: every module is composed at the build its machine runs (novox/hq - // issue 490), so there is nothing here for a gate to judge. - if everyBuildKept(ctx) { - return names, nil - } scope := scopeOf(ctx) if scope.person { return names, nil @@ -258,8 +254,19 @@ func ungatedIn(ctx context.Context, open *stores, names []string, addedHolder st if err != nil { return nil, err } + // The grants step composes every module at the build its machine was last sent (novox/hq issue 490): + // a move it keeps is not made. Read, not assumed, so a move it could not keep is still refused here. + var keeps map[string]string + if everyBuildKept(ctx) { + if keeps, err = recordedKept(ctx, open, n); err != nil { + return nil, err + } + } var waiting []string for _, mv := range moves { + if was, kept := keeps[mv.Module]; kept && sameCommit(was, mv.From) { + continue + } if !scope.judged[n] && !scope.modules[mv.Module] { waiting = append(waiting, fmt.Sprintf("%s %s → %s", mv.Module, short(mv.From), short(mv.To))) } @@ -961,6 +968,19 @@ func grantsStep(ctx context.Context, open *stores, node string) *inventory.Grant // list is the one already sent. return nil } + // **The user list and nothing else**: checked before it is sent, against what the machine was last sent. + // Kept builds are not the whole of a declaration — a new assignment, a settings change, an assignment + // taken away, a provision's logins are composed as the mesh holds them now — and a step that carried any + // of it would be the unjudged change this step exists to avoid. + if other, err := grantsOnlyOf(ctx, open, holder); err != nil || other != "" { + why := other + if err != nil { + why = err.Error() + } + fmt.Printf("grants step: %s is not sent its user list ahead of %s, which is sent without it: %s\n", + holder, node, why) + return &inventory.GrantsStep{Node: holder, Failed: why} + } if _, err := sendRollout(keepingEveryBuild(withScope(ctx, sendScope{})), open, []string{holder}); err != nil { fmt.Printf("grants step: the bus's user list could not be sent to %s ahead of %s, which is sent without "+ "it — the bus may refuse what the send newly grants: %v\n", holder, node, err) @@ -972,6 +992,87 @@ func grantsStep(ctx context.Context, open *stores, node string) *inventory.Grant return &inventory.GrantsStep{Node: holder, At: &now} } +// grantsOnlyOf is grantsOnly: a variable so a test of the walk needs no store. +var grantsOnlyOf = grantsOnly + +// grantsOnly is what the grants step's send to the machine holding the bus would change there besides its +// user list, against what it was last sent (novox/hq issue 490); empty when nothing else. Composed as the +// step's send composes it, without allocating anything (Foreseeing), and compared resource by resource +// with the summary the last send kept (ADR 0217): exact, rather than a list of the cases that can differ. +// A module there that was never sent is a new assignment, and refused too, whatever its resources. +func grantsOnly(ctx context.Context, open *stores, holder string) (string, error) { + inv := open.inventory + kept := keepingEveryBuild(keepingRecorded(ctx)) + plan, settings, err := planFor(kept, open, holder) + if err != nil { + return "", err + } + sent, known, err := inv.SentBuilds(ctx, holder) + if err != nil { + return "", err + } + if !known { + return fmt.Sprintf("what %s was last sent is not known", holder), nil + } + var other []string + listID := "" + for _, m := range plan.Modules { + if _, was := sent[m.Module]; !was { + other = append(other, m.Module+" newly assigned") + } + if m.BusUsers != "" && m.ClaimsSeat(catalogue.BrokerSeat) { + listID = m.Module + "." + catalogue.BusUsersID() + } + } + gens, err := generators(kept, open) + if err != nil { + return "", err + } + declared, err := declarationWith(kept, open, holder, plan, settings, gens, Foreseeing) + if err != nil { + return "", err + } + for _, f := range declared.foreseen { + other = append(other, f+" would be minted") + } + body, err := declared.Body() + if err != nil { + return "", err + } + after, err := summarize(body) + if err != nil { + return "", err + } + prev, err := inv.SentSummary(ctx, holder) + if err != nil { + return "", err + } + if len(prev) == 0 { + return fmt.Sprintf("what %s was last sent is not kept for comparison", holder), nil + } + var before []sentResource + if err := json.Unmarshal(prev, &before); err != nil { + return "", fmt.Errorf("what %s was last sent is kept in a form this version does not read: %w", holder, err) + } + d := diffSent(before, after) + for _, id := range d.Added { + other = append(other, "+"+id) + } + for _, id := range d.Removed { + other = append(other, "-"+id) + } + for _, c := range d.Changed { + if c.ID == listID { + continue + } + other = append(other, fmt.Sprintf("~%s (%s)", c.ID, strings.Join(c.Fields, ", "))) + } + if len(other) == 0 { + return "", nil + } + return "its send would change more than the bus's user list: " + strings.Join(other, "; "), nil +} + // sendJudged is a gated send's sends: the grants step when the user list changed, then the machine judged. func sendJudged(ctx context.Context, open *stores, node string) ([]string, *inventory.GrantsStep, error) { grants := grantsStep(ctx, open, node) From 78205d7405555874c2be673604c819e39392b7ac Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 19:12:33 +0200 Subject: [PATCH 3/6] Judge the grants step on the declaration it sends, so nothing composed between the check and the send slips through (review of #230) The guard composed its own declaration ahead of the send; now sendToEach judges the one it composed and stamped, and refuses it unsent. --- cmd/mesh-controller/grants_step_store_test.go | 85 +++++++++++-------- cmd/mesh-controller/grants_step_test.go | 19 ++--- cmd/mesh-controller/push.go | 11 +++ cmd/mesh-controller/release.go | 61 ++++++------- 4 files changed, 96 insertions(+), 80 deletions(-) diff --git a/cmd/mesh-controller/grants_step_store_test.go b/cmd/mesh-controller/grants_step_store_test.go index ecd664ac..ae01d6e7 100644 --- a/cmd/mesh-controller/grants_step_store_test.go +++ b/cmd/mesh-controller/grants_step_store_test.go @@ -1,6 +1,7 @@ package main import ( + "context" "encoding/json" "strings" "testing" @@ -65,31 +66,50 @@ func TestTheGrantsStepSendsTheUserListAndNothingElse(t *testing.T) { } } - // What anchor was last sent: everything at the builds of before the merge, kept as a send keeps it. - sendAnchor := func() { + wasEpoch := epochForActs + epochForActs = func(context.Context) (uint64, error) { return 7, nil } + t.Cleanup(func() { epochForActs = wasEpoch }) + + // A send to anchor composed as sendToEach composes it: numbered, planned, declared, stamped. + compose := func(under context.Context) (catalogue.Resolution, sendable) { t.Helper() - plan, settings, err := planFor(keepingRecorded(ctx), open, "anchor") + numbered, err := allot(under, inv, "anchor") if err != nil { t.Fatal(err) } - gens, err := generators(ctx, open) + plan, settings, err := planFor(under, open, "anchor") if err != nil { t.Fatal(err) } - declared, err := declarationWith(keepingRecorded(ctx), open, "anchor", plan, settings, gens, Allocating) + gens, err := generators(under, open) if err != nil { t.Fatal(err) } + declared, err := declarationWith(under, open, "anchor", plan, settings, gens, Allocating) + if err != nil { + t.Fatal(err) + } + numbered.stamp(&declared) + return plan, declared + } + record := func(declared sendable) { + t.Helper() body, err := declared.Body() if err != nil { t.Fatal(err) } - if _, err := recordSent(ctx, inv, "anchor", body, declared.Builds, 0); err != nil { + if _, err := recordSent(ctx, inv, "anchor", body, declared.Builds, declared.Epoch, + sentRecordOf(ctx, declared)); err != nil { t.Fatal(err) } } - sendAnchor() - if only, err := grantsOnly(ctx, open, "anchor"); err != nil || only != "" { + kept := keepingEveryBuild(keepingRecorded(ctx)) + + // What anchor was last sent: everything at the builds of before the merge, as an ordinary send sends it. + _, before := compose(keepingRecorded(ctx)) + record(before) + plan, declared := compose(kept) + if only, err := grantsOnlyIn(ctx, open, "anchor", plan, declared); err != nil || only != "" { t.Fatalf("with nothing changed, the step reads %q, %v", only, err) } @@ -100,62 +120,53 @@ func TestTheGrantsStepSendsTheUserListAndNothingElse(t *testing.T) { } // The step's send to anchor: restic as anchor runs it, c1, and the user list granting c2's tool. - kept := keepingEveryBuild(keepingRecorded(ctx)) - plan, settings, err := planFor(kept, open, "anchor") + generation, err := inv.AssignmentGeneration(ctx) if err != nil { t.Fatal(err) } + plan, declared = compose(kept) for _, m := range plan.Modules { if m.Module == "restic" && len(m.Tools) > 0 { t.Fatalf("the grants step composed restic's new build on anchor: tools %v", m.Tools) } } - list, _, err := busUserList(ctx, inv, plan.Modules) - if err != nil { - t.Fatal(err) - } - if !strings.Contains(list, "mesh.mod.restic.tool.backup_health.laptop") { - t.Errorf("the user list the step sends does not grant laptop's node-engine restic's new tool:\n%s", list) - } - gens, err := generators(kept, open) - if err != nil { - t.Fatal(err) - } - declared, err := declarationWith(kept, open, "anchor", plan, settings, gens, Allocating) - if err != nil { - t.Fatal(err) - } if declared.Builds["restic"] != "c1" { t.Fatalf("the step's send records it carries restic %q; want c1, which anchor runs", declared.Builds["restic"]) } if !strings.Contains(declared.BusUsers, "mesh.mod.restic.tool.backup_health.laptop") { - t.Error("the step's declaration does not carry the new grant") + t.Errorf("the step's declaration does not grant laptop's node-engine restic's new tool:\n%s", declared.BusUsers) } - // Recorded as a send records it: anchor still runs restic c1, and its gate is still to come there. - if err := inv.RecordSent(ctx, nodeID(t, open, "anchor"), "d-grants", declared.Builds); err != nil { - t.Fatal(err) + // The generation it carries is the current one (novox/hq issue 234), and the step does not raise it. + if declared.composedFrom != generation { + t.Errorf("the step's send was composed from generation %d; the mesh is at %d", declared.composedFrom, generation) } - if sent, _, err := inv.SentBuilds(ctx, "anchor"); err != nil || sent["restic"] != "c1" { - t.Fatalf("after the step anchor reads as sent restic %q (%v); want c1", sent["restic"], err) - } - // The guard: only the user list differs from what anchor was last sent. - if only, err := grantsOnly(ctx, open, "anchor"); err != nil || only != "" { + // The guard, on the very declaration the send sends: only the user list differs from the last send. + if only, err := grantsOnlyIn(ctx, open, "anchor", plan, declared); err != nil || only != "" { t.Fatalf("the step reads as changing more than the user list: %q, %v", only, err) } // And the gate's refusal still reads the step's send: it moves nothing there. if names, err := ungatedIn(kept, open, []string{"anchor"}, ""); err != nil || strings.Join(names, ",") != "anchor" { t.Fatalf("the step's send is refused as a move: %v, %v", names, err) } + // Recorded as the send records it: anchor still runs restic c1, its gate still to come there. + record(declared) + if sent, _, err := inv.SentBuilds(ctx, "anchor"); err != nil || sent["restic"] != "c1" { + t.Fatalf("after the step anchor reads as sent restic %q (%v); want c1", sent["restic"], err) + } + if after, err := inv.AssignmentGeneration(ctx); err != nil || after != generation { + t.Fatalf("the step moved the assignment generation from %d to %d (%v)", generation, after, err) + } - // A module newly assigned to anchor: the step would install it, unjudged, so it is not sent. + // A module newly assigned to anchor: the step would install it, unjudged, so it is refused unsent. if _, err := inv.Assign(ctx, "anchor", "wallpaper"); err != nil { t.Fatal(err) } - only, err := grantsOnly(ctx, open, "anchor") + plan, declared = compose(kept) + only, err := grantsOnlyIn(ctx, open, "anchor", plan, declared) if err != nil { t.Fatal(err) } - if !strings.Contains(only, "wallpaper newly assigned") || !strings.Contains(only, "wallpaper") { + if !strings.Contains(only, "wallpaper newly assigned") || !strings.Contains(only, "+wallpaper") { t.Fatalf("a new assignment on the bus's machine reads %q; want the step refused, naming it", only) } } diff --git a/cmd/mesh-controller/grants_step_test.go b/cmd/mesh-controller/grants_step_test.go index fe128009..7592f8f1 100644 --- a/cmd/mesh-controller/grants_step_test.go +++ b/cmd/mesh-controller/grants_step_test.go @@ -3,6 +3,7 @@ package main import ( "context" "errors" + "fmt" "slices" "strings" "testing" @@ -32,9 +33,8 @@ type aSend struct { func sendsRecorded(t *testing.T, holder string, behind bool, refuse error) *[]aSend { t.Helper() var sends []aSend - wasSend, wasBehind, wasOnly := sendRollout, brokerBehindOf, grantsOnlyOf - t.Cleanup(func() { sendRollout, brokerBehindOf, grantsOnlyOf = wasSend, wasBehind, wasOnly }) - grantsOnlyOf = func(context.Context, *stores, string) (string, error) { return "", nil } + wasSend, wasBehind := sendRollout, brokerBehindOf + t.Cleanup(func() { sendRollout, brokerBehindOf = wasSend, wasBehind }) brokerBehindOf = func(_ context.Context, _ *stores, names []string) (string, bool, error) { if slices.Contains(names, holder) { return holder, false, nil @@ -123,18 +123,17 @@ func TestAGrantsStepThatFailsIsSaidAndTheSendGoesOn(t *testing.T) { } } -// A step whose send would change more than the user list is not sent: it fails, said, and the walk goes on. +// A step whose send would change more than the user list is refused unsent by the send (sendToEach): the +// gate says it FAILED with what differed, and the walk goes on. func TestAGrantsStepThatWouldCarryMoreIsNotSent(t *testing.T) { - sends := sendsRecorded(t, "novox", true, nil) - grantsOnlyOf = func(context.Context, *stores, string) (string, error) { - return "its send would change more than the bus's user list: -postgres.login-n8n", nil - } + refusal := fmt.Errorf("%w: its send would change more than the bus's user list: -postgres.login-n8n", errNotGrantsOnly) + sends := sendsRecorded(t, "novox", true, refusal) sent, grants, err := sendJudged(t.Context(), nil, "ace") if err != nil { t.Fatal(err) } - if len(*sends) != 1 || (*sends)[0].keepsAll || strings.Join(sent, ",") != "ace" { - t.Errorf("the walk made %+v, want ace's judged send alone", *sends) + if strings.Join(sent, ",") != "ace" || len(*sends) != 2 { + t.Errorf("the walk made %+v, want the refused step and then ace's judged send", *sends) } if grants == nil || grants.At != nil || !strings.Contains(grants.Failed, "-postgres.login-n8n") { t.Fatalf("the gate keeps %+v", grants) diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index 9f3c1434..30e3fd84 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -1236,6 +1236,17 @@ func sendToEach(ctx context.Context, open *stores, names []string) ([]string, er continue } numbered.stamp(&declared) + // **The grants step sends the user list and nothing else** (novox/hq issue 490): judged on this very + // declaration, the one sent, so nothing composed between a check and the send can slip through. + if everyBuildKept(ctx) { + other, err := grantsOnlyIn(ctx, open, name, plan, declared) + if err != nil { + return nil, err + } + if other != "" { + return nil, fmt.Errorf("%w: %s", errNotGrantsOnly, other) + } + } reportLeftOut(name, declared) sending = append(sending, readyNode{name, declared}) } diff --git a/cmd/mesh-controller/release.go b/cmd/mesh-controller/release.go index afc2db23..1b872036 100644 --- a/cmd/mesh-controller/release.go +++ b/cmd/mesh-controller/release.go @@ -968,19 +968,6 @@ func grantsStep(ctx context.Context, open *stores, node string) *inventory.Grant // list is the one already sent. return nil } - // **The user list and nothing else**: checked before it is sent, against what the machine was last sent. - // Kept builds are not the whole of a declaration — a new assignment, a settings change, an assignment - // taken away, a provision's logins are composed as the mesh holds them now — and a step that carried any - // of it would be the unjudged change this step exists to avoid. - if other, err := grantsOnlyOf(ctx, open, holder); err != nil || other != "" { - why := other - if err != nil { - why = err.Error() - } - fmt.Printf("grants step: %s is not sent its user list ahead of %s, which is sent without it: %s\n", - holder, node, why) - return &inventory.GrantsStep{Node: holder, Failed: why} - } if _, err := sendRollout(keepingEveryBuild(withScope(ctx, sendScope{})), open, []string{holder}); err != nil { fmt.Printf("grants step: the bus's user list could not be sent to %s ahead of %s, which is sent without "+ "it — the bus may refuse what the send newly grants: %v\n", holder, node, err) @@ -992,21 +979,19 @@ func grantsStep(ctx context.Context, open *stores, node string) *inventory.Grant return &inventory.GrantsStep{Node: holder, At: &now} } -// grantsOnlyOf is grantsOnly: a variable so a test of the walk needs no store. -var grantsOnlyOf = grantsOnly +// errNotGrantsOnly is a grants step refused unsent: its declaration would change more than the user list. +var errNotGrantsOnly = errors.New("the grants step would change more than the bus's user list, and is not sent") -// grantsOnly is what the grants step's send to the machine holding the bus would change there besides its -// user list, against what it was last sent (novox/hq issue 490); empty when nothing else. Composed as the -// step's send composes it, without allocating anything (Foreseeing), and compared resource by resource -// with the summary the last send kept (ADR 0217): exact, rather than a list of the cases that can differ. -// A module there that was never sent is a new assignment, and refused too, whatever its resources. -func grantsOnly(ctx context.Context, open *stores, holder string) (string, error) { +// grantsOnlyIn is what a grants step's declaration, composed for sending, would change on the machine holding +// the bus besides its user list, against what it was last sent (novox/hq issue 490); empty when nothing else. +// Judged by sendToEach on the very declaration it then sends — one composition, judged, then sent or refused +// unsent. Kept builds are not the whole of a declaration: a new assignment, a settings change, an assignment +// taken away, a provision's logins are composed as the mesh holds them now, and a step that carried any of +// it would be the unjudged change this step exists to avoid. Compared resource by resource with the summary +// the last send kept (ADR 0217): exact, rather than a list of the cases that can differ. A module there that +// was never sent is a new assignment, refused whatever its resources. +func grantsOnlyIn(ctx context.Context, open *stores, holder string, plan catalogue.Resolution, declared sendable) (string, error) { inv := open.inventory - kept := keepingEveryBuild(keepingRecorded(ctx)) - plan, settings, err := planFor(kept, open, holder) - if err != nil { - return "", err - } sent, known, err := inv.SentBuilds(ctx, holder) if err != nil { return "", err @@ -1024,13 +1009,22 @@ func grantsOnly(ctx context.Context, open *stores, holder string) (string, error listID = m.Module + "." + catalogue.BusUsersID() } } - gens, err := generators(kept, open) - if err != nil { - return "", err - } - declared, err := declarationWith(kept, open, holder, plan, settings, gens, Foreseeing) - if err != nil { - return "", err + var facts *moveFacts + for m, was := range sent { + now, carried := declared.Builds[m] + if !carried || sameCommit(now, was) { + continue + } + if facts == nil { + f, err := readMoveFacts(ctx, inv) + if err != nil { + return "", err + } + facts = &f + } + if !facts.identical(m, was, now) { + other = append(other, fmt.Sprintf("%s %s → %s", m, short(was), short(now))) + } } for _, f := range declared.foreseen { other = append(other, f+" would be minted") @@ -1067,6 +1061,7 @@ func grantsOnly(ctx context.Context, open *stores, holder string) (string, error } other = append(other, fmt.Sprintf("~%s (%s)", c.ID, strings.Join(c.Fields, ", "))) } + sort.Strings(other) if len(other) == 0 { return "", nil } From 0f853e93fa11aee77f74cab9934d6943fc173338 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 19:17:28 +0200 Subject: [PATCH 4/6] Exempt only the user list's content from the grants step's guard, and test the guard in the send itself (review of #230) The guard let any field of the list's resource move; its path or mode changing is a change the step must not carry. The store test now calls sendToEach, so removing the guard there fails it. --- cmd/mesh-controller/grants_step_store_test.go | 8 ++++++ cmd/mesh-controller/grants_step_test.go | 28 +++++++++++++++++++ cmd/mesh-controller/release.go | 22 ++++++++++----- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/cmd/mesh-controller/grants_step_store_test.go b/cmd/mesh-controller/grants_step_store_test.go index ae01d6e7..7f675c07 100644 --- a/cmd/mesh-controller/grants_step_store_test.go +++ b/cmd/mesh-controller/grants_step_store_test.go @@ -3,11 +3,13 @@ package main import ( "context" "encoding/json" + "errors" "strings" "testing" "time" "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/identity" "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" ) @@ -169,4 +171,10 @@ func TestTheGrantsStepSendsTheUserListAndNothingElse(t *testing.T) { if !strings.Contains(only, "wallpaper newly assigned") || !strings.Contains(only, "+wallpaper") { t.Fatalf("a new assignment on the bus's machine reads %q; want the step refused, naming it", only) } + // And the send itself refuses it, unsent: judged where it is composed, before the bus is dialled. + identity.ForTest(t) + if _, err := sendToEach(kept, open, []string{"anchor"}); !errors.Is(err, errNotGrantsOnly) || + !strings.Contains(err.Error(), "wallpaper") { + t.Fatalf("the grants step's send of a new assignment was not refused by the send: %v", err) + } } diff --git a/cmd/mesh-controller/grants_step_test.go b/cmd/mesh-controller/grants_step_test.go index 7592f8f1..2820bb51 100644 --- a/cmd/mesh-controller/grants_step_test.go +++ b/cmd/mesh-controller/grants_step_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "encoding/json" "errors" "fmt" "slices" @@ -140,6 +141,33 @@ func TestAGrantsStepThatWouldCarryMoreIsNotSent(t *testing.T) { } } +// The step's exemption is the list's content alone: its path or mode moving is a change like any other. +func TestOnlyTheUserListsContentIsExempt(t *testing.T) { + list := "nats.bus-users" + file := func(fields map[string]any) sentResource { + r := map[string]any{"id": list, "type": "file", "path": "/conf/accounts.conf", "mode": "0600", "content": "a"} + for k, v := range fields { + r[k] = v + } + body, _ := json.Marshal(map[string]any{"resources": []any{r}}) + s, err := summarize(body) + if err != nil { + t.Fatal(err) + } + return s[0] + } + was := file(nil) + if other := otherThanTheList(diffSent([]sentResource{was}, []sentResource{file(map[string]any{"content": "b"})}), list); len(other) != 0 { + t.Errorf("the list's new content reads as more: %v", other) + } + for _, f := range []map[string]any{{"mode": "0644"}, {"path": "/elsewhere"}, {"content": "b", "owner": "nats"}} { + other := otherThanTheList(diffSent([]sentResource{was}, []sentResource{file(f)}), list) + if len(other) != 1 || !strings.HasPrefix(other[0], "~"+list) { + t.Errorf("the list's resource changed by %v reads %v; want it refused", f, other) + } + } +} + // The delivery plan says the step before the merge. func TestAChangePlanSaysTheGrantsStep(t *testing.T) { const catalogue_ = "http://forge.internal:20000/novox/mesh-catalog.git" diff --git a/cmd/mesh-controller/release.go b/cmd/mesh-controller/release.go index 1b872036..8be20f46 100644 --- a/cmd/mesh-controller/release.go +++ b/cmd/mesh-controller/release.go @@ -1048,7 +1048,19 @@ func grantsOnlyIn(ctx context.Context, open *stores, holder string, plan catalog if err := json.Unmarshal(prev, &before); err != nil { return "", fmt.Errorf("what %s was last sent is kept in a form this version does not read: %w", holder, err) } - d := diffSent(before, after) + other = append(other, otherThanTheList(diffSent(before, after), listID)...) + sort.Strings(other) + if len(other) == 0 { + return "", nil + } + return "its send would change more than the bus's user list: " + strings.Join(other, "; "), nil +} + +// otherThanTheList is what a diff against the last send changes besides the user list's content: every +// resource added, removed or changed, but the list's own resource when its content is all that changed — +// its path, mode and every other field must be as last sent (novox/hq issue 490). +func otherThanTheList(d sentDiff, listID string) []string { + var other []string for _, id := range d.Added { other = append(other, "+"+id) } @@ -1056,16 +1068,12 @@ func grantsOnlyIn(ctx context.Context, open *stores, holder string, plan catalog other = append(other, "-"+id) } for _, c := range d.Changed { - if c.ID == listID { + if c.ID == listID && len(c.Fields) == 1 && c.Fields[0] == "content" { continue } other = append(other, fmt.Sprintf("~%s (%s)", c.ID, strings.Join(c.Fields, ", "))) } - sort.Strings(other) - if len(other) == 0 { - return "", nil - } - return "its send would change more than the bus's user list: " + strings.Join(other, "; "), nil + return other } // sendJudged is a gated send's sends: the grants step when the user list changed, then the machine judged. From 325575b2929c93935218a504b96a03921eac68a0 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 19:30:10 +0200 Subject: [PATCH 5/6] Keep the builds of the grants step's machine alone, not of every machine its composition resolves (repo-check of #230) Composing the bus's machine resolves the others on the same context, and a machine never recorded as sent refused the whole composition. --- cmd/mesh-controller/grants_step_store_test.go | 5 ++- cmd/mesh-controller/grants_step_test.go | 11 ++++++- cmd/mesh-controller/push.go | 2 +- cmd/mesh-controller/recorded_kept.go | 31 +++++++++++-------- cmd/mesh-controller/release.go | 4 +-- 5 files changed, 35 insertions(+), 18 deletions(-) diff --git a/cmd/mesh-controller/grants_step_store_test.go b/cmd/mesh-controller/grants_step_store_test.go index 7f675c07..3248a1b4 100644 --- a/cmd/mesh-controller/grants_step_store_test.go +++ b/cmd/mesh-controller/grants_step_store_test.go @@ -105,7 +105,10 @@ func TestTheGrantsStepSendsTheUserListAndNothingElse(t *testing.T) { t.Fatal(err) } } - kept := keepingEveryBuild(keepingRecorded(ctx)) + // Kept for anchor alone. laptop is never recorded as sent, on purpose: composing anchor resolves laptop + // too (who is on the private network), and a step that kept every machine's builds refused anchor's + // composition for want of laptop's last send (repo-check of #230 at 0f853e93). + kept := keepingEveryBuild(keepingRecorded(ctx), "anchor") // What anchor was last sent: everything at the builds of before the merge, as an ordinary send sends it. _, before := compose(keepingRecorded(ctx)) diff --git a/cmd/mesh-controller/grants_step_test.go b/cmd/mesh-controller/grants_step_test.go index 2820bb51..48b34bb5 100644 --- a/cmd/mesh-controller/grants_step_test.go +++ b/cmd/mesh-controller/grants_step_test.go @@ -43,7 +43,7 @@ func sendsRecorded(t *testing.T, holder string, behind bool, refuse error) *[]aS return holder, behind, nil } sendRollout = func(ctx context.Context, _ *stores, names []string) ([]string, error) { - s := aSend{names: append([]string(nil), names...), keepsAll: everyBuildKept(ctx)} + s := aSend{names: append([]string(nil), names...), keepsAll: len(names) == 1 && everyBuildKept(ctx, names[0])} for n := range scopeOf(ctx).judged { s.judged = append(s.judged, n) } @@ -168,6 +168,15 @@ func TestOnlyTheUserListsContentIsExempt(t *testing.T) { } } +// The step keeps the builds of the machine it sends alone: the others its composition resolves are composed +// as any send composes them. +func TestTheGrantsStepKeepsOneMachinesBuilds(t *testing.T) { + ctx := keepingEveryBuild(t.Context(), "novox") + if !everyBuildKept(ctx, "novox") || everyBuildKept(ctx, "ace") || everyBuildKept(t.Context(), "novox") { + t.Error("the grants step keeps the builds of a machine it does not send") + } +} + // The delivery plan says the step before the merge. func TestAChangePlanSaysTheGrantsStep(t *testing.T) { const catalogue_ = "http://forge.internal:20000/novox/mesh-catalog.git" diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index 30e3fd84..2e134b15 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -1238,7 +1238,7 @@ func sendToEach(ctx context.Context, open *stores, names []string) ([]string, er numbered.stamp(&declared) // **The grants step sends the user list and nothing else** (novox/hq issue 490): judged on this very // declaration, the one sent, so nothing composed between a check and the send can slip through. - if everyBuildKept(ctx) { + if everyBuildKept(ctx, name) { other, err := grantsOnlyIn(ctx, open, name, plan, declared) if err != nil { return nil, err diff --git a/cmd/mesh-controller/recorded_kept.go b/cmd/mesh-controller/recorded_kept.go index 5c81bec8..a8e8cf5b 100644 --- a/cmd/mesh-controller/recorded_kept.go +++ b/cmd/mesh-controller/recorded_kept.go @@ -63,19 +63,24 @@ func keptExcept(ctx context.Context) (map[string]bool, bool) { type keepEveryBuildKey struct{} -// keepingEveryBuild is a context whose sends compose every module — recorded or rolling out — at the build -// its machine was last sent (novox/hq issue 490): the grants step, which sends the machine holding the bus -// its new user list and nothing of any module's new code. That code waits there for a gate like any other -// move; the list must not, or the first machine's gate is judged against a bus that refuses what the change -// newly grants. -func keepingEveryBuild(ctx context.Context) context.Context { - return context.WithValue(ctx, keepEveryBuildKey{}, true) +// keepingEveryBuild is a context whose sends compose every module of one machine — recorded or rolling out — +// at the build that machine was last sent (novox/hq issue 490): the grants step, which sends the machine +// holding the bus its new user list and nothing of any module's new code. That code waits there for a gate +// like any other move; the list must not, or the first machine's gate is judged against a bus that refuses +// what the change newly grants. +// +// **That machine alone.** Composing one machine resolves the others too — who is on the private network, +// who answers a requirement — on the same context, and those are no part of the step's send: they are +// composed as any send composes them. Kept for every machine, a machine whose last send is not known +// refused the bus's machine's composition. +func keepingEveryBuild(ctx context.Context, node string) context.Context { + return context.WithValue(ctx, keepEveryBuildKey{}, node) } -// everyBuildKept is whether this context keeps every module at the build its machine runs. -func everyBuildKept(ctx context.Context) bool { - on, _ := ctx.Value(keepEveryBuildKey{}).(bool) - return on +// everyBuildKept is whether this context keeps every module of this machine at the build it runs. +func everyBuildKept(ctx context.Context, node string) bool { + on, _ := ctx.Value(keepEveryBuildKey{}).(string) + return on != "" && on == node } // recordedKept is, for a send under keepingRecorded, every recorded module the machine was last sent a @@ -87,7 +92,7 @@ func everyBuildKept(ctx context.Context) bool { // 490), and a machine whose last send is not known refuses the send: there is nothing to keep it at, and // composing the mesh's builds would be the very move the grants step must not make. func recordedKept(ctx context.Context, open *stores, node string) (map[string]string, error) { - every := everyBuildKept(ctx) + every := everyBuildKept(ctx, node) skip, on := keptExcept(ctx) if !on && !every { return nil, nil @@ -150,7 +155,7 @@ func keepRecorded(ctx context.Context, open *stores, node string, shelf map[stri if err != nil { return nil, err } - if !found && everyBuildKept(ctx) { + if !found && everyBuildKept(ctx, node) { return nil, fmt.Errorf("%s runs %s's build %s, which the build records no longer hold: the bus's user "+ "list cannot be sent there alone with it kept (novox/hq issue 490)", node, m, short(kept[m])) } diff --git a/cmd/mesh-controller/release.go b/cmd/mesh-controller/release.go index 8be20f46..555ebeb2 100644 --- a/cmd/mesh-controller/release.go +++ b/cmd/mesh-controller/release.go @@ -257,7 +257,7 @@ func ungatedIn(ctx context.Context, open *stores, names []string, addedHolder st // The grants step composes every module at the build its machine was last sent (novox/hq issue 490): // a move it keeps is not made. Read, not assumed, so a move it could not keep is still refused here. var keeps map[string]string - if everyBuildKept(ctx) { + if everyBuildKept(ctx, n) { if keeps, err = recordedKept(ctx, open, n); err != nil { return nil, err } @@ -968,7 +968,7 @@ func grantsStep(ctx context.Context, open *stores, node string) *inventory.Grant // list is the one already sent. return nil } - if _, err := sendRollout(keepingEveryBuild(withScope(ctx, sendScope{})), open, []string{holder}); err != nil { + if _, err := sendRollout(keepingEveryBuild(withScope(ctx, sendScope{}), holder), open, []string{holder}); err != nil { fmt.Printf("grants step: the bus's user list could not be sent to %s ahead of %s, which is sent without "+ "it — the bus may refuse what the send newly grants: %v\n", holder, node, err) return &inventory.GrantsStep{Node: holder, Failed: err.Error()} From eb8233ac7ca8c953b81a6deea71ef203fe8c182a Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 19:43:50 +0200 Subject: [PATCH 6/6] Give the grants step's restic fixture a check beside its tool, as the module rules require (repo-check of #230) A tool check alone is refused, so the store test's c2 never registered; the fixtures are now parsed by a test that needs no store. --- cmd/mesh-controller/grants_step_store_test.go | 53 ++++++++++++------- cmd/mesh-controller/grants_step_test.go | 16 ++++++ 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/cmd/mesh-controller/grants_step_store_test.go b/cmd/mesh-controller/grants_step_store_test.go index 3248a1b4..095a84f1 100644 --- a/cmd/mesh-controller/grants_step_store_test.go +++ b/cmd/mesh-controller/grants_step_store_test.go @@ -26,7 +26,6 @@ func TestTheGrantsStepSendsTheUserListAndNothingElse(t *testing.T) { ctx := t.Context() inv := open.inventory start := time.Now().Add(-time.Hour) - image := "registry.invalid:5000/restic/backup@sha256:" + strings.Repeat("b", 64) build := func(module, commit string, at time.Time, manifest map[string]any) link.BuildResult { manifest["module"], manifest["version"] = module, "1" raw, _ := json.Marshal(manifest) @@ -34,24 +33,10 @@ func TestTheGrantsStepSendsTheUserListAndNothingElse(t *testing.T) { On: "anchor", Module: module, Commit: commit, Manifest: raw, Source: &link.SourceOnSeat{Seat: "git", Repository: "novox/mesh-catalog"}} } - restic := func(health map[string]any, tools ...string) map[string]any { - r := map[string]any{"id": "backup", "type": "container", "name": "restic-backup", "image": image} - if health != nil { - r["health"] = health - } - m := map[string]any{"resources": []any{r}} - if len(tools) > 0 { - m["tools"] = tools - } - return m - } - nats := map[string]any{"bus-users": "/var/lib/nats-module/conf/accounts.conf", - "claims": []any{map[string]any{"name": catalogue.BrokerSeat, "scope": catalogue.ScopeMesh}}} for _, b := range []link.BuildResult{ - build("nats", "n1", start, nats), - build("restic", "c1", start.Add(time.Second), restic(nil)), - build("wallpaper", "w1", start.Add(2*time.Second), map[string]any{"resources": []any{ - map[string]any{"id": "paper", "type": "container", "name": "wallpaper", "image": image}}}), + build("nats", "n1", start, natsFixture()), + build("restic", "c1", start.Add(time.Second), resticFixture(false)), + build("wallpaper", "w1", start.Add(2*time.Second), wallpaperFixture()), } { if _, _, err := takeIn(ctx, inv, asTheOperator(t, inv, b)); err != nil { t.Fatal(err) @@ -120,7 +105,7 @@ func TestTheGrantsStepSendsTheUserListAndNothingElse(t *testing.T) { // The merge: restic's c2 gives it a health tool. if _, _, err := takeIn(ctx, inv, build("restic", "c2", start.Add(time.Minute), - restic(map[string]any{"kind": catalogue.HealthTool, "tool": "backup_health"}, "backup_health"))); err != nil { + resticFixture(true))); err != nil { t.Fatal(err) } @@ -181,3 +166,33 @@ func TestTheGrantsStepSendsTheUserListAndNothingElse(t *testing.T) { t.Fatalf("the grants step's send of a new assignment was not refused by the send: %v", err) } } + +// fixtureImage is the image every fixture container of the grants step's tests runs. +var fixtureImage = "registry.invalid:5000/restic/backup@sha256:" + strings.Repeat("b", 64) + +// resticFixture is restic's manifest in the shape of mesh-catalog #210: at c2 (withTool) its backup judged by +// its new tool `backup_health`, beside a check of another kind it does not run itself — a tool check alone +// is refused (ADR 0227 rule 8, ADR 0240 rule 2) — and at c1 neither. +func resticFixture(withTool bool) map[string]any { + backup := map[string]any{"id": "backup", "type": "container", "name": "restic-backup", "image": fixtureImage} + measure := map[string]any{"id": "measure", "type": "container", "name": "restic-measure", "image": fixtureImage} + m := map[string]any{"resources": []any{backup, measure}} + if withTool { + backup["health"] = map[string]any{"kind": catalogue.HealthTool, "tool": "backup_health"} + measure["health"] = map[string]any{"kind": "runtime"} + m["tools"] = []string{"backup_health"} + } + return m +} + +// natsFixture is the bus's module: it holds mesh-broker and is sent the user list. +func natsFixture() map[string]any { + return map[string]any{"bus-users": "/var/lib/nats-module/conf/accounts.conf", + "claims": []any{map[string]any{"name": catalogue.BrokerSeat, "scope": catalogue.ScopeMesh}}} +} + +// wallpaperFixture is a module the bus's machine is newly assigned. +func wallpaperFixture() map[string]any { + return map[string]any{"resources": []any{ + map[string]any{"id": "paper", "type": "container", "name": "wallpaper", "image": fixtureImage}}} +} diff --git a/cmd/mesh-controller/grants_step_test.go b/cmd/mesh-controller/grants_step_test.go index 48b34bb5..70e275b2 100644 --- a/cmd/mesh-controller/grants_step_test.go +++ b/cmd/mesh-controller/grants_step_test.go @@ -177,6 +177,22 @@ func TestTheGrantsStepKeepsOneMachinesBuilds(t *testing.T) { } } +// The store test's manifests are manifests the controller takes in: checked here, where no store is needed, +// so a fixture the module rules refuse fails before the store test is run (repo-check of #230 at 325575b2). +func TestTheGrantsStepFixturesAreManifests(t *testing.T) { + for name, m := range map[string]map[string]any{"nats": natsFixture(), "restic-c1": resticFixture(false), + "restic-c2": resticFixture(true), "wallpaper": wallpaperFixture()} { + m["module"], m["version"] = strings.SplitN(name, "-", 2)[0], "1" + raw, err := json.Marshal(m) + if err != nil { + t.Fatal(err) + } + if _, err := catalogue.ParseManifest(raw); err != nil { + t.Errorf("%s: %v", name, err) + } + } +} + // The delivery plan says the step before the merge. func TestAChangePlanSaysTheGrantsStep(t *testing.T) { const catalogue_ = "http://forge.internal:20000/novox/mesh-catalog.git"