From 013b7420f6f0b94799cfdf6e94e67ec79ec774ba Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 29 Oct 2026 21:18:21 +0100 Subject: [PATCH] mesh-grants: hold a grant only while the loaded list lacks its permissions, read from the part's own record of the lists it sealed, so a release whose bus tools are down still moves (issue 774) --- modules/mesh-grants/grants_test.go | 64 +++++--- modules/mesh-grants/internal/grants/grants.go | 144 +++++++++++++++--- modules/mesh-grants/internal/grants/part.go | 17 +-- modules/mesh-grants/internal/grants/sealed.go | 3 +- 4 files changed, 177 insertions(+), 51 deletions(-) diff --git a/modules/mesh-grants/grants_test.go b/modules/mesh-grants/grants_test.go index 81dfbe9..f6391bc 100644 --- a/modules/mesh-grants/grants_test.go +++ b/modules/mesh-grants/grants_test.go @@ -1994,15 +1994,17 @@ func TestGrants(t *testing.T) { t.Errorf("a module with no hashed account kept its grant: %+v, held %v", res.Grants, held()) } - // Issue 774: after take-over a new grant is written only once the list the bus loaded carries it. mod-x moves to a - // new trunk fingerprint while the list is held (users of the loaded list have no hash): it waits, said per key. + // Issue 774: after take-over a grant is written at once when the list the bus loaded carries its accounts' permissions, + // held list or not, and waits only when it adds one. (1) mod-x moves to a new fingerprint with the same permissions + // while the list is held (users of the loaded list have no hash): written. busKey, err := seal.GenerateKeyPair() if err != nil { t.Fatal(err) } loads := func(source, digest string) { b.put(grants.ReadHave, "a.nats", delivery.Have{V: delivery.Version, Phase: delivery.PhaseRunning, Health: delivery.HealthOK, - UserList: &delivery.UserListWritten{Source: source, Revision: 8, Wanted: digest, Digest: digest, Users: 6, Key: busKey.PublicKey}}) + UserList: &delivery.UserListWritten{Source: source, Revision: 8, Wanted: digest, Digest: digest, Users: 6, Key: busKey.PublicKey, + Follows: delivery.FollowGrants}}) } loads("a.nats.src2", "sha256:aa") p.Objects = &fakeObjects{} @@ -2010,27 +2012,55 @@ func TestGrants(t *testing.T) { if _, err := (ownStore{b}).Put(grants.StateTakeover, grants.KeySwitch, sw); err != nil { t.Fatal(err) } - fp2 := "src2:" + strings.Repeat("cd", 32) - key2, _ := delivery.PartGrantKey("mod-x", fp2) - b.put(grants.ReadModules, "mod-x", delivery.Module{V: delivery.Version, FP2: fp2, Manifest: delivery.ManifestDigest([]byte(xManifest)), Repository: "novox/x", Commit: "c"}) - res = p.Compose() - if _, written := b.own[grants.StateGrants][key2]; written || res.Sealed == nil || !strings.Contains(res.Sealed.Why, "held") || - len(res.Grants.Waits) != 1 || !strings.HasPrefix(res.Grants.Waits[0], key2+": waits for the bus to load") { - t.Fatalf("a held list let a new grant through: sealed %+v, grants %+v", res.Sealed, res.Grants) + moves := func(module, manifest, fp string) string { + digest := delivery.ManifestDigest([]byte(manifest)) + b.put(grants.ReadManifests, digest, delivery.Manifest{V: delivery.Version, Module: module, JSON: manifest}) + b.put(grants.ReadModules, module, delivery.Module{V: delivery.Version, FP2: fp, Manifest: digest, Repository: "novox/x", Commit: "c"}) + k, _ := delivery.PartGrantKey(module, fp) + return k } - // Every user hashed: the list is sealed, and the grants still wait while the bus has another list loaded. + written := func(k string) bool { _, ok := b.own[grants.StateGrants][k]; return ok } + keyX2 := moves("mod-x", xManifest, "src2:"+strings.Repeat("cd", 32)) + res = p.Compose() + if !written(keyX2) || res.Sealed == nil || !strings.Contains(res.Sealed.Why, "held") || len(res.Grants.Waits) != 0 { + t.Fatalf("a new fingerprint with the same permissions under a held list: sealed %+v, grants %+v", res.Sealed, res.Grants) + } + // Every user hashed: the list is sealed, and the part keeps its permissions. The bus loads it, and its tools go quiet. for _, u := range shadowOf(t, b) { hash(u.User) } - res = p.Compose() + p.Compose() sealed, _, _ := p.Sealed() - if _, written := b.own[grants.StateGrants][key2]; written || sealed.Digest == "" || len(res.Grants.Waits) != 2 { - t.Fatalf("a grant was written before the bus loaded the list carrying it: sealed %+v, grants %+v", sealed, res.Grants) + if sealed.Digest == "" { + t.Fatal("no list sealed once every user has a hash") } - // The bus loads the sealed list: both written, nothing waits. loads(delivery.GrantsUserListSource, sealed.Digest) - if res := p.Compose(); len(res.Grants.Waits) != 0 || !slices.Equal(res.Grants.Written, []string{key2, key("mod-y")}) { - t.Errorf("once the bus loaded the list carrying them: %+v", res.Grants) + b.refuseUsers = true + // (2) mod-y's trunk adds a state, so its account adds permissions: it waits, judged from the part's own record of the + // loaded list; mod-x at another fingerprint with the same permissions is written. + fp3 := "src2:" + strings.Repeat("ef", 32) + keyY3 := moves("mod-y", strings.Replace(yManifest, `"reads"`, `"state":[{"name":"fresh"}],"reads"`, 1), fp3) + keyX3 := moves("mod-x", xManifest, fp3) + res = p.Compose() + if !written(keyX3) || written(keyY3) || len(res.Grants.Waits) != 1 || + !strings.HasPrefix(res.Grants.Waits[0], keyY3+": waits for the bus to load a list carrying what it adds: b.mod-y") { + t.Fatalf("an added permission against the loaded list: %+v", res.Grants) + } + // A loaded list the part keeps no permissions of: every grant naming an account waits, said. + loads(delivery.GrantsUserListSource, "sha256:bb") + keyX4 := moves("mod-x", xManifest, "src2:"+strings.Repeat("12", 32)) + if res := p.Compose(); written(keyX4) || len(res.Grants.Waits) != 2 || !strings.Contains(res.Grants.Waits[0], "unknown") { + t.Fatalf("an unknown loaded list let a grant through: %+v", res.Grants) + } + // The bus loads a list carrying mod-y's state (its tools answer it): both written, nothing waits. + listRecs, _ := grants.ListRecords(grants.ReadInputs(b, &grants.Manifests{})) + now, err := grants.ComposeUsers(listRecs) + if err != nil { + t.Fatal(err) + } + b.refuseUsers, b.loaded = false, now + if res := p.Compose(); len(res.Grants.Waits) != 0 || !written(keyY3) || !written(keyX4) { + t.Errorf("once the loaded list carries them: %+v", res.Grants) } }) diff --git a/modules/mesh-grants/internal/grants/grants.go b/modules/mesh-grants/internal/grants/grants.go index 18c6fc0..92df498 100644 --- a/modules/mesh-grants/internal/grants/grants.go +++ b/modules/mesh-grants/internal/grants/grants.go @@ -1,6 +1,7 @@ package grants import ( + "encoding/json" "fmt" "slices" "sort" @@ -40,28 +41,121 @@ type GrantsWritten struct { } // GrantWaits says why a grant composed now, new or changed, is not written yet, or "" when it may be (issue 774). The -// assigner moves a module's want to the fingerprint once its grant is there, so a grant is the word that the bus's user list -// in force already carries the module's subjects at that fingerprint. After take-over that is so when the list the bus -// node says it loaded is the grants part's own and is the list composed now (composed: its digest), which ListRecords -// composes from what runs and what the trunk names; a list held, not yet sealed, or not yet loaded is not it. In shadow -// nobody reads the part's grants and the controller's list is in force, so nothing waits. Pure. -func GrantWaits(tookOver bool, loaded *delivery.UserListWritten, composed string) string { - switch { - case !tookOver: +// assigner moves a module's want to the fingerprint once its grant is there, so the grant is the word that the list the bus +// has loaded already carries the module's accounts as composed now (composed, from ListRecords: what runs and what the trunk +// names): every publish and subscribe allowed, the allowance of responses, and no deny the composed account lacks. A grant +// that adds no permission is written at once, whatever list is held or pending; one that adds any waits until a list that +// carries it is loaded. loaded is the loaded list by user, nil when its contents are unknown (unknown says why): then only +// a grant naming no account is written. In shadow nobody reads the part's grants and nothing waits. Pure. +func GrantWaits(tookOver bool, accounts []string, composed, loaded map[string]User, unknown string) string { + if !tookOver || len(accounts) == 0 { return "" - case composed == "": - return "waits: the user list could not be composed, so no list carries it" - case loaded == nil: - return "waits for the bus node to say which user list it loaded" - case loaded.Source != delivery.GrantsUserListSource: - return "waits for the bus to load the grants part's list " + short(composed) + ": it loaded " + loaded.Source + "'s" - case loaded.Error != "": - return "waits for the bus to load the list " + short(composed) + ": its node could not write it: " + loaded.Error - case loaded.Digest != composed: - return "waits for the bus to load the list " + short(composed) + ", which carries it: it has " + short(loaded.Digest) + - " loaded (a list held, or sealed and not yet loaded)" } - return "" + if loaded == nil { + return "waits: the contents of the list the bus loaded are unknown (" + unknown + "), so no grant adding an account is written" + } + var adds []string + for _, user := range accounts { + c, ok := composed[user] + if !ok { + continue + } + l, ok := loaded[user] + if !ok { + adds = append(adds, user+" (not in the loaded list)") + continue + } + n := len(minus(c.Publish, l.Publish)) + len(minus(c.Subscribe, l.Subscribe)) + if c.AllowResponses && !l.AllowResponses { + n++ + } + if l.PublishDeny != nil { + n += len(minus(l.PublishDeny, c.PublishDeny)) + } + if n > 0 { + adds = append(adds, fmt.Sprintf("%s (%d permissions)", user, n)) + } + } + if len(adds) == 0 { + return "" + } + return "waits for the bus to load a list carrying what it adds: " + strings.Join(adds, ", ") +} + +// KeySealedGrants is the key of the state user-list holding, per list the part sealed (the last two, by digest), each +// user's permissions as sealed, never a hash: what tells which grants the list the bus loaded carries, without the bus +// module's tools (issue 774). +const KeySealedGrants = "sealed-grants" + +// SealedGrants is the value of KeySealedGrants. +type SealedGrants struct { + Lists []SealedList `json:"lists"` +} + +// SealedList is one sealed list's users and their permissions. +type SealedList struct { + Digest string `json:"digest"` + Users []User `json:"users"` +} + +// keepSealedGrants records the permissions of the list sealed now, beside the one before. +func (p *Part) keepSealedGrants(res *Result, digest string, list []byte, composed []User) { + in := listUsers(list) + var users []User + for _, u := range composed { + if slices.Contains(in, u.User) { + u.Hash = "" + users = append(users, u) + } + } + var was SealedGrants + if raw, _, found, err := p.Store.Get(StateUserList, KeySealedGrants); err == nil && found { + _ = json.Unmarshal(raw, &was) + } + next := SealedGrants{Lists: []SealedList{{Digest: digest, Users: users}}} + for _, l := range was.Lists { + if l.Digest != digest && len(next.Lists) < 2 { + next.Lists = append(next.Lists, l) + } + } + data, _ := json.Marshal(next) + if _, err := p.Store.Put(StateUserList, KeySealedGrants, data); err != nil { + res.Errors = append(res.Errors, fmt.Sprintf("its own %s %s cannot be written: %v", StateUserList, KeySealedGrants, err)) + } +} + +// loadedContents is the list the bus loaded, by user: as the bus module's nats_users read it, else as the part sealed the +// list of the digest the bus node says it loaded from the part's key; nil and why when neither says. +func (p *Part) loadedContents(in Inputs, read []User, readErr error) (map[string]User, string) { + byUser := func(us []User) map[string]User { + out := map[string]User{} + for _, u := range us { + out[u.User] = u + } + return out + } + if readErr == nil { + return byUser(read), "" + } + l := in.Loaded + switch { + case l == nil: + return nil, "the bus node says no loaded list, and nats_users: " + readErr.Error() + case l.Source != delivery.GrantsUserListSource || l.Follows != delivery.FollowGrants || l.Error != "": + return nil, "the bus loaded no list of the grants part's (source " + l.Source + ", follows " + l.Follows + "), and nats_users: " + readErr.Error() + } + raw, _, found, err := p.Store.Get(StateUserList, KeySealedGrants) + if err == nil && found { + var sg SealedGrants + if json.Unmarshal(raw, &sg) == nil { + for _, sl := range sg.Lists { + if sl.Digest == l.Digest { + return byUser(sl.Users), "" + } + } + } + } + return nil, "the part keeps no permissions of the loaded list " + short(l.Digest) + " (sealed before it kept them), and nats_users: " + readErr.Error() } // GrantMissingCondition is the key of the warning raised while a placed module has no grant (issue 1126). @@ -203,7 +297,7 @@ func grantsReadFailed(in Inputs) string { // writeGrants writes each grant whose key is absent or differs and says grant-written for each, and deletes the key of a // module that has none now. -func (p *Part) writeGrants(res *Result, in Inputs, recs buscompose.Records, composed string) { +func (p *Part) writeGrants(res *Result, in Inputs, recs buscompose.Records, composed []User, loaded map[string]User, unknown string) { if why := grantsReadFailed(in); why != "" { res.Grants = &GrantsWritten{Kept: []string{"no grant is written or deleted in a pass that could not read " + why}} return @@ -220,7 +314,11 @@ func (p *Part) writeGrants(res *Result, in Inputs, recs buscompose.Records, comp done := &GrantsWritten{Wanted: len(want), Missing: MissingGrants(in, want, drop, skip)} res.Grants = done p.sayMissing(done.Missing) - waits := GrantWaits(p.SwitchState() == TookOver, in.Loaded, composed) + tookOver := p.SwitchState() == TookOver + byUser := map[string]User{} + for _, u := range composed { + byUser[u.User] = u + } for _, key := range sortedKeys(want) { g := want[key] data, err := delivery.EncodeControllerGrant(g) @@ -236,7 +334,7 @@ func (p *Part) writeGrants(res *Result, in Inputs, recs buscompose.Records, comp continue } } - if waits != "" { + if waits := GrantWaits(tookOver, g.Accounts, byUser, loaded, unknown); waits != "" { done.Waits = append(done.Waits, key+": "+waits) continue } diff --git a/modules/mesh-grants/internal/grants/part.go b/modules/mesh-grants/internal/grants/part.go index 1e306be..ca23375 100644 --- a/modules/mesh-grants/internal/grants/part.go +++ b/modules/mesh-grants/internal/grants/part.go @@ -251,10 +251,10 @@ func (p *Part) Compose() Result { } users = WithHashes(users, in.Hashes) st.Digest, st.Users = Digest(users), len(users) - loaded, err := LoadedUsers(p.Call) + loaded, loadErr := LoadedUsers(p.Call) st.Compare = Compare(users, loaded, Placed(in)).Excusing(ExcusedBy(MultiHeld(listRecs))) - if err != nil { - st.Compare.Unread = err.Error() + if loadErr != nil { + st.Compare.Unread = loadErr.Error() } st.Credentials = CredentialsOf(users, loaded, in.Hashes) st.Inputs = st.inputs() @@ -280,14 +280,11 @@ func (p *Part) Compose() Result { p.writeAccounts(&res, in, listRecs) } list, n, leftOut, err := ComposeList(listRecs, in.Hashes) - composed := "" - if err == nil { - composed = delivery.UserListDigest(list) - } // The grants the assigner reads once the lease is handed away, in shadow until then (ADR 0547 §4, §5); after take-over - // each new one only once the bus loaded a list that carries it (issue 774). + // one that adds a permission only once the bus loaded a list that carries it (issue 774). if p.Store != nil { - p.writeGrants(&res, in, listRecs, composed) + inForce, unknown := p.loadedContents(in, loaded, loadErr) + p.writeGrants(&res, in, listRecs, users, inForce, unknown) } // The list itself, sealed to the bus node and named in `user-list.sealed` (ADR 0547 clause 7). if err != nil { @@ -299,7 +296,7 @@ func (p *Part) Compose() Result { bus = names(loaded) lost = holdings.LastHolders(holdings.ByUsers(loaded), holdings.ByUsers(users)) } - p.writeSealed(&res, list, n, leftOut, in, bus, lost) + p.writeSealed(&res, list, n, leftOut, in, bus, lost, users) } ms := Memberships(recs) res.Memberships = len(ms) diff --git a/modules/mesh-grants/internal/grants/sealed.go b/modules/mesh-grants/internal/grants/sealed.go index 55ff811..097e09d 100644 --- a/modules/mesh-grants/internal/grants/sealed.go +++ b/modules/mesh-grants/internal/grants/sealed.go @@ -74,7 +74,7 @@ func followOf(state string) string { // writeSealed puts the list sealed to the bus node and writes the key, each only when what it says changed. It says in // res what it wrote, or why it wrote nothing. -func (p *Part) writeSealed(res *Result, list []byte, users int, leftOut []string, in Inputs, bus []string, lost []string) { +func (p *Part) writeSealed(res *Result, list []byte, users int, leftOut []string, in Inputs, bus []string, lost []string, composed []User) { cur, _, had := p.Sealed() next := cur next.Follow = followOf(p.SwitchState()) @@ -137,6 +137,7 @@ func (p *Part) writeSealed(res *Result, list []byte, users int, leftOut []string } if next.Digest == digest && cur.Digest != digest { p.keepWrittenUsers(res, list, in.Hashes) + p.keepSealedGrants(res, digest, list, composed) p.acceptedWritten(res, digest) } // The object the key named before is taken away once the key names the new one: the bucket keeps the list in force.