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
+41 -13
View File
@@ -141,30 +141,58 @@ func (i *Inventory) LockBusAccount(ctx context.Context, username string) (func()
}
// ForgetBusAccountsOf forgets the bus accounts of module on node, and the credentials sealed to the node for them (its
// `broker`, each process's `broker-<id>`, its provisioner client's): an unassigned module keeps no account, so an
// assignment again mints and seals afresh, never trusting a row whose credential the node no longer holds (issue 1945,
// live on 2026-10-25 at 20:04: ace's mesh-build-go rows from 17:19 survived its unassignment, nothing was minted or
// sealed again, and the module found no credential).
func (i *Inventory) ForgetBusAccountsOf(ctx context.Context, node, module string) error {
// `broker`, each process's `broker-<id>`, and with nodeOwned its provisioner client's): a module that no longer resolves
// on a node keeps no account there, so an assignment again mints and seals afresh, never trusting a row whose credential
// the node no longer holds (issue 1945, live on 2026-10-25 at 20:04: ace's mesh-build-go rows from 17:19 outlived its
// unassignment, nothing was issued again, and the module found no credential). The provisioner client's account is the
// node's (`provisioner.<node>`), forgotten only with nodeOwned. Each account's lock is held across its delete, so an issue
// in flight never writes a row back after it. It answers the accounts forgotten.
func (i *Inventory) ForgetBusAccountsOf(ctx context.Context, node, module string, nodeOwned bool) ([]string, error) {
if node == "" || module == "" {
return errors.New("forgetting the bus accounts of no node or no module would forget others' accounts")
return nil, errors.New("forgetting the bus accounts of no node or no module would forget others' accounts")
}
rows, err := i.store.Pool().Query(ctx,
`select username from bus_user where node = $1 and module = $2 and kind <> $3 and ($4 or kind <> $5)
order by username`, node, module, BusController, nodeOwned, BusProvisioner)
if err != nil {
return nil, err
}
var names []string
for rows.Next() {
var n string
if err := rows.Scan(&n); err != nil {
rows.Close()
return nil, err
}
names = append(names, n)
}
rows.Close()
if err := rows.Err(); err != nil {
return nil, err
}
for _, n := range names { // in order, one lock at a time per issuer: no two hold each other's
release, err := i.LockBusAccount(ctx, n)
if err != nil {
return nil, err
}
defer release()
}
tx, err := i.store.Pool().Begin(ctx)
if err != nil {
return err
return nil, err
}
defer func() { _ = tx.Rollback(context.WithoutCancel(ctx)) }()
if _, err := tx.Exec(ctx, `delete from bus_user where node = $1 and module = $2 and kind <> $3`,
node, module, BusController); err != nil {
return fmt.Errorf("the bus accounts of %s on %s are not forgotten: %w", module, node, err)
if _, err := tx.Exec(ctx, `delete from bus_user where username = any($1)`, names); err != nil {
return nil, fmt.Errorf("the bus accounts of %s on %s are not forgotten: %w", module, node, err)
}
if _, err := tx.Exec(ctx,
`delete from module_secret s using node n
where s.node = n.id and n.name = $1 and s.module = $2
and (s.name = 'broker' or s.name like 'broker-%' or s.name = 'provisioner-broker')`, node, module); err != nil {
return fmt.Errorf("the bus credentials of %s on %s are not forgotten: %w", module, node, err)
and (s.name = 'broker' or s.name like 'broker-%' or ($3 and s.name = 'provisioner-broker'))`,
node, module, nodeOwned); err != nil {
return nil, fmt.Errorf("the bus credentials of %s on %s are not forgotten: %w", module, node, err)
}
return tx.Commit(ctx)
return names, tx.Commit(ctx)
}
// MintBusPasswordUnsealed mints u's password, replacing whatever stands, with no seal yet: the account is not issued
@@ -105,7 +105,7 @@ func TestMintingAgainRotatesAndMintingIfAbsentHasOneWinner(t *testing.T) {
}
release()
<-waiting
if err := inv.ForgetBusAccountsOf(ctx, "one", "racer"); err != nil {
if _, err := inv.ForgetBusAccountsOf(ctx, "one", "racer", true); err != nil {
t.Fatal(err)
}
if minted, _, _ := inv.BusAccountIssued(ctx, "one.racer"); minted {
-4
View File
@@ -670,10 +670,6 @@ func (i *Inventory) Unassign(ctx context.Context, nodeName, module string) error
if err := i.chainUnassign(ctx, c, node, module); err != nil {
return err
}
// Its bus accounts on the node go with it: an assignment again issues them afresh (issue 1945).
if err := i.ForgetBusAccountsOf(ctx, nodeName, module); err != nil {
return err
}
// The seat it held falls back to derivation rather than pointing at nothing: what the store's foreign key did.
if _, err := i.store.Pool().Exec(ctx, `delete from seat_holding where node = $1 and module = $2`, node.ID, module); err != nil {
return err