diff --git a/cmd/mesh-controller/acts.go b/cmd/mesh-controller/acts.go index 8de675b5..354e83cc 100644 --- a/cmd/mesh-controller/acts.go +++ b/cmd/mesh-controller/acts.go @@ -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 @@ -421,9 +437,9 @@ func issueOnAssign(ctx context.Context, open *stores, node, module string) strin // A module running the provisioner client alone is minted the client's account (issue 917). user = broker.Principal{Kind: broker.KindProvisioner, Node: node, Module: module}.Username() } - if _, minted, err := inv.BusUserHash(ctx, user); err != nil { + if _, sealed, err := inv.BusAccountIssued(ctx, user); err != nil { return "" - } else if minted { + } else if sealed { // a row whose credential is not sealed to the node is not issued (issue 1945) // Its own account stands and is kept; a process it gained is not issued yet (issue 1226). return processAccountsOnAssign(ctx, inv, m, node) } diff --git a/cmd/mesh-controller/assignment_accounts.go b/cmd/mesh-controller/assignment_accounts.go index 8c6ba982..242853de 100644 --- a/cmd/mesh-controller/assignment_accounts.go +++ b/cmd/mesh-controller/assignment_accounts.go @@ -5,6 +5,8 @@ import ( "encoding/json" "errors" "fmt" + "maps" + "slices" "strings" "sync" "time" @@ -39,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 @@ -85,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 { @@ -92,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 @@ -113,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 } @@ -195,22 +211,27 @@ func issueAssignmentAccounts(ctx context.Context, inv *inventory.Inventory, shel if !known || mayIssue(m) != nil { return false, "", nil } - if user, own := ownAccountOf(m, node); own { - _, minted, err := inv.BusUserHash(ctx, user) + if _, own := ownAccountOf(m, node); own { + minted, err := inv.BusMinted(ctx) // sealed accounts alone: a row with no sealed credential is not issued if err != nil { - return false, "", fmt.Errorf("whether %s is minted cannot be read: %w", user, err) + return false, "", fmt.Errorf("the issued accounts cannot be read: %w", err) } - if !minted { + if missingAccounts(m, node, minted) { busAddress, err := broker.BusAddress() if err != nil { return false, "", err } - // Its processes' accounts too: issueOnTheNewBus issues every one of them with the module's own. + // Its processes' and provisioner client's accounts too: issueOnTheNewBus issues each that is not issued. + ctx, got := withIssued(ctx) if err := issueOnTheNewBus(ctx, inv, m, node, busAddress); err != nil { return false, "", err } - return true, "its bus credential issued as " + user, nil + if issued := got.list(); len(issued) > 0 { + return true, "issued and sealed to " + node + ": " + strings.Join(issued, ", "), nil + } + return false, "", nil } + return false, "its bus accounts on " + node + " stood issued and sealed", nil } line, err := issueMissingProcessAccounts(ctx, inv, m, node) if err != nil { @@ -219,6 +240,18 @@ func issueAssignmentAccounts(ctx context.Context, inv *inventory.Inventory, shel return line != "", line, nil } +// missingAccounts says some account of m on node is not issued: its own, a process's, or its provisioner client's. +func missingAccounts(m catalogue.Manifest, node string, minted []string) bool { + if user, own := ownAccountOf(m, node); own && !slices.Contains(minted, user) { + return true + } + if runsTheProvisionerClient(m) && + !slices.Contains(minted, broker.Principal{Kind: broker.KindProvisioner, Node: node, Module: m.Module}.Username()) { + return true + } + return len(processAccountsMissing(m, node, minted)) > 0 +} + // liveAssignmentAccountSteps are the steps on the controller's own stores and the bus: the composer's passes, as the // done keys' take-ins use them, so a node asked for by both is composed once a pass. func liveAssignmentAccountSteps(open *stores) assignmentAccountSteps { @@ -229,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) }, @@ -385,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 +} diff --git a/cmd/mesh-controller/assignment_accounts_test.go b/cmd/mesh-controller/assignment_accounts_test.go index 277e8c8d..0dd9cbc2 100644 --- a/cmd/mesh-controller/assignment_accounts_test.go +++ b/cmd/mesh-controller/assignment_accounts_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "reflect" "strings" "testing" "time" @@ -12,6 +13,7 @@ import ( "git.novox.be/novox/mesh-sdk/go/catalogue" "github.com/novox/mesh-controller/internal/broker" "github.com/novox/mesh-controller/internal/chain" + "github.com/novox/mesh-controller/internal/inventory" ) // An assignment that comes to stand issues its node's bus accounts and the module's grant (ADR 0543, issue 1945): on @@ -193,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}, @@ -213,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 @@ -261,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) } @@ -324,6 +343,68 @@ func TestTheAccountsAModuleSpeaksWithOnANode(t *testing.T) { } }) + // An issue mints and seals an account only when it is not issued: no row, or a row with no sealed credential (issue + // 1945, live on 2026-10-25 at 20:04: ace's mesh-build-go had a row from 17:19 and no credential), under the lock on + // the account; a seal that fails leaves it unsealed for the next issue. A rotation always mints. + for _, c := range []struct { + name string + only bool + minted, sealed bool + sealErr error + want string + }{ + {"no row: minted unsealed, sealed, marked", true, false, false, nil, "lock mint-unsealed seal mark release"}, + {"a row sealed: nothing", true, true, true, nil, "lock release"}, + {"a row with no sealed credential: minted and sealed again", true, true, false, nil, "lock mint-unsealed seal mark release"}, + {"a seal that fails: not marked", true, false, false, errors.New("the vault did not take it"), "lock mint-unsealed seal release"}, + {"a rotation mints whatever stands", false, true, true, nil, "mint seal"}, + } { + t.Run("issuing an account: "+c.name, func(t *testing.T) { + acts := &anAccountStore{minted: c.minted, sealed: c.sealed} + ctx, got := withIssued(t.Context()) + if c.only { + ctx = onlyIfAbsent(ctx) + } + err := issueAccount(ctx, acts, inventory.BusUser{Username: "ace.mesh-build-go", Kind: inventory.BusModule}, + func(password string) error { + acts.did("seal") + if password == "" { + t.Error("sealed no password") + } + return c.sealErr + }) + if (err != nil) != (c.sealErr != nil) { + t.Fatalf("error %v, want %v", err, c.sealErr) + } + if strings.Join(acts.acts, " ") != c.want { + t.Fatalf("acts %q, want %q", strings.Join(acts.acts, " "), c.want) + } + if issued := len(got.list()) == 1; issued != (c.sealErr == nil && strings.Contains(c.want, "seal")) { + t.Fatalf("said issued %v", got.list()) + } + }) + } + + // 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 { @@ -342,3 +423,30 @@ func TestTheAccountsAModuleSpeaksWithOnANode(t *testing.T) { } }) } + +// anAccountStore is an accountIssuer that writes down what was asked of it. +type anAccountStore struct { + minted, sealed bool + acts []string +} + +func (a *anAccountStore) did(act string) { a.acts = append(a.acts, act) } +func (a *anAccountStore) LockBusAccount(context.Context, string) (func(), error) { + a.did("lock") + return func() { a.did("release") }, nil +} +func (a *anAccountStore) BusAccountIssued(context.Context, string) (bool, bool, error) { + return a.minted, a.sealed, nil +} +func (a *anAccountStore) MintBusPassword(context.Context, inventory.BusUser) (string, error) { + a.did("mint") + return "a-password", nil +} +func (a *anAccountStore) MintBusPasswordUnsealed(context.Context, inventory.BusUser) (string, error) { + a.did("mint-unsealed") + return "a-password", nil +} +func (a *anAccountStore) MarkBusAccountSealed(context.Context, string) error { + a.did("mark") + return nil +} diff --git a/cmd/mesh-controller/grants.go b/cmd/mesh-controller/grants.go index 32310c53..4151aa0c 100644 --- a/cmd/mesh-controller/grants.go +++ b/cmd/mesh-controller/grants.go @@ -23,10 +23,13 @@ import ( // writes `.` 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 @@ -248,13 +257,27 @@ func grantsIssued(ctx context.Context, inv *inventory.Inventory, js jetstream.Je } hashes := make(map[string]string, len(kept)) for name, u := range kept { - hashes[name] = u.PasswordHash + // A grant names an account whose credential is sealed to its node, never one only minted: the assigner wants a + // node only once the grant names its account (issue 1945, rule 6 per node). + if u.Sealed { + hashes[name] = u.PasswordHash + } } current, err := currentFingerprints(ctx, inv) 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) } diff --git a/cmd/mesh-controller/grants_test.go b/cmd/mesh-controller/grants_test.go index b14715ef..9bbe0395 100644 --- a/cmd/mesh-controller/grants_test.go +++ b/cmd/mesh-controller/grants_test.go @@ -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) diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index c690dd34..1b2a5140 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -9,8 +9,10 @@ import ( "net" "os" "path/filepath" + "slices" "sort" "strings" + "sync" "time" "git.novox.be/novox/mesh-sdk/go/catalogue" @@ -965,24 +967,99 @@ func processPrincipalFor(m catalogue.Manifest, processID string) (broker.Process return broker.ProcessPrincipal{}, false } -// mintOnlyIfAbsent marks a context whose issue mints only accounts no row names yet (onlyIfAbsent). +// mintOnlyIfAbsent marks a context whose issue mints only accounts that are not issued (onlyIfAbsent). type mintOnlyIfAbsent struct{} -// onlyIfAbsent is ctx for an issue that must never replace a minted account: the assign act's and the assignments' -// watch's, which may run at once for one account (novox/hq ADR 0543, the review of issue 1945). +// onlyIfAbsent is ctx for an issue that never replaces an issued account: the assign act's, the assignments' watch's +// and a registration's, which may run at once for one account (novox/hq ADR 0543, issue 1945). func onlyIfAbsent(ctx context.Context) context.Context { return context.WithValue(ctx, mintOnlyIfAbsent{}, true) } -// mintBusUser mints u's password. By default it replaces whatever stands (a rotation: `module issue`); under -// onlyIfAbsent it mints only when no row names u, and won false says another caller minted it first and seals it, -// so this one seals nothing. -func mintBusUser(ctx context.Context, inv *inventory.Inventory, u inventory.BusUser) (string, bool, error) { - if only, _ := ctx.Value(mintOnlyIfAbsent{}).(bool); only { - return inv.MintBusPasswordIfAbsent(ctx, u) +// issuedNow collects the accounts an issue minted and sealed, for the line that says it (withIssued). +type issuedNow struct { + mu sync.Mutex + users []string +} + +type issuedNowKey struct{} + +// withIssued is ctx collecting the accounts its issues mint and seal. +func withIssued(ctx context.Context) (context.Context, *issuedNow) { + got := &issuedNow{} + return context.WithValue(ctx, issuedNowKey{}, got), got +} + +func (g *issuedNow) list() []string { + if g == nil { + return nil } - password, err := inv.MintBusPassword(ctx, u) - return password, err == nil, err + g.mu.Lock() + defer g.mu.Unlock() + return slices.Clone(g.users) +} + +// accountIssuer is what issuing one account needs of the store. +type accountIssuer interface { + LockBusAccount(ctx context.Context, username string) (func(), error) + BusAccountIssued(ctx context.Context, username string) (minted, sealed bool, err error) + MintBusPassword(ctx context.Context, u inventory.BusUser) (string, error) + MintBusPasswordUnsealed(ctx context.Context, u inventory.BusUser) (string, error) + 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 +// left ace's mesh-build-go with no credential), under the store's lock on the account, so two issuers never interleave +// their look, mint and seal: the hash the bus holds is always the password sealed. A seal that fails leaves the +// account unsealed, issued again by the next issue. +func issueAccount(ctx context.Context, accounts accountIssuer, u inventory.BusUser, seal func(password string) error) error { + only, _ := ctx.Value(mintOnlyIfAbsent{}).(bool) + if !only { + password, err := accounts.MintBusPassword(ctx, u) + if err != nil { + return err + } + if err := seal(password); err != nil { + 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 + } + defer release() + if minted, sealed, err := accounts.BusAccountIssued(ctx, u.Username); err != nil { + return err + } else if minted && sealed { + return nil + } + password, err := accounts.MintBusPasswordUnsealed(ctx, u) + if err != nil { + return err + } + if err := seal(password); err != nil { + return err + } + if err := accounts.MarkBusAccountSealed(ctx, u.Username); err != nil { + return fmt.Errorf("%s is sealed and not marked so, and is issued again by the next issue: %w", u.Username, err) + } + } + if got, _ := ctx.Value(issuedNowKey{}).(*issuedNow); got != nil { + got.mu.Lock() + got.users = append(got.users, u.Username) + got.mu.Unlock() + } + return nil } // issueOnTheNewBus gives an assigned module its credential on the bus being built. @@ -1010,17 +1087,12 @@ func issueOnTheNewBus(ctx context.Context, inv *inventory.Inventory, m catalogue } if _, reads := m.OwnSecrets["broker"]; reads { user := broker.Principal{Kind: broker.KindModule, Node: node, Module: m.Module}.Username() - password, won, err := mintBusUser(ctx, inv, inventory.BusUser{ - Username: user, Kind: busKindOf(m.Module), Node: node, Module: m.Module, - }) - if err != nil { + if err := issueAccount(ctx, inv, inventory.BusUser{Username: user, Kind: busKindOf(m.Module), Node: node, + Module: m.Module}, func(password string) error { + return issueWith(ctx, inv, m, node, busAddress, known, reachable, user, password) + }); err != nil { return err } - if won { - if err := issueWith(ctx, inv, m, node, busAddress, known, reachable, user, password); err != nil { - return err - } - } } // Every process with a bus declaration gets an account of its own (novox/hq ADR 0436 §1, §2), minted and // delivered whether or not the module itself speaks on the bus — its own secret is broker-, @@ -1043,12 +1115,15 @@ func issueOnTheNewBus(ctx context.Context, inv *inventory.Inventory, m catalogue func issueProcess(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest, pr broker.ProcessPrincipal, node, busAddress string, known broker.Broker, reachable string) error { user := broker.Principal{Kind: broker.KindProcess, Node: node, Module: m.Module, Process: pr.Process}.Username() - password, won, err := mintBusUser(ctx, inv, inventory.BusUser{ - Username: user, Kind: inventory.BusProcess, Node: node, Module: m.Module, - }) - if err != nil || !won { - return err // lost: the caller that minted it seals it - } + return issueAccount(ctx, inv, inventory.BusUser{Username: user, Kind: inventory.BusProcess, Node: node, Module: m.Module}, + func(password string) error { + return sealProcess(ctx, inv, m, pr, node, busAddress, known, reachable, user, password) + }) +} + +// sealProcess seals a process's minted account to its node, hands it to the vault and makes its work-queue consumers. +func sealProcess(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest, pr broker.ProcessPrincipal, + node, busAddress string, known broker.Broker, reachable, user, password string) error { name := processSecretName(pr.Process) held, err := heldProcessAccount(known, reachable, node, m.Module, user, password) if err != nil { @@ -1113,12 +1188,15 @@ func runsTheProvisionerClient(m catalogue.Manifest) bool { func issueProvisionerClient(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest, node string, known broker.Broker, reachable string) error { user := broker.Principal{Kind: broker.KindProvisioner, Node: node, Module: m.Module}.Username() - password, won, err := mintBusUser(ctx, inv, inventory.BusUser{ - Username: user, Kind: inventory.BusProvisioner, Node: node, Module: m.Module, + return issueAccount(ctx, inv, inventory.BusUser{Username: user, Kind: inventory.BusProvisioner, Node: node, + Module: m.Module}, func(password string) error { + return sealProvisionerClient(ctx, inv, m, node, known, reachable, user, password) }) - if err != nil || !won { - return err // lost: the caller that minted it seals it - } +} + +// sealProvisionerClient seals the provisioner client's minted account to its machine and hands it to the vault. +func sealProvisionerClient(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest, node string, + known broker.Broker, reachable, user, password string) error { held, err := json.Marshal(struct { URL string `json:"url"` Fingerprint string `json:"fingerprint,omitempty"` diff --git a/cmd/mesh-controller/process_accounts.go b/cmd/mesh-controller/process_accounts.go index 2032697f..d7bede7a 100644 --- a/cmd/mesh-controller/process_accounts.go +++ b/cmd/mesh-controller/process_accounts.go @@ -94,15 +94,18 @@ func issueMissingProcessAccounts(ctx context.Context, inv *inventory.Inventory, if err != nil { busAddress = "" } - var ids []string + ctx, got := withIssued(ctx) for _, pr := range missing { if err := issueProcess(ctx, inv, m, pr, node, busAddress, known, reachable); err != nil { return "", err } - ids = append(ids, pr.Process) + } + issued := got.list() + if len(issued) == 0 { + return "", nil // another issuer sealed them meanwhile } return fmt.Sprintf("its process account(s) %s issued and sealed to %s, and arrive with the push", - strings.Join(ids, ", "), node), nil + strings.Join(issued, ", "), node), nil } // issueGainedProcessAccounts runs after a module's new definition is registered: on each machine assigned it, the diff --git a/internal/inventory/bususers.go b/internal/inventory/bususers.go index ee5c3be1..08374b35 100644 --- a/internal/inventory/bususers.go +++ b/internal/inventory/bususers.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "sync/atomic" + "time" "github.com/jackc/pgx/v5" "golang.org/x/crypto/bcrypt" @@ -52,6 +53,8 @@ type BusUser struct { // PasswordHash is what the composed file carries. The plaintext is returned once, by Mint, and // then exists only where it was sealed. PasswordHash string + // Sealed says its credential reached its node: an issue's mint is not sealed until then (ADR 0543 §2). + Sealed bool } // The kinds of bus user the mesh records. The same words the composer uses, so a row and a @@ -100,52 +103,126 @@ func (i *Inventory) RecordBusPassword(ctx context.Context, u BusUser, password s return i.writeBusUser(ctx, u, string(hash)) } -// writeBusUser is the row, whoever chose the password. +// writeBusUser is the row, whoever chose the password, taken as sealed. func (i *Inventory) writeBusUser(ctx context.Context, u BusUser, hash string) error { + return i.writeBusUserSealed(ctx, u, hash, true) +} + +// writeBusUserSealed is the row, sealed now or not yet. +func (i *Inventory) writeBusUserSealed(ctx context.Context, u BusUser, hash string, sealed bool) error { if _, err := i.store.Pool().Exec(ctx, - `insert into bus_user (username, kind, node, module, password_hash) - values ($1, $2, $3, $4, $5) + `insert into bus_user (username, kind, node, module, password_hash, sealed_at) + values ($1, $2, $3, $4, $5, case when $6 then now() end) on conflict (username) do update set kind = excluded.kind, node = excluded.node, module = excluded.module, - password_hash = excluded.password_hash, minted_at = now()`, - u.Username, u.Kind, u.Node, u.Module, hash); err != nil { + password_hash = excluded.password_hash, minted_at = now(), sealed_at = excluded.sealed_at`, + u.Username, u.Kind, u.Node, u.Module, hash, sealed); err != nil { return fmt.Errorf("cannot record the bus user %s: %w", u.Username, err) } return nil } -// MintBusPasswordIfAbsent mints u's password only when no row names u yet, in one statement: of two callers that both -// found the account missing, exactly one wins, and the other is told it lost (won false, no password) and must seal -// nothing, so the hash the bus holds is always the password the node is sealed (novox/hq ADR 0543, the review of -// issue 1945: the assign act and the assignments' watch once each minted, the later hash beside the earlier seal). -func (i *Inventory) MintBusPasswordIfAbsent(ctx context.Context, u BusUser) (string, bool, error) { - if u.Username == "" || u.Kind == "" { - return "", false, errors.New("a bus user needs a username and a kind") - } - raw := make([]byte, 32) - if _, err := rand.Read(raw); err != nil { - return "", false, fmt.Errorf("cannot generate a bus password: %w", err) - } - password := base64.RawURLEncoding.EncodeToString(raw) - hash, err := hashBusPassword(password) +// LockBusAccount holds the store's lock on one account until the answered release is called, so two issuers of it +// (the assign act, the assignments' watch, a registration's process accounts) never interleave their look, mint and +// seal: the hash the bus holds is always the password sealed to the node (novox/hq ADR 0543 §2, issue 1945). +func (i *Inventory) LockBusAccount(ctx context.Context, username string) (func(), error) { + conn, err := i.store.Pool().Acquire(ctx) if err != nil { - return "", false, fmt.Errorf("cannot hash a bus password: %w", err) + return nil, fmt.Errorf("the lock on %s cannot be taken: %w", username, err) } - tag, err := i.store.Pool().Exec(ctx, - `insert into bus_user (username, kind, node, module, password_hash) - values ($1, $2, $3, $4, $5) - on conflict (username) do nothing`, - u.Username, u.Kind, u.Node, u.Module, string(hash)) + if _, err := conn.Exec(ctx, `select pg_advisory_lock(hashtext($1))`, "bus_user/"+username); err != nil { + conn.Release() + return nil, fmt.Errorf("the lock on %s cannot be taken: %w", username, err) + } + return func() { + _, _ = conn.Exec(context.WithoutCancel(ctx), `select pg_advisory_unlock(hashtext($1))`, "bus_user/"+username) + conn.Release() + }, nil +} + +// ForgetBusAccountsOf forgets the bus accounts of module on node, and the credentials sealed to the node for them (its +// `broker`, each process's `broker-`, 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.`), 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 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 "", false, fmt.Errorf("cannot record the bus user %s: %w", u.Username, err) + return nil, err } - if tag.RowsAffected() == 0 { - return "", false, nil + 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) } - return password, true, nil + 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 nil, err + } + defer func() { _ = tx.Rollback(context.WithoutCancel(ctx)) }() + 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 ($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 names, tx.Commit(ctx) +} + +// MintBusPasswordUnsealed mints u's password, replacing whatever stands, with no seal yet: the account is not issued +// until MarkBusAccountSealed says its credential reached the node (ADR 0543 §2). +func (i *Inventory) MintBusPasswordUnsealed(ctx context.Context, u BusUser) (string, error) { + return i.mintBusPassword(ctx, u, false) +} + +// MarkBusAccountSealed says the credential last minted for username is sealed to its node. +func (i *Inventory) MarkBusAccountSealed(ctx context.Context, username string) error { + _, err := i.store.Pool().Exec(ctx, `update bus_user set sealed_at = now() where username = $1`, username) + return err +} + +// BusAccountIssued says whether an account is minted, and whether its credential is sealed to its node. +func (i *Inventory) BusAccountIssued(ctx context.Context, username string) (minted, sealed bool, err error) { + var at *time.Time + err = i.store.Pool().QueryRow(ctx, `select sealed_at from bus_user where username = $1`, username).Scan(&at) + if errors.Is(err, pgx.ErrNoRows) { + return false, false, nil + } + return err == nil, err == nil && at != nil, err } func (i *Inventory) MintBusPassword(ctx context.Context, u BusUser) (string, error) { + return i.mintBusPassword(ctx, u, true) +} + +// mintBusPassword mints, marked sealed when the caller seals it itself in the same act (every mint but an issue's). +func (i *Inventory) mintBusPassword(ctx context.Context, u BusUser, sealed bool) (string, error) { if u.Username == "" || u.Kind == "" { return "", errors.New("a bus user needs a username and a kind") } @@ -163,7 +240,7 @@ func (i *Inventory) MintBusPassword(ctx context.Context, u BusUser) (string, err return "", fmt.Errorf("cannot hash a bus password: %w", err) } - if err := i.writeBusUser(ctx, u, string(hash)); err != nil { + if err := i.writeBusUserSealed(ctx, u, string(hash), sealed); err != nil { return "", err } return password, nil @@ -176,7 +253,7 @@ func (i *Inventory) MintBusPassword(ctx context.Context, u BusUser) (string, err // password, and the composer refuses it rather than writing a user anybody is. func (i *Inventory) BusUsers(ctx context.Context) (map[string]BusUser, error) { rows, err := i.store.Pool().Query(ctx, - `select username, kind, node, module, password_hash from bus_user order by username`) + `select username, kind, node, module, password_hash, sealed_at is not null from bus_user order by username`) if err != nil { return nil, err } @@ -184,7 +261,7 @@ func (i *Inventory) BusUsers(ctx context.Context) (map[string]BusUser, error) { out := map[string]BusUser{} for rows.Next() { var u BusUser - if err := rows.Scan(&u.Username, &u.Kind, &u.Node, &u.Module, &u.PasswordHash); err != nil { + if err := rows.Scan(&u.Username, &u.Kind, &u.Node, &u.Module, &u.PasswordHash, &u.Sealed); err != nil { return nil, err } out[u.Username] = u diff --git a/internal/inventory/bususers_integration_test.go b/internal/inventory/bususers_integration_test.go index e04a2786..8b185f80 100644 --- a/internal/inventory/bususers_integration_test.go +++ b/internal/inventory/bususers_integration_test.go @@ -4,7 +4,9 @@ package inventory import ( "context" + "slices" "testing" + "time" "golang.org/x/crypto/bcrypt" ) @@ -63,48 +65,54 @@ func TestMintingAgainRotatesAndMintingIfAbsentHasOneWinner(t *testing.T) { t.Fatal("the previous password still verifies, so a rotation revoked nothing") } - // **Minting only if absent never rotates, and of several at once exactly one wins** (ADR 0543, the review of issue - // 1945): the assign act and the assignments' watch may both find an account missing; the winner's password is the - // one the store's hash verifies, so the one it seals is the one the bus accepts, and a loser seals nothing. - if _, won, err := inv.MintBusPasswordIfAbsent(ctx, aBusUser("shop")); err != nil || won { - t.Fatalf("a mint-if-absent over a minted account won %v (%v): it would replace a sealed password", won, err) + // **An issue's mint is not issued until its seal is marked, and forgotten with its assignment** (ADR 0543 §2, issue + // 1945, live on 2026-10-25 at 20:04): a row whose credential the node does not hold is not issued. + if minted, sealed, err := inv.BusAccountIssued(ctx, "one.shop"); err != nil || !minted || !sealed { + t.Fatalf("a mint that seals in its own act: minted %v sealed %v (%v)", minted, sealed, err) } - const racers = 8 - type minted struct { - password string - won bool - err error + if _, err := inv.MintBusPasswordUnsealed(ctx, aBusUser("racer")); err != nil { + t.Fatal(err) } - results := make(chan minted, racers) - for range racers { - go func() { - p, won, err := inv.MintBusPasswordIfAbsent(ctx, aBusUser("racer")) - results <- minted{p, won, err} - }() + if minted, sealed, err := inv.BusAccountIssued(ctx, "one.racer"); err != nil || !minted || sealed { + t.Fatalf("an issue's mint before its seal: minted %v sealed %v (%v)", minted, sealed, err) } - var winner string - wins := 0 - for range racers { - r := <-results - if r.err != nil { - t.Fatal(r.err) - } - if r.won { - wins++ - winner = r.password - } else if r.password != "" { - t.Fatal("a caller that lost was handed a password to seal") - } + if names, err := inv.BusMinted(ctx); err != nil || slices.Contains(names, "one.racer") { + t.Fatalf("an account not sealed counts as minted: %v (%v)", names, err) } - if wins != 1 { - t.Fatalf("%d of %d concurrent mints won, want exactly one", wins, racers) + if err := inv.MarkBusAccountSealed(ctx, "one.racer"); err != nil { + t.Fatal(err) } - stored, _, err := inv.BusUserHash(ctx, "one.racer") + if _, sealed, _ := inv.BusAccountIssued(ctx, "one.racer"); !sealed { + t.Fatal("an account marked sealed is not") + } + // The lock on an account holds a second issuer until the first lets go. + release, err := inv.LockBusAccount(ctx, "one.racer") if err != nil { t.Fatal(err) } - if err := bcrypt.CompareHashAndPassword([]byte(stored), []byte(winner)); err != nil { - t.Fatal("the stored hash is not the winner's password: the node would be sealed a password the bus refuses") + waiting := make(chan struct{}) + go func() { + again, err := inv.LockBusAccount(ctx, "one.racer") + if err == nil { + again() + } + close(waiting) + }() + select { + case <-waiting: + t.Fatal("a second issuer took the lock the first held") + case <-time.After(200 * time.Millisecond): + } + release() + <-waiting + if _, err := inv.ForgetBusAccountsOf(ctx, "one", "racer", true); err != nil { + t.Fatal(err) + } + if minted, _, _ := inv.BusAccountIssued(ctx, "one.racer"); minted { + t.Fatal("an unassigned module's account was kept") + } + if minted, _, _ := inv.BusAccountIssued(ctx, "one.shop"); !minted { + t.Fatal("forgetting one module's accounts forgot another's") } } diff --git a/internal/inventory/migrations/0108-a-bus-account-says-it-is-sealed.sql b/internal/inventory/migrations/0108-a-bus-account-says-it-is-sealed.sql new file mode 100644 index 00000000..da7da155 --- /dev/null +++ b/internal/inventory/migrations/0108-a-bus-account-says-it-is-sealed.sql @@ -0,0 +1,8 @@ +-- A bus account says when the credential minted for it was sealed to its node (novox/hq ADR 0543 §2, issue 1945). The +-- assign act and the assignments' watch issue an account only when it is not issued: a row with no sealed credential +-- is not issued. On 2026-10-25 at 20:04 ace's mesh-build-go had a row minted at 17:19 whose credential the node no +-- longer held; minting nothing, the mesh sealed nothing, and the module crash-looped for its missing credential. Null +-- while an issue mints and seals; set once the credential is handed over. Every account standing before this migration +-- is taken as sealed: each works or worked, and its own issue sealed it. +alter table bus_user add column sealed_at timestamptz; +update bus_user set sealed_at = minted_at; diff --git a/internal/inventory/read_state.go b/internal/inventory/read_state.go index 7b7591be..f826f9cf 100644 --- a/internal/inventory/read_state.go +++ b/internal/inventory/read_state.go @@ -226,8 +226,10 @@ func (i *Inventory) BusMinted(ctx context.Context) ([]string, error) { return nil, err } out := make([]string, 0, len(kept)) - for name := range kept { - out = append(out, name) + for name, u := range kept { + if u.Sealed { // an account whose credential is not sealed to its node is not issued (ADR 0543 §2) + out = append(out, name) + } } sort.Strings(out) return out, nil