From 054754482cf4fe6f5449dce0484492f9905dc8de Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 01:53:29 +0100 Subject: [PATCH] The running provisioner account is kept only for a client held this pass, and rows hold the guard (issue 774, review) The review removed the guard and the suite still passed. The guard is now provisionerHeld, a placed client held as its stand-in, rather than any placed client, so one placed and not held that composes no account keeps none stale. Rows in the held client's table: the client taken out of the node's placement while what runs names the account, read from the running own composition and from the client's running process, names no account; a placed client whose process names no user names none. With the guard removed the first three fail; with it reduced to "placed", the last. Claude-Session: https://claude.ai/code/session_01JG73KG7AfSdh6FP8VrBqCn --- .../frombus/frombustest/frombustest_test.go | 55 ++++++++++++++++--- internal/compose/rendering.go | 16 +++--- 2 files changed, 54 insertions(+), 17 deletions(-) diff --git a/internal/compose/frombus/frombustest/frombustest_test.go b/internal/compose/frombus/frombustest/frombustest_test.go index eb5eca8..4264ff3 100644 --- a/internal/compose/frombus/frombustest/frombustest_test.go +++ b/internal/compose/frombus/frombustest/frombustest_test.go @@ -2,7 +2,9 @@ package frombustest import ( "context" + "encoding/json" "reflect" + "slices" "strings" "testing" @@ -223,7 +225,11 @@ func TestTheProvisionedMeshHandsAPlanWithItsProvision(t *testing.T) { for _, c := range before { running = append(running, c) } - for _, own := range []bool{true, false} { + for _, row := range []struct { + own, placed bool // the account read from the running own composition, else the client's running process; the client placed + userless bool // the client placed and not held, its process naming no user: it composes no account + }{{true, true, false}, {false, true, false}, {true, false, false}, {false, false, false}, {true, true, true}} { + own := row.own runs := running if !own { // what runs names no account of its own: the client's running process says it runs = nil @@ -234,16 +240,47 @@ func TestTheProvisionedMeshHandsAPlanWithItsProvision(t *testing.T) { runs = append(runs, c) } } - b.Builds["mesh-provisioner-client "+fps["mesh-provisioner-client"]] = &delivery.BuilderBuild{State: delivery.BuildBuilding} - after, _, held, err := frombus.Compose(context.Background(), b, frombus.Mesh{}, frombus.Scope{Node: Node, Running: runs}, fps) - if _, isHeld := held["mesh-provisioner-client"]; err != nil || !isHeld { - t.Fatalf("the client with no done build is not held: %v %v", held, err) + bb, err := Provisioned() + if err != nil { + t.Fatal(err) } - if got := after[ownKey].Own.Provisioner; got != account { - t.Errorf("the client held (account from its own composition: %v): the node's own names %q, want %q", own, got, account) + want := account + if row.userless { + // placed and not held, composing no account of its own: no stale account is kept from what runs + mod := bb.Module["mesh-provisioner-client"] + var m map[string]any + if err := json.Unmarshal(bb.Manifests[mod.Manifest], &m); err != nil { + t.Fatal(err) + } + for _, r := range m["resources"].([]any) { + delete(r.(map[string]any), "user") + } + raw, _ := json.Marshal(m) + mod.Manifest = delivery.ManifestDigest(raw) + bb.Module["mesh-provisioner-client"], bb.Manifests[mod.Manifest] = mod, raw + built := *bb.Builds["mesh-provisioner-client "+fps["mesh-provisioner-client"]] + built.Manifest = mod.Manifest + bb.Builds["mesh-provisioner-client "+fps["mesh-provisioner-client"]] = &built + want = "" + } else if row.placed { + // held: its build not done while it runs + bb.Builds["mesh-provisioner-client "+fps["mesh-provisioner-client"]] = &delivery.BuilderBuild{State: delivery.BuildBuilding} + } else { + // taken out of the node's placement while what runs still names it: the account goes with it, never kept stale + p := bb.Placement[Node] + p.Modules = slices.DeleteFunc(slices.Clone(p.Modules), func(m delivery.PlacedModule) bool { return m.Module == "mesh-provisioner-client" }) + bb.Placement[Node] = p + want = "" } - if !reflect.DeepEqual(after[resticKey].Filled, before[resticKey].Filled) { - t.Errorf("the client held (account from its own composition: %v): restic's filled files %v, want %v", own, after[resticKey].Filled, before[resticKey].Filled) + after, _, held, err := frombus.Compose(context.Background(), bb, frombus.Mesh{}, frombus.Scope{Node: Node, Running: runs}, fps) + if _, isHeld := held["mesh-provisioner-client"]; err != nil || isHeld != (row.placed && !row.userless) { + t.Fatalf("%+v: held %v: %v", row, held, err) + } + if got := after[ownKey].Own.Provisioner; got != want { + t.Errorf("%+v: the node's own names %q, want %q", row, got, want) + } + if row.placed && !row.userless && !reflect.DeepEqual(after[resticKey].Filled, before[resticKey].Filled) { + t.Errorf("%+v: restic's filled files %v, want %v", row, after[resticKey].Filled, before[resticKey].Filled) } } } diff --git a/internal/compose/rendering.go b/internal/compose/rendering.go index c6b195b..7aaae27 100644 --- a/internal/compose/rendering.go +++ b/internal/compose/rendering.go @@ -74,10 +74,10 @@ func (c *composer) places(nodeName string, resolved Resolved) (Placed, error) { // not sent the field they would refuse. r.Hands = takesHanded(node) r.Provisioner = provisionerAccount(plan) - if r.Provisioner == "" && provisionerPlaced(plan) { - // The provisioner's client is placed here but composes no account this pass (held as its stub, which runs no - // process of its own composition): it keeps running as it runs, so the account it runs as stays named, and every - // module stays handed to it. + if r.Provisioner == "" && provisionerHeld(plan) { + // The provisioner's client is placed here and held this pass (its stub, or carried as it runs, composes no process + // of its own): it keeps running as it runs, so the account it runs as stays named, and every module stays handed + // to it. A client not held that composes no account, or none placed, names none: no stale account is kept. r.Provisioner = node.RunningProvisioner } r.Assigned = node.Handed @@ -196,11 +196,11 @@ func provisionerAccount(plan catalogue.Resolution) string { return "" } -// provisionerPlaced says the plan places a module claiming the provisioner client's seat on the machine, whether or not -// it composes the client's process this pass. -func provisionerPlaced(plan catalogue.Resolution) bool { +// provisionerHeld says the plan places a module claiming the provisioner client's seat on the machine that this pass +// holds (its stand-in: Stub, or Carried as it runs), which composes none of the client's process. +func provisionerHeld(plan catalogue.Resolution) bool { for _, m := range plan.Modules { - if m.ClaimsSeat(NodeProvisionerSeat) { + if m.Stubbed && m.ClaimsSeat(NodeProvisionerSeat) { return true } }