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)