Forget the accounts of modules no longer on their nodes at the start's look, and grant sealed accounts alone (issue 1945, ADR 0543, review)
mesh/delivery held for a person: merged without a passing check: only a person decides what follows
mesh/delivery-group group fix/1945-credential-when-row-without-seal held: a member is held: novox/mesh-controller@fbf14b68dbab (merged without a passing check:…
mesh/check checking fbf14b68dbab on 40034be479f8

Migration 0108 marks every standing account sealed, ace's stale mesh-build-go
rows from 17:19 among them, and the forgetting ran only at a later unassign:
the next assignment to ace would have issued nothing again.

- The start's look first forgets, from each node's own resolution (assigned
  and required), the accounts of every module that no longer resolves on its
  node, and says each; only from a reading that holds assignments, and a node
  whose resolution is unknown is left alone.
- Unassign forgets a module's accounts only once it no longer resolves on the
  node (a module another requires keeps running), and never the node's own
  provisioner account; the forgetting moved from the inventory to the act.
- Forgetting holds each account's lock across its delete, so an issue in flight
  never writes a row back after it.
- A module whose accounts are not all sealed is granted its sealed accounts
  alone, and its grant is dropped when none is: an older grant naming ace no
  longer lets the assigner want it.
- An issue's locked section has a deadline of two minutes, since it holds a
  connection of the store's pool across the vault and the bus.
Tests: cases in the existing tables.
Research 072: the grants piece (memberships and credentials heard from placement).
This commit is contained in:
2026-10-25 20:27:30 +01:00
parent 79ac62ffbc
commit fbf14b68db
9 changed files with 234 additions and 32 deletions
+16
View File
@@ -292,6 +292,22 @@ func unassign(ctx context.Context, open *stores, node string, modules ...string)
answer += "\n " + strings.TrimSpace(line)
}
}
// Its bus accounts on the node go with it once it no longer resolves there (a module another requires keeps running
// and keeps them), so an assignment again issues them afresh (issue 1945). The node's own provisioner account stays.
if after.err == nil {
for _, module := range modules {
if slices.Contains(after.modules, module) {
continue
}
forgot, err := open.inventory.ForgetBusAccountsOf(ctx, node, module, false)
switch {
case err != nil:
answer += fmt.Sprintf("\n its bus accounts on %s are not forgotten: %v", node, err)
case len(forgot) > 0:
answer += fmt.Sprintf("\n its bus accounts on %s forgotten: %s", node, strings.Join(forgot, ", "))
}
}
}
// What it leaves behind that is irreplaceable is kept and retired, never removed (novox/hq ADR 0233).
for _, line := range keptOnUnassign(ctx, open.inventory, node, modules) {
answer += "\n " + line
+87 -3
View File
@@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"maps"
"slices"
"strings"
"sync"
@@ -40,6 +41,9 @@ import (
type assignmentAccountSteps struct {
// holds says this controller holds the lease: only its holder issues.
holds func(ctx context.Context) bool
// forgetStale forgets every module's accounts on a node where the module no longer resolves, and answers a line for
// each (issue 1945: rows of an unassignment before this release, marked sealed by migration 0108).
forgetStale func(ctx context.Context) ([]string, error)
// catalogue reads the catalogue once, for every assignment a look issues for.
catalogue func(ctx context.Context) (map[string]catalogue.Manifest, error)
// issue issues the module's accounts on the node that are not minted, from the catalogue read; issued says it
@@ -86,6 +90,17 @@ func (s assignmentAccountSteps) settle(ctx context.Context, node, module string)
// it issued (an assignment that came to stand while no controller heard it may have its accounts and still lack its
// place in the bus's user list or its grant). It answers what it did.
func (s assignmentAccountSteps) look(ctx context.Context, standing []chain.Assignment) (string, error) {
var forgot []string
if len(standing) > 0 && s.forgetStale != nil {
// Only from a reading of the assignments that holds some: an empty or failed one would forget every account.
if !s.holds(ctx) {
return "", errNotHolding
}
var err error
if forgot, err = s.forgetStale(ctx); err != nil {
return "", fmt.Errorf("the accounts of modules no longer on their nodes are not forgotten: %w", err)
}
}
var looked []chain.Assignment
for _, a := range standing {
if a.Module != controllerModule {
@@ -93,7 +108,7 @@ func (s assignmentAccountSteps) look(ctx context.Context, standing []chain.Assig
}
}
if len(looked) == 0 {
return "", nil
return strings.Join(forgot, "\n"), nil
}
if !s.holds(ctx) {
return "", errNotHolding
@@ -114,7 +129,7 @@ func (s assignmentAccountSteps) look(ctx context.Context, standing []chain.Assig
}
}
said, err := s.cover(ctx)
lines = append(lines, fmt.Sprintf("%d standing assignment(s) looked at; %s", len(looked), said))
lines = append(append(forgot, lines...), fmt.Sprintf("%d standing assignment(s) looked at; %s", len(looked), said))
return strings.Join(lines, "\n"), err
}
@@ -247,7 +262,8 @@ func liveAssignmentAccountSteps(open *stores) assignmentAccountSteps {
_, err := theLease.epoch(ctx)
return err == nil
},
catalogue: func(ctx context.Context) (map[string]catalogue.Manifest, error) { return inv.Catalogue(ctx) },
forgetStale: func(ctx context.Context) ([]string, error) { return forgetStaleAccounts(ctx, open) },
catalogue: func(ctx context.Context) (map[string]catalogue.Manifest, error) { return inv.Catalogue(ctx) },
issue: func(ctx context.Context, shelf map[string]catalogue.Manifest, node, module string) (bool, string, error) {
return issueAssignmentAccounts(ctx, inv, shelf, node, module)
},
@@ -403,3 +419,71 @@ func watchAssignmentsOnce(ctx context.Context, watch func(context.Context) (<-ch
}
}
}
// staleAccounts is, by node, each module some account of which stands on a node where the module no longer resolves:
// the accounts of a module's own, its processes' and its provisioner client's. A node whose resolution is unknown is
// left alone.
func staleAccounts(users map[string]inventory.BusUser, resolved map[string]map[string]bool) map[string][]string {
seen := map[string]map[string]bool{}
for _, u := range users {
switch u.Kind {
case inventory.BusModule, inventory.BusNodeTools, inventory.BusProcess, inventory.BusProvisioner:
default:
continue
}
on, known := resolved[u.Node]
if u.Node == "" || u.Module == "" || !known || on[u.Module] {
continue
}
if seen[u.Node] == nil {
seen[u.Node] = map[string]bool{}
}
seen[u.Node][u.Module] = true
}
out := map[string][]string{}
for node, modules := range seen {
out[node] = slices.Sorted(maps.Keys(modules))
}
return out
}
// forgetStaleAccounts forgets the accounts of every module on a node where it no longer resolves (assigned or
// required), from each node's own resolution; a node that does not resolve is left alone.
func forgetStaleAccounts(ctx context.Context, open *stores) ([]string, error) {
inv := open.inventory
users, err := inv.BusUsers(ctx)
if err != nil {
return nil, err
}
ctx = withInputCache(ctx) // one reading of the mesh for every node resolved
resolved := map[string]map[string]bool{}
for _, u := range users {
if _, done := resolved[u.Node]; done || u.Node == "" {
continue
}
plan, _, err := resolveNode(ctx, open, u.Node)
if err != nil {
continue // unknown: nothing of this node is forgotten
}
on := map[string]bool{}
for _, m := range plan.Modules {
on[m.Module] = true
}
resolved[u.Node] = on
}
var lines []string
stale := staleAccounts(users, resolved)
for _, node := range slices.Sorted(maps.Keys(stale)) {
for _, module := range stale[node] {
forgot, err := inv.ForgetBusAccountsOf(ctx, node, module, true)
if err != nil {
return lines, err
}
if len(forgot) > 0 {
lines = append(lines, fmt.Sprintf("%s no longer runs on %s: its bus accounts forgotten: %s", module, node,
strings.Join(forgot, ", ")))
}
}
}
return lines, nil
}
@@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"reflect"
"strings"
"testing"
"time"
@@ -194,6 +195,7 @@ func TestSettlingAndTheStartsLook(t *testing.T) {
wantComposed []string
wantGrants bool
wantErr string
stale []string // what forgetting the accounts of modules no longer on their nodes says
}{
{name: "an account not minted", module: "mesh-build-node", issued: true, holder: "novox", wantIssues: 1,
wantComposed: []string{"novox"}, wantGrants: true},
@@ -214,13 +216,21 @@ func TestSettlingAndTheStartsLook(t *testing.T) {
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"},
{name: "the start's look forgets the accounts of modules no longer on their nodes first",
look: []chain.Assignment{ace("mesh-build-node")}, holder: "novox", wantIssues: 1, wantComposed: []string{"novox"},
wantGrants: true, stale: []string{"mesh-build-go no longer runs on ace: its bus accounts forgotten: ace.mesh-build-go"}},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
var composed []string
issues, catalogues, grants := 0, 0, 0
forgets := 0
steps := assignmentAccountSteps{
holds: func(context.Context) bool { return !c.notHolding },
forgetStale: func(context.Context) ([]string, error) {
forgets++
return c.stale, nil
},
catalogue: func(context.Context) (map[string]catalogue.Manifest, error) {
catalogues++
return map[string]catalogue.Manifest{}, nil
@@ -262,6 +272,14 @@ func TestSettlingAndTheStartsLook(t *testing.T) {
if want := map[bool]int{true: 1}[c.wantGrants]; grants != want {
t.Fatalf("grants written %d time(s), want %d", grants, want)
}
if want := map[bool]int{true: 1}[len(c.look) > 0 && !c.notHolding]; forgets != want {
t.Fatalf("forgot stale accounts %d time(s), want %d (only from a reading that holds assignments)", forgets, want)
}
for _, line := range c.stale {
if !strings.Contains(said, line) {
t.Fatalf("said %q, want it to say %q", said, line)
}
}
if c.wantGrants && c.wantErr == "" && !strings.Contains(said, "the grants written") {
t.Fatalf("said %q, want it to name the grants", said)
}
@@ -367,6 +385,26 @@ func TestTheAccountsAModuleSpeaksWithOnANode(t *testing.T) {
})
}
// The accounts of a module that no longer resolves on its node are stale, its provisioner client's among them; a
// module still resolved (assigned or required), an account of another kind, and a node whose resolution is unknown
// are left alone (issue 1945: ace's mesh-build-go rows from 17:19, marked sealed by migration 0108).
t.Run("stale accounts", func(t *testing.T) {
u := func(kind, node, module string) inventory.BusUser {
return inventory.BusUser{Username: node + "." + module + "." + kind, Kind: kind, Node: node, Module: module}
}
users := map[string]inventory.BusUser{}
for _, b := range []inventory.BusUser{u(inventory.BusModule, "ace", "mesh-build-go"),
u(inventory.BusProcess, "ace", "mesh-build-go"), u(inventory.BusProvisioner, "ace", "mesh-provisioner"),
u(inventory.BusModule, "ace", "mesh-build-node"), u(inventory.BusNode, "ace", ""),
u(inventory.BusModule, "g14", "mesh-build-go")} {
users[b.Username] = b
}
got := staleAccounts(users, map[string]map[string]bool{"ace": {"mesh-build-node": true}})
if want := map[string][]string{"ace": {"mesh-build-go", "mesh-provisioner"}}; !reflect.DeepEqual(got, want) {
t.Fatalf("stale %v, want %v", 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 {
+26 -7
View File
@@ -23,10 +23,13 @@ import (
// writes `<module>.<fingerprint hex>` then, naming the accounts it composed for the module, never removed. A module one
// of whose accounts has no password yet (the list leaves it out) has no grant until a later send carries it.
// grantsFrom is the grant of every module whose accounts all have a password in hashes, at the fingerprint current names
// for it; a module whose current build names no fingerprint of the new form has none. principals are the bus's users
// as broker.Users composes them.
func grantsFrom(principals []broker.Principal, hashes map[string]string, current map[string]string) map[string]sdkdelivery.ControllerGrant {
// grantsFrom is the grant of every module at the fingerprint current names for it, naming its accounts that have a
// sealed password in hashes; a module whose current build names no fingerprint of the new form has none. A module one
// of whose accounts has none is granted its sealed accounts alone, and when it has none, its grant is dropped (drop):
// an older grant naming a node whose account is gone would let the assigner want it (issue 1945, rule 6 per node), and
// an empty grant names every node. principals are the bus's users as broker.Users composes them.
func grantsFrom(principals []broker.Principal, hashes map[string]string, current map[string]string) (
map[string]sdkdelivery.ControllerGrant, []string) {
accounts, missing := map[string][]string{}, map[string]bool{}
for _, p := range principals {
if p.Module == "" || (p.Kind != broker.KindModule && p.Kind != broker.KindProcess) {
@@ -39,8 +42,9 @@ func grantsFrom(principals []broker.Principal, hashes map[string]string, current
accounts[p.Module] = append(accounts[p.Module], p.Username())
}
out := map[string]sdkdelivery.ControllerGrant{}
var drop []string
for module, fp := range current {
if missing[module] || !strings.HasPrefix(fp, sdkdelivery.SourcePrefix2) {
if !strings.HasPrefix(fp, sdkdelivery.SourcePrefix2) {
continue
}
key, err := sdkdelivery.ControllerGrantKey(module, fp)
@@ -48,10 +52,15 @@ func grantsFrom(principals []broker.Principal, hashes map[string]string, current
continue
}
names := accounts[module]
if missing[module] && len(names) == 0 {
drop = append(drop, key)
continue
}
sort.Strings(names)
out[key] = sdkdelivery.ControllerGrant{Accounts: names}
}
return out
sort.Strings(drop)
return out, drop
}
// currentFingerprints is, by module, the fingerprint of the build each registered module is registered at: its
@@ -258,7 +267,17 @@ func grantsIssued(ctx context.Context, inv *inventory.Inventory, js jetstream.Je
if err != nil {
return fmt.Errorf("the grants issued are not written: %w", err)
}
wrote, err := keepGrants(ctx, kv, grantsFrom(principals, hashes, current))
grants, drop := grantsFrom(principals, hashes, current)
wrote, err := keepGrants(ctx, kv, grants)
for _, key := range drop {
if _, gerr := kv.Get(ctx, key); gerr == nil {
if derr := kv.Delete(ctx, key); derr != nil {
err = errors.Join(err, fmt.Errorf("%s: %w", key, derr))
} else {
fmt.Printf("%sthe grant %s dropped: none of its accounts is sealed to its node\n", indent, key)
}
}
}
if wrote > 0 {
fmt.Printf("%s%d grant(s) issued written for the assigner\n", indent, wrote)
}
+17 -4
View File
@@ -23,12 +23,25 @@ func TestTheGrantsAListIssues(t *testing.T) {
y := broker.Principal{Kind: broker.KindModule, Node: "ace", Module: "mesh-y"}
node := broker.Principal{Kind: broker.KindNode, Node: "novox"}
hashes := map[string]string{x.Username(): "h1", xp.Username(): "h2", node.Username(): "h3"}
got := grantsFrom([]broker.Principal{x, xp, y, node}, hashes,
map[string]string{"mesh-x": grantFP('a'), "mesh-y": grantFP('b'), "web": grantFP('c'), "old": "src1:abc"})
// mesh-z is sealed on novox and not on ace (issue 1945: an older grant must not name ace): its grant names novox alone.
zn := broker.Principal{Kind: broker.KindModule, Node: "novox", Module: "mesh-z"}
za := broker.Principal{Kind: broker.KindModule, Node: "ace", Module: "mesh-z"}
hashes[zn.Username()] = "h4"
got, drop := grantsFrom([]broker.Principal{x, xp, y, node, zn, za}, hashes,
map[string]string{"mesh-x": grantFP('a'), "mesh-y": grantFP('b'), "web": grantFP('c'), "old": "src1:abc",
"mesh-z": grantFP('d')})
xKey, _ := sdkdelivery.ControllerGrantKey("mesh-x", grantFP('a'))
yKey, _ := sdkdelivery.ControllerGrantKey("mesh-y", grantFP('b'))
zKey, _ := sdkdelivery.ControllerGrantKey("mesh-z", grantFP('d'))
webKey, _ := sdkdelivery.ControllerGrantKey("web", grantFP('c'))
if len(got) != 2 {
t.Fatalf("grants = %v, want mesh-x's and web's", got)
if len(got) != 3 {
t.Fatalf("grants = %v, want mesh-x's, mesh-z's and web's", got)
}
if g := got[zKey]; len(g.Accounts) != 1 || g.Accounts[0] != zn.Username() {
t.Errorf("mesh-z's grant = %+v, want its sealed account alone", g)
}
if len(drop) != 1 || drop[0] != yKey {
t.Errorf("dropped %v, want mesh-y's grant, none of whose accounts is sealed", drop)
}
if g := got[xKey]; len(g.Accounts) != 2 || g.Accounts[0] != x.Username() && g.Accounts[1] != x.Username() {
t.Errorf("mesh-x's grant = %+v, want both its accounts", g)
+8
View File
@@ -1008,6 +1008,10 @@ type accountIssuer interface {
MarkBusAccountSealed(ctx context.Context, username string) error
}
// issueLockedFor is the longest an issue holds an account's lock: a seal cut by it leaves the account unsealed, issued
// again by the next issue.
const issueLockedFor = 2 * time.Minute
// issueAccount mints u's password and seals it to its node with seal. By default it replaces whatever stands (a
// rotation: `module issue`). Under onlyIfAbsent it issues only an account that is not issued, a row with no sealed
// credential counting as none (issue 1945, live on 2026-10-25 at 20:04: a row whose credential the node no longer held
@@ -1025,6 +1029,10 @@ func issueAccount(ctx context.Context, accounts accountIssuer, u inventory.BusUs
return err
}
} else {
// A deadline on the locked section: it holds a connection of the store's pool across the vault and the bus.
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(ctx, issueLockedFor)
defer cancel()
release, err := accounts.LockBusAccount(ctx, u.Username)
if err != nil {
return err