Mint an assignment's account once, whoever finds it missing first (issue 1945, ADR 0543, review)

The assign act and the assignments' watch both checked whether an account was minted and then minted it, and minting overwrites the hash: interleaved, the bus kept one caller's hash and the node was sealed the other's password, the crash loop this branch fixes. Both now mint through one statement that inserts the row only when none stands; the caller whose insert wins seals, the other seals nothing. issueMissingProcessAccounts mints the same way. A watch opened again after one ended handed the standing assignments as the start's look, so one that came to stand in the gap got no bus composition or grant when its account already stood: only the process's first watch is the start's look now. The start's look reads the catalogue once and ends with one composition of the bus's node and the grants; a controller not holding the lease is told so and tries again rather than dropping the key. Tests folded to stay within the budget: the assignment tests are three table tests, the process account and mayIssue tests folded into one of them, and the concurrent mint is a case of the inventory's rotation test. Research 072: the grants piece (memberships and credentials heard from placement).
2026-10-25 17:51:06 +01:00
parent ca5f74eadb
commit 9ff47f6aaa
8 changed files with 389 additions and 223 deletions
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"strings"
 	"testing"
 	"time"
@@ -27,8 +28,11 @@ func anAssignmentValue(t *testing.T, a chain.Assignment) []byte {
 }

 // Only a write that makes the key stand turns it: a rewrite of a standing assignment (the provisioner's plan kept in its
-// value), an ended one, a pending one or a module only pulled in by another does not.
-func TestAnAssignmentTurnsWhenItComesToStand(t *testing.T) {
+// value), an ended one, a pending one or a module only pulled in by another does not. The process's first watch hands
+// the assignments standing when it starts to the start's look (not fresh), says once that they are all read, and hands
+// each assignment written afterwards as fresh; a watch opened again after one ended hands every standing assignment it
+// has not seen stand as fresh and never calls the start's look (the review of issue 1945).
+func TestAnAssignmentThatComesToStand(t *testing.T) {
 	at := time.Date(2026, 10, 25, 16, 19, 42, 0, time.UTC)
 	stands := chain.Assignment{Node: "ace", Module: "mesh-build-node", Assigned: at}
 	ended := stands
@@ -43,7 +47,7 @@ func TestAnAssignmentTurnsWhenItComesToStand(t *testing.T) {
 		value   *chain.Assignment // nil for a delete
 		turning bool
 	}
-	cases := []struct {
+	for _, c := range []struct {
 		name   string
 		writes []write
 	}{
@@ -54,8 +58,7 @@ func TestAnAssignmentTurnsWhenItComesToStand(t *testing.T) {
 		{"a pending one does not, and turns once made", []write{{&pending, false}, {&stands, true}}},
 		{"a module only pulled in does not", []write{{&pulled, false}}},
 		{"one deleted and written again turns again", []write{{&stands, true}, {nil, false}, {&stands, true}}},
-	}
-	for _, c := range cases {
+	} {
 		t.Run(c.name, func(t *testing.T) {
 			turns := &assignmentTurns{}
 			for i, w := range c.writes {
@@ -73,50 +76,119 @@ func TestAnAssignmentTurnsWhenItComesToStand(t *testing.T) {
 			}
 		})
 	}
+
+	type handed struct {
+		key   string
+		fresh bool
+	}
+	for _, first := range []bool{true, false} {
+		t.Run(fmt.Sprintf("the watch, first %v", first), func(t *testing.T) {
+			bucket := chain.NewMemory()
+			put := func(a chain.Assignment) {
+				t.Helper()
+				value := anAssignmentValue(t, a)
+				if e, held, _ := bucket.Get(t.Context(), a.Key()); held {
+					if _, err := bucket.Update(t.Context(), a.Key(), value, e.Revision); err != nil {
+						t.Fatal(err)
+					}
+					return
+				}
+				if _, err := bucket.Create(t.Context(), a.Key(), value); err != nil {
+					t.Fatal(err)
+				}
+			}
+			put(chain.Assignment{Node: "g14", Module: "mesh-build-go", Assigned: at})
+			got := make(chan handed, 8)
+			read := make(chan struct{}, 8)
+			ctx, stop := context.WithCancel(t.Context())
+			done := make(chan error, 1)
+			go func() {
+				done <- watchAssignmentsOnce(ctx, bucket.Watch, &assignmentTurns{}, first,
+					func(a chain.Assignment, fresh bool) { got <- handed{a.Key(), fresh} },
+					func() { read <- struct{}{} })
+			}()
+			next := func() handed {
+				t.Helper()
+				select {
+				case h := <-got:
+					return h
+				case <-time.After(5 * time.Second):
+					t.Fatal("nothing handed")
+					return handed{}
+				}
+			}
+			// Standing when the watch opened: the start's look on the first watch, fresh on any other.
+			if h := next(); h != (handed{"g14.mesh-build-go", !first}) {
+				t.Fatalf("the standing assignment was handed %+v", h)
+			}
+			put(chain.Assignment{Node: "g14", Module: "mesh-build-go", Assigned: at, Provisioning: json.RawMessage(`{"plan":2}`)})
+			put(chain.Assignment{Node: "ace", Module: "mesh-build-node", Assigned: at})
+			if h := next(); h != (handed{"ace.mesh-build-node", true}) {
+				t.Fatalf("handed %+v, want ace.mesh-build-node as fresh (and nothing for the rewrite before it)", h)
+			}
+			stop()
+			if err := <-done; err != nil {
+				t.Fatal(err)
+			}
+			if want := map[bool]int{true: 1, false: 0}[first]; len(read) != want {
+				t.Fatalf("the start's look was called %d time(s), want %d", len(read), want)
+			}
+		})
+	}
 }

 // settle issues what is missing and then puts it on the bus: the node holding the bus composed (its user list) and the
-// grants written; for an assignment that just came to stand even when its accounts were minted before (a module
-// assigned again keeps its account), and never by a controller that does not hold the lease.
-func TestSettlingAnAssignmentIssuesComposesTheBusAndGrants(t *testing.T) {
+// grants written, even when every account was minted before (a module assigned again keeps its account). The start's
+// look reads the catalogue once, issues for each standing assignment, and composes the bus's node and writes the grants
+// once when it looked at any. Neither acts for a controller that does not hold the lease, nor for the controller's own
+// module.
+func TestSettlingAndTheStartsLook(t *testing.T) {
+	ace := func(m string) chain.Assignment { return chain.Assignment{Node: "ace", Module: m} }
 	cases := []struct {
 		name               string
+		look               []chain.Assignment // nil: settle mesh-build-node on ace
 		module             string
-		holds, fresh       bool
+		notHolding         bool
 		issued             bool
 		issueErr, composeE error
 		holder             string
+		wantIssues         int
 		wantComposed       []string
 		wantGrants         bool
 		wantErr            string
 	}{
-		{name: "a new assignment whose account is not minted", module: "mesh-build-node", holds: true, fresh: true,
-			issued: true, holder: "novox", wantComposed: []string{"novox"}, wantGrants: true},
-		{name: "a new assignment whose account was minted before", module: "mesh-build-go", holds: true, fresh: true,
-			holder: "novox", wantComposed: []string{"novox"}, wantGrants: true},
-		{name: "the start's look issuing a missing account", module: "mesh-build-node", holds: true, issued: true,
-			holder: "novox", wantComposed: []string{"novox"}, wantGrants: true},
-		{name: "the start's look with every account minted does nothing", module: "mesh-build-go", holds: true,
-			holder: "novox"},
-		{name: "no lease, nothing", module: "mesh-build-node", fresh: true, issued: true, holder: "novox"},
-		{name: "the controller's own module is the terminal's", module: controllerModule, holds: true, fresh: true,
-			issued: true, holder: "novox"},
-		{name: "no bus holder: the grants still", module: "mesh-build-node", holds: true, fresh: true, issued: true,
-			wantGrants: true},
-		{name: "an issue that fails is the error, nothing composed", module: "mesh-build-node", holds: true, fresh: true,
-			issueErr: errors.New("the bus cannot be reached"), holder: "novox", wantErr: "the bus cannot be reached"},
-		{name: "a composition that fails is the error, no grant", module: "mesh-build-node", holds: true, fresh: true,
-			issued: true, holder: "novox", composeE: errors.New("store refused"), wantComposed: []string{"novox"},
-			wantErr: "store refused"},
+		{name: "an account not minted", module: "mesh-build-node", issued: true, holder: "novox", wantIssues: 1,
+			wantComposed: []string{"novox"}, wantGrants: true},
+		{name: "an account minted before: the bus and the grants still", module: "mesh-build-go", holder: "novox",
+			wantIssues: 1, wantComposed: []string{"novox"}, wantGrants: true},
+		{name: "no lease: nothing, and tried again", module: "mesh-build-node", notHolding: true, issued: true,
+			holder: "novox", wantErr: errNotHolding.Error()},
+		{name: "the controller's own module is the terminal's", module: controllerModule, issued: true, holder: "novox"},
+		{name: "no bus holder: the grants still", module: "mesh-build-node", issued: true, wantIssues: 1, wantGrants: true},
+		{name: "an issue that fails is the error, nothing composed", module: "mesh-build-node",
+			issueErr: errors.New("the bus cannot be reached"), holder: "novox", wantIssues: 1, wantErr: "the bus cannot be reached"},
+		{name: "a composition that fails is the error, no grant", module: "mesh-build-node", issued: true, holder: "novox",
+			composeE: errors.New("store refused"), wantIssues: 1, wantComposed: []string{"novox"}, wantErr: "store refused"},
+		{name: "the start's look: each issued, the bus and the grants once",
+			look: []chain.Assignment{ace("mesh-build-node"), ace(controllerModule), ace("mesh-build-go")}, issued: true,
+			holder: "novox", wantIssues: 2, wantComposed: []string{"novox"}, wantGrants: true},
+		{name: "the start's look with every account minted still covers once",
+			look: []chain.Assignment{ace("mesh-build-node"), ace("mesh-build-go")}, holder: "novox", wantIssues: 2,
+			wantComposed: []string{"novox"}, wantGrants: true},
+		{name: "the start's look at nothing does nothing", look: []chain.Assignment{ace(controllerModule)}, holder: "novox"},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
 			var composed []string
-			issuedFor, grants := "", false
+			issues, catalogues, grants := 0, 0, 0
 			steps := assignmentAccountSteps{
-				holds: func(context.Context) bool { return c.holds },
-				issue: func(_ context.Context, node, module string) (bool, string, error) {
-					issuedFor = module + "@" + node
+				holds: func(context.Context) bool { return !c.notHolding },
+				catalogue: func(context.Context) (map[string]catalogue.Manifest, error) {
+					catalogues++
+					return map[string]catalogue.Manifest{}, nil
+				},
+				issue: func(context.Context, map[string]catalogue.Manifest, string, string) (bool, string, error) {
+					issues++
 					if c.issued {
 						return true, "its bus credential issued", c.issueErr
 					}
@@ -127,9 +199,15 @@ func TestSettlingAnAssignmentIssuesComposesTheBusAndGrants(t *testing.T) {
 					composed = append(composed, nodes...)
 					return c.composeE
 				},
-				grants: func(context.Context) error { grants = true; return nil },
+				grants: func(context.Context) error { grants++; return nil },
+			}
+			var said string
+			var err error
+			if c.look != nil {
+				said, err = steps.look(t.Context(), c.look)
+			} else {
+				said, err = steps.settle(t.Context(), "ace", c.module)
 			}
-			said, err := steps.settle(t.Context(), "ace", c.module, c.fresh)
 			if c.wantErr != "" {
 				if err == nil || !strings.Contains(err.Error(), c.wantErr) {
 					t.Fatalf("error %v, want one saying %q", err, c.wantErr)
@@ -137,29 +215,30 @@ func TestSettlingAnAssignmentIssuesComposesTheBusAndGrants(t *testing.T) {
 			} else if err != nil {
 				t.Fatal(err)
 			}
+			if issues != c.wantIssues || catalogues > 1 {
+				t.Fatalf("%d issue(s) from %d reading(s) of the catalogue, want %d from at most one", issues, catalogues, c.wantIssues)
+			}
 			if strings.Join(composed, ",") != strings.Join(c.wantComposed, ",") {
 				t.Fatalf("composed %v, want %v", composed, c.wantComposed)
 			}
-			if grants != c.wantGrants {
-				t.Fatalf("grants written %v, want %v", grants, c.wantGrants)
+			if want := map[bool]int{true: 1}[c.wantGrants]; grants != want {
+				t.Fatalf("grants written %d time(s), want %d", grants, want)
 			}
-			skipped := !c.holds || c.module == controllerModule
-			if skipped != (issuedFor == "") {
-				t.Fatalf("issue asked for %q (lease held %v, module %s)", issuedFor, c.holds, c.module)
-			}
-			if c.wantGrants && c.wantErr == "" && !strings.Contains(said, "its grant written") {
-				t.Fatalf("said %q, want it to name the grant", said)
+			if c.wantGrants && c.wantErr == "" && !strings.Contains(said, "the grants written") {
+				t.Fatalf("said %q, want it to name the grants", said)
 			}
 		})
 	}
 }

-// The account a module speaks with on a node: the module's own when it reads `broker`, its provisioner client's when it
-// runs one alone, none when its accounts are its processes' or it does not speak on the bus.
-func TestAModulesOwnAccountOnANode(t *testing.T) {
+// The accounts a module speaks with on a node: the module's own when it reads `broker`, its provisioner client's when
+// it runs one alone, none when its accounts are its processes' or it does not speak on the bus. A process's account is
+// held in the provisioner client's form, named for the process and carrying no claims, and may be issued only when
+// the process declares its own secret broker-<process id> (ADR 0436 §2; issue 1181).
+func TestTheAccountsAModuleSpeaksWithOnANode(t *testing.T) {
 	client := aProvisioner()
 	delete(client.OwnSecrets, "broker")
-	cases := []struct {
+	for _, c := range []struct {
 		name string
 		m    catalogue.Manifest
 		user string
@@ -173,8 +252,7 @@ func TestAModulesOwnAccountOnANode(t *testing.T) {
 		{"the provisioner client alone", client,
 			broker.Principal{Kind: broker.KindProvisioner, Node: "ace", Module: "mesh-provisioner"}.Username(), true},
 		{"no broker, no client", catalogue.Manifest{Module: "hello-web"}, "", false},
-	}
-	for _, c := range cases {
+	} {
 		t.Run(c.name, func(t *testing.T) {
 			user, own := ownAccountOf(c.m, "ace")
 			if user != c.user || own != c.own {
@@ -182,70 +260,48 @@ func TestAModulesOwnAccountOnANode(t *testing.T) {
 			}
 		})
 	}
-}

-// The watch hands the assignments standing when it starts to the start's look (not fresh), says once that they are all
-// read, and hands each assignment written afterwards as fresh; a rewrite of one that stands is handed nothing.
-func TestTheWatchHandsTheStartsLookThenEachNewAssignment(t *testing.T) {
-	at := time.Date(2026, 10, 25, 16, 19, 42, 0, time.UTC)
-	bucket := chain.NewMemory()
-	put := func(a chain.Assignment) {
-		t.Helper()
-		value := anAssignmentValue(t, a)
-		if e, held, _ := bucket.Get(t.Context(), a.Key()); held {
-			if _, err := bucket.Update(t.Context(), a.Key(), value, e.Revision); err != nil {
-				t.Fatal(err)
-			}
-			return
+	t.Run("a process's account", func(t *testing.T) {
+		user := broker.Principal{Kind: broker.KindProcess, Node: "novox", Module: "mesh-mail", Process: "send"}.Username()
+		if user != "novox.mesh-mail.send" {
+			t.Fatalf("the process's username: %q", user)
 		}
-		if _, err := bucket.Create(t.Context(), a.Key(), value); err != nil {
+		held, err := heldProcessAccount(broker.Broker{Fingerprint: "abc"}, "novox.example:4222", "novox", "mesh-mail",
+			user, "a-password")
+		if err != nil {
 			t.Fatal(err)
 		}
-	}
-	put(chain.Assignment{Node: "g14", Module: "mesh-build-go", Assigned: at})
-
-	type handed struct {
-		key   string
-		fresh bool
-	}
-	got := make(chan handed, 8)
-	read := make(chan struct{}, 8)
-	ctx, stop := context.WithCancel(t.Context())
-	done := make(chan error, 1)
-	go func() {
-		done <- watchAssignmentsOnce(ctx, bucket.Watch, &assignmentTurns{},
-			func(a chain.Assignment, fresh bool) { got <- handed{a.Key(), fresh} },
-			func() { read <- struct{}{} })
-	}()
-	next := func() handed {
-		t.Helper()
-		select {
-		case h := <-got:
-			return h
-		case <-time.After(5 * time.Second):
-			t.Fatal("nothing handed")
-			return handed{}
+		var got map[string]any
+		if err := json.Unmarshal(held, &got); err != nil {
+			t.Fatal(err)
 		}
-	}
-	if h := next(); h != (handed{"g14.mesh-build-go", false}) {
-		t.Fatalf("the start's look handed %+v", h)
-	}
-	select {
-	case <-read:
-	case <-time.After(5 * time.Second):
-		t.Fatal("the current values were never said read")
-	}
-	replanned := chain.Assignment{Node: "g14", Module: "mesh-build-go", Assigned: at, Provisioning: json.RawMessage(`{"plan":2}`)}
-	put(replanned)
-	put(chain.Assignment{Node: "ace", Module: "mesh-build-node", Assigned: at})
-	if h := next(); h != (handed{"ace.mesh-build-node", true}) {
-		t.Fatalf("handed %+v, want ace.mesh-build-node as fresh (and nothing for the rewrite before it)", h)
-	}
-	stop()
-	if err := <-done; err != nil {
-		t.Fatal(err)
-	}
-	if len(read) != 0 {
-		t.Fatal("the current values were said read more than once")
-	}
+		want := map[string]any{"url": "nats://novox.example:4222", "fingerprint": "abc", "node": "novox",
+			"module": "mesh-mail", "user": "novox.mesh-mail.send", "password": "a-password"}
+		for k, v := range want {
+			if got[k] != v {
+				t.Errorf("%s: %v, want %v", k, got[k], v)
+			}
+		}
+		if len(got) != len(want) {
+			t.Errorf("the held account carries %v; want exactly %v (never claims — a process holds no seat)", got, want)
+		}
+	})
+
+	t.Run("a process may be issued only with its own broker secret", func(t *testing.T) {
+		// mesh-mail's real shape: no `broker` of its own, its process declaring bus (review round 1 of issue 1181).
+		if err := mayIssue(mailSenderWithProcess()); err != nil {
+			t.Fatalf("a module with no broker of its own, whose process declares bus, was refused: %v", err)
+		}
+		m := mailSenderWithProcess()
+		delete(m.OwnSecrets, "broker-send")
+		err := mayIssue(m)
+		if err == nil {
+			t.Fatal("a process with no broker-<id> own secret was issued an account")
+		}
+		for _, want := range []string{"mesh-mail", "send", "broker-send", "own-secrets"} {
+			if !strings.Contains(err.Error(), want) {
+				t.Errorf("the refusal does not say %q: %v", want, err)
+			}
+		}
+	})
 }