diff --git a/modules/mesh-delivery-indexer/judge/rules.go b/modules/mesh-delivery-indexer/judge/rules.go index 1a981af2..9a56f0a7 100644 --- a/modules/mesh-delivery-indexer/judge/rules.go +++ b/modules/mesh-delivery-indexer/judge/rules.go @@ -451,6 +451,28 @@ func busyProblems(m catalogue.Manifest) []string { return problems } +// readOnlyStateProblems refuses a state declared read-only that no process's bus state names: the module's own account +// and the tool runner carrying it read it and publish nothing on it, so only such a process writes it. +func readOnlyStateProblems(m catalogue.Manifest) []string { + named := map[string]bool{} + for _, a := range m.BusAccounts() { + if a.Tools { + continue + } + for _, s := range a.Bus.State { + named[s] = true + } + } + var problems []string + for _, s := range m.State { + if s.ReadOnly && !named[s.Name] { + problems = append(problems, fmt.Sprintf("%s declares its state %s read-only and no process's bus state names it: "+ + "only such a process writes a read-only state, so nothing would", m.Module, s.Name)) + } + } + return problems +} + // AttributeFits is whether an item's attribute is of the shape its collection allows: words of lower case, except // the environment's `name`, which is a variable's name (design 57 L3, issue 795). The same rule is mesh-catalog's // provisioner client's, and the inventory's. @@ -3312,6 +3334,9 @@ func parseManifest(raw []byte, predefined catalogue.PredefinedList) (catalogue.M problems = append(problems, healthProblems(m)...) // And whether a container is busy (novox/hq ADR 0335), likewise. problems = append(problems, busyProblems(m)...) + // A read-only state is written by a process naming it in its bus state alone (review round 2): one no process names + // would be written by nobody. + problems = append(problems, readOnlyStateProblems(m)...) if m.DeclaresConfig() { // Said in the words it was written in: the rules shared with the settings form say ${setting:} // and takes-settings. diff --git a/modules/mesh-delivery-indexer/judge/rules_test.go b/modules/mesh-delivery-indexer/judge/rules_test.go index e8634ae6..7fbffcd9 100644 --- a/modules/mesh-delivery-indexer/judge/rules_test.go +++ b/modules/mesh-delivery-indexer/judge/rules_test.go @@ -2928,7 +2928,7 @@ func TestAStateNameIsLocalAndOneToken(t *testing.T) { {`{"module":"a.b","version":"1","state":["s"]}`, `no dot`}, {`{"module":"a","version":"1","reads":["bindings"]}`, `a read is .`}, {`{"module":"a","version":"1","reads":["a.s"]}`, `which is its own state`}, - {`{"module":"a","version":"1","state":[{"name":"s","shared":true}]}`, `{name, history, ttl-seconds, per-machine, key-ttl}`}, + {`{"module":"a","version":"1","state":[{"name":"s","shared":true}]}`, `{name, history, ttl-seconds, per-machine, key-ttl, read-only}`}, } { _, err := ParseManifest([]byte(c.manifest)) if c.says == "" { @@ -4353,8 +4353,15 @@ func TestTheBussAccountSaysNothingOnTheBus(t *testing.T) { {"the keeper invokes its own tool", `{"keeps":true,"invokes":["nats.nats_server"]}`, "", "a subject on which a tool"}, {"the keeper invokes every tool", `{"keeps":true,"invokes":["*"]}`, "", "a subject on which a tool"}, {"the keeper invokes its own seat's verb", `{"keeps":true,"invokes":["seat:mesh-broker.list"]}`, "", "a subject on which a tool"}, + // Review round 2: a read-only state is named by the process that writes it; one no process names is refused. + {"spent read-only, the keeper's", `{"keeps":true,"state":["requests","spent"]}`, "", ""}, + {"spent read-only, no process's", `{"keeps":true,"state":["requests"]}`, "", "its state spent read-only and no process's bus state names it"}, } { - _, err := ParseManifest(keeper(c.bus, c.more)) + raw := keeper(c.bus, c.more) + if strings.HasPrefix(c.name, "spent") { + raw = []byte(strings.Replace(string(raw), `"state":[{"name":"requests","history":5}]`, `"state":[{"name":"requests","history":5},{"name":"spent","read-only":true}]`, 1)) + } + _, err := ParseManifest(raw) if c.refused == "" && err != nil || c.refused != "" && (err == nil || !strings.Contains(err.Error(), c.refused)) { t.Errorf("%s: want refused %q, got %v", c.name, c.refused, err) } diff --git a/modules/mesh-grants/grants_test.go b/modules/mesh-grants/grants_test.go index 27dd3585..aac08d80 100644 --- a/modules/mesh-grants/grants_test.go +++ b/modules/mesh-grants/grants_test.go @@ -548,6 +548,10 @@ func TestGrants(t *testing.T) { }{ {"every record on the bus", func(*fakeBus) {}, []string{"a.mod-x", "a.nats", "b.mod-y", "node.a", "node.b"}, "people"}, + // Review round 2: a state the manifest declares read-only is read on the module's own account and never written. + {"a read-only state is read and not written by its module's account", func(b *fakeBus) { + b.manifestOf("mod-x", strings.Replace(xManifest, `{"name":"kept","history":3}`, `{"name":"kept","history":3,"read-only":true}`, 1)) + }, []string{"a.mod-x", "a.nats", "b.mod-y", "node.a", "node.b"}, "people"}, {"a manifest missing is a gap and its user is left out", func(b *fakeBus) { delete(b.states[grants.ReadModules], "mod-y") }, []string{"a.mod-x", "a.nats", "node.a", "node.b"}, "manifest of mod-y"}, {"a want's built manifest is composed, not the trunk's; the list's records carry both (issue 774)", func(b *fakeBus) { @@ -577,6 +581,12 @@ func TestGrants(t *testing.T) { if !gapNamed(append(in.Gaps, left...), row.gap) { t.Errorf("gaps %v name no %q", in.Gaps, row.gap) } + if strings.HasPrefix(row.name, "a read-only state") { + x := users[slices.IndexFunc(users, func(u grants.User) bool { return u.User == "a.mod-x" })] + if slices.Contains(x.Publish, "$KV.mod-x_kept.>") || !slices.Contains(x.Publish, "$JS.API.DIRECT.GET.KV_mod-x_kept.>") { + t.Errorf("a.mod-x and its read-only state kept: %v", x.Publish) + } + } if strings.HasPrefix(row.name, "a want's") { x := users[slices.IndexFunc(users, func(u grants.User) bool { return u.User == "a.mod-x" })] if !slices.Contains(x.Publish, "$KV.mod-x_newer.>") || slices.Contains(x.Publish, "$KV.mod-x_kept.>") { diff --git a/modules/mesh-grants/internal/grants/records.go b/modules/mesh-grants/internal/grants/records.go index 511a47e9..651b68ed 100644 --- a/modules/mesh-grants/internal/grants/records.go +++ b/modules/mesh-grants/internal/grants/records.go @@ -244,7 +244,7 @@ func bucketsOf(m catalogue.Manifest) []buscompose.Bucket { var out []buscompose.Bucket for _, s := range m.State { out = append(out, buscompose.Bucket{Module: m.Module, Name: s.Name, History: s.History, TTLSeconds: s.TTLSeconds, - PerMachine: s.PerMachine, KeyTTL: s.KeyTTL}) + PerMachine: s.PerMachine, KeyTTL: s.KeyTTL, ReadOnly: s.ReadOnly}) } return out }