Refuse the lease write's check when the indexer's manifests were not all read (issue 774)
mesh/delivery merged: source.main moved to novox/mesh-delivery@e559798972b4
mesh/check being carried to 2dd5239726ca on e559798972b4 | from d409bdf20f48
mesh/delivery merged: source.main moved to novox/mesh-delivery@e559798972b4
mesh/check being carried to 2dd5239726ca on e559798972b4 | from d409bdf20f48
Review of 09791f5: with the indexer's modules bucket readable but empty, nothing was read, so
nothing was found and both phases passed; the operator could have handed the lease away while an
unkept read still stood. Judge now refuses when no manifest was read, or fewer of the indexer's
modules than it holds (a provided manifest does not count for a missing one), saying why; a
missing or refused manifest is named as before, now under test.
The controller's grants leave the keep list: ADR 0571 §9 has no manifest read them from L7's
delivery, changing ADR 0567 §10 and ADR 0547 §5's shadow compare, so the assigner's and the grants
part's reads are refused, and a refusal names the record that ended a declaration where one does.
Quick fix: the counts in the grants part's Judge, as here. Thorough fix: the same coverage rule
in the shared mesh-sdk derivation once the keep list moves there; the quick one stands until then.
This commit is contained in:
@@ -81,9 +81,7 @@ var KeepList = []Kept{
|
||||
{Module: "mesh-delivery-assigner", Kind: DependsInvoke, What: "seat:mesh-controller.unpin", Record: "ADR 0567 §8"},
|
||||
{Module: "mesh-delivery-assigner", Kind: DependsRead, What: "mesh-controller.assignments", Record: "ADR 0567 §8"},
|
||||
{Module: "mesh-delivery-assigner", Kind: DependsRead, What: "mesh-controller.composed", Record: "ADR 0571 §8"},
|
||||
{Module: "mesh-delivery-assigner", Kind: DependsRead, What: "mesh-controller.grants", Record: "ADR 0567 §10"},
|
||||
{Module: "mesh-grants", Kind: DependsRead, What: "mesh-controller.assignments", Record: "ADR 0549 §5"},
|
||||
{Module: "mesh-grants", Kind: DependsRead, What: "mesh-controller.grants", Record: "ADR 0547 §5"},
|
||||
{Module: "mesh-grants", Kind: DependsConsume, What: "mesh-controller.condition-raised", Record: "ADR 0564 §9"},
|
||||
{Module: "mesh-grants", Kind: DependsConsume, What: "mesh-controller.condition-cleared", Record: "ADR 0564 §9"},
|
||||
{Module: "messenger", Kind: DependsConsume, What: "mesh-controller.condition-raised", Record: "ADR 0564 §9"},
|
||||
@@ -98,6 +96,18 @@ var KeepList = []Kept{
|
||||
{Module: "mesh-hand-acts", Kind: DependsRead, What: "mesh-controller.hand-acts", Record: "ADR 0565 §11"},
|
||||
}
|
||||
|
||||
// EndedBy names, for a declaration no entry keeps, the record that already ended it, so a refusal says where it moved.
|
||||
// The controller's `grants` is one: ADR 0571 §9 has no manifest read it from L7's delivery, which changes ADR 0567 §10
|
||||
// and ADR 0547 §5's shadow compare, so neither the assigner's nor the grants part's read of it is kept.
|
||||
var EndedBy = map[string]string{
|
||||
DependsRead + " mesh-controller.grants": "ADR 0571 §9: no manifest reads the controller's grants; the grants part's key is the one read",
|
||||
DependsInvoke + " seat:mesh-controller.hand-act": "ADR 0565 §11: every recorder calls mesh-hand-acts",
|
||||
DependsInvoke + " seat:mesh-controller.warranted": "ADR 0565 §11: every recorder calls mesh-hand-acts",
|
||||
DependsInvoke + " seat:mesh-controller.root-free": "ADR 0571 §11: root-free is the self-check's probe",
|
||||
DependsInvoke + " seat:mesh-controller.held": "ADR 0571 §2: held is the seat mesh-artifact-sweep's",
|
||||
DependsRead + " mesh-controller.declared-packages": "ADR 0565 §6: each node agent writes its own node's packages",
|
||||
}
|
||||
|
||||
// keptFor is the list's entry for one dependency, or nil.
|
||||
func keptFor(d Dependency) *Kept {
|
||||
for i, k := range KeepList {
|
||||
@@ -198,15 +208,29 @@ type Verdict struct {
|
||||
Reasons []string `json:"reasons"`
|
||||
}
|
||||
|
||||
// Coverage is how much of the indexer the dependencies were derived from: the manifests read, the modules the indexer
|
||||
// holds (-1 when its keys could not be listed, which Unread then names), and each manifest that could not be read.
|
||||
type Coverage struct {
|
||||
Read, Indexed int
|
||||
Unread []Gap
|
||||
}
|
||||
|
||||
// Judge runs the lease write's check over the dependencies (ADR 0571 §13) in one phase: before the lease write's own
|
||||
// delivery, every dependency must be on the named list; at the write, only the three kept to the write itself may stay.
|
||||
// A manifest that could not be read fails it too, since it could hide a dependency; and a lease already handed away
|
||||
// leaves nothing to write.
|
||||
func Judge(deps []Dependency, phase string, unread []Gap, handedAway bool, leaseSays string) Verdict {
|
||||
// It passes only over every module the indexer holds: no manifest read, fewer read than the indexer holds, or one that
|
||||
// could not be read fails it, since what was not read could hide a dependency. A lease already handed away leaves
|
||||
// nothing to write.
|
||||
func Judge(deps []Dependency, phase string, cov Coverage, handedAway bool, leaseSays string) Verdict {
|
||||
v := Verdict{Phase: phase, Reasons: []string{}}
|
||||
if handedAway {
|
||||
v.Reasons = append(v.Reasons, "there is nothing to write: "+leaseSays)
|
||||
}
|
||||
switch {
|
||||
case cov.Read == 0:
|
||||
v.Reasons = append(v.Reasons, fmt.Sprintf("no manifest was read from the indexer (it holds %d modules): with nothing read, nothing on the controller can be judged absent", max(cov.Indexed, 0)))
|
||||
case cov.Indexed >= 0 && cov.Read < cov.Indexed:
|
||||
v.Reasons = append(v.Reasons, fmt.Sprintf("only %d of the %d modules the indexer holds had their manifest read, so what the others declare on the controller is not known", cov.Read, cov.Indexed))
|
||||
}
|
||||
for _, d := range deps {
|
||||
who := d.Module
|
||||
if d.By != "" {
|
||||
@@ -214,12 +238,16 @@ func Judge(deps []Dependency, phase string, unread []Gap, handedAway bool, lease
|
||||
}
|
||||
switch {
|
||||
case d.Kept == "":
|
||||
v.Reasons = append(v.Reasons, fmt.Sprintf("%s declares %s %s, which no record keeps until the lease write (ADR 0571 §13)", who, d.Kind, d.What))
|
||||
why := fmt.Sprintf("%s declares %s %s, which no record keeps until the lease write (ADR 0571 §13)", who, d.Kind, d.What)
|
||||
if ended := EndedBy[d.Kind+" "+d.What]; ended != "" {
|
||||
why += "; ended by " + ended
|
||||
}
|
||||
v.Reasons = append(v.Reasons, why)
|
||||
case phase == PhaseWrite && !d.ToTheWrite:
|
||||
v.Reasons = append(v.Reasons, fmt.Sprintf("%s declares %s %s, kept by %s only until the lease write's own delivery, which drops it", who, d.Kind, d.What, d.Kept))
|
||||
}
|
||||
}
|
||||
for _, g := range unread {
|
||||
for _, g := range cov.Unread {
|
||||
v.Reasons = append(v.Reasons, fmt.Sprintf("%s could not be read (%s), so what it declares on the controller is not known", g.Input, g.Why))
|
||||
}
|
||||
v.Passes = len(v.Reasons) == 0
|
||||
@@ -378,7 +406,9 @@ type LeaseAnswer struct {
|
||||
// Unkept are the dependencies no record keeps, by module: what the check refuses in every phase.
|
||||
Unkept map[string][]string `json:"unkept"`
|
||||
Manifests int `json:"manifests_read"`
|
||||
Unread []Gap `json:"manifests_unread,omitempty"`
|
||||
// Indexed is how many modules the indexer holds; -1 when its keys could not be listed.
|
||||
Indexed int `json:"modules_indexed"`
|
||||
Unread []Gap `json:"manifests_unread,omitempty"`
|
||||
// Streams are the controller's streams someone consumes now; Idle those no one does.
|
||||
Streams []StreamUse `json:"streams,omitempty"`
|
||||
Idle []string `json:"streams_idle,omitempty"`
|
||||
@@ -432,7 +462,18 @@ func (p *Part) LeaseCheck() (any, error) {
|
||||
} else {
|
||||
out.Streams, out.Idle = used, idle
|
||||
}
|
||||
out.Check = Judge(out.Dependencies, PhaseWrite, out.Unread, out.Lease.HandedAway, out.Lease.Says)
|
||||
out.Before = Judge(out.Dependencies, PhaseBefore, out.Unread, out.Lease.HandedAway, out.Lease.Says)
|
||||
// Only the indexer's own modules count as read: a manifest the mesh provides itself must not stand in for one missing.
|
||||
cov := Coverage{Indexed: -1, Unread: out.Unread}
|
||||
if keys, err := p.Read.Keys(ReadModules); err == nil {
|
||||
cov.Indexed = len(keys)
|
||||
for _, k := range keys {
|
||||
if _, ok := in.Manifests[k]; ok {
|
||||
cov.Read++
|
||||
}
|
||||
}
|
||||
}
|
||||
out.Indexed = cov.Indexed
|
||||
out.Check = Judge(out.Dependencies, PhaseWrite, cov, out.Lease.HandedAway, out.Lease.Says)
|
||||
out.Before = Judge(out.Dependencies, PhaseBefore, cov, out.Lease.HandedAway, out.Lease.Says)
|
||||
return out, nil
|
||||
}
|
||||
|
||||
@@ -6,6 +6,8 @@ import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"git.novox.be/novox/mesh-sdk/go/delivery"
|
||||
|
||||
"mesh-grants/internal/grants"
|
||||
)
|
||||
|
||||
@@ -13,6 +15,7 @@ import (
|
||||
// the controller's lease, every manifest's dependency on the controller with what keeps it, the controller's streams in
|
||||
// use, and the lease write's check run dry, at the write (the three kept to it alone) and before its delivery.
|
||||
func TestTheLeaseCheckJudgesEveryDependencyOnTheController(t *testing.T) {
|
||||
const mManifest = `{"module":"mod-m","version":"1","reads":["mesh-controller.declared-packages"]}`
|
||||
controllerHolds := map[string]any{"instance": "controller@novox-be pid 1 since 2026-10-30T14:52:10Z", "epoch": 60,
|
||||
"taken": "2026-10-30T14:52:10Z", "renewed": "2026-10-30T22:00:00Z"}
|
||||
handedAway := map[string]any{"handed-to": "the operator's hand-over", "handed-by": "operator"}
|
||||
@@ -21,6 +24,7 @@ func TestTheLeaseCheckJudgesEveryDependencyOnTheController(t *testing.T) {
|
||||
writePasses bool
|
||||
beforePasses bool
|
||||
reason string // a reason the check at the write must name, or ""
|
||||
bothPhases bool // the check before the delivery must name the reason too
|
||||
handedAway bool
|
||||
durable []string // durable consumers of KV_mesh-controller_assignments
|
||||
}
|
||||
@@ -29,37 +33,49 @@ func TestTheLeaseCheckJudgesEveryDependencyOnTheController(t *testing.T) {
|
||||
manifests map[string]string
|
||||
lease map[string]any
|
||||
want want
|
||||
setup func(b *fakeBus)
|
||||
}{
|
||||
{"a read of the lease alone is no dependency, and the controller's own manifest is not judged",
|
||||
map[string]string{"mod-l": `{"module":"mod-l","version":"1","reads":["mesh-controller.lease"]}`,
|
||||
"mesh-controller": `{"module":"mesh-controller","version":"1","invokes":["seat:mesh-controller.status"]}`},
|
||||
controllerHolds, want{writePasses: true, beforePasses: true}},
|
||||
controllerHolds, want{writePasses: true, beforePasses: true}, nil},
|
||||
{"the builder's consume of artifacts-gone is kept to the write itself",
|
||||
map[string]string{"mesh-delivery-builder": `{"module":"mesh-delivery-builder","version":"1","consumes":["mesh-controller.artifacts-gone"]}`},
|
||||
controllerHolds, want{deps: []string{"mesh-delivery-builder consume mesh-controller.artifacts-gone kept"}, writePasses: true, beforePasses: true}},
|
||||
controllerHolds, want{deps: []string{"mesh-delivery-builder consume mesh-controller.artifacts-gone kept"}, writePasses: true, beforePasses: true}, nil},
|
||||
{"the assigner's read of composed is kept until the write's delivery, which drops it",
|
||||
map[string]string{"mesh-delivery-assigner": `{"module":"mesh-delivery-assigner","version":"1","reads":["mesh-controller.composed","mesh-controller.lease"]}`},
|
||||
controllerHolds, want{deps: []string{"mesh-delivery-assigner read mesh-controller.composed kept"}, beforePasses: true,
|
||||
reason: "mesh-delivery-assigner declares read mesh-controller.composed, kept by ADR 0571 §8 only until the lease write's own delivery"}},
|
||||
reason: "mesh-delivery-assigner declares read mesh-controller.composed, kept by ADR 0571 §8 only until the lease write's own delivery"}, nil},
|
||||
{"a read no record keeps fails both phases, named",
|
||||
map[string]string{"pacman": `{"module":"pacman","version":"1","reads":["mesh-controller.declared-packages"]}`},
|
||||
controllerHolds, want{deps: []string{"pacman read mesh-controller.declared-packages unkept"},
|
||||
reason: "pacman declares read mesh-controller.declared-packages, which no record keeps until the lease write"}},
|
||||
reason: "pacman declares read mesh-controller.declared-packages, which no record keeps until the lease write"}, nil},
|
||||
{"a seat verb and a module tool of the controller's, a consume and a wait are each one; every tool (*) is none",
|
||||
map[string]string{"mod-i": `{"module":"mod-i","version":"1","invokes":["seat:mesh-controller.root-free","mesh-controller.status","*","nats.nats_users"],` +
|
||||
`"consumes":["mesh-controller.checked","mod-x.thing"],"uses":["mesh-controller"]}`,
|
||||
"node-tools": `{"module":"node-tools","version":"1","waits":["mesh-controller.checked"]}`},
|
||||
controllerHolds, want{deps: []string{"mod-i consume mesh-controller.checked unkept", "mod-i invoke mesh-controller.status unkept",
|
||||
"mod-i invoke seat:mesh-controller.root-free unkept", "mod-i uses mesh-controller unkept", "node-tools waits mesh-controller.checked unkept"},
|
||||
reason: "mod-i declares invoke seat:mesh-controller.root-free, which no record keeps"}},
|
||||
reason: "mod-i declares invoke seat:mesh-controller.root-free, which no record keeps"}, nil},
|
||||
{"a process's own invoke of the controller is its module's dependency, named by the process",
|
||||
map[string]string{"mod-p": `{"module":"mod-p","version":"1","resources":[{"id":"account","type":"user","name":"mod-p"},{"id":"worker","type":"process","run":["./w"],"user":"mod-p",` +
|
||||
`"bus":{"invokes":["seat:mesh-controller.held"]}}]}`},
|
||||
controllerHolds, want{deps: []string{"mod-p invoke seat:mesh-controller.held unkept"},
|
||||
reason: "mod-p (process worker) declares invoke seat:mesh-controller.held"}},
|
||||
reason: "mod-p (process worker) declares invoke seat:mesh-controller.held"}, nil},
|
||||
{"no manifest read from the indexer fails both phases, though nothing read depends on the controller",
|
||||
map[string]string{},
|
||||
controllerHolds, want{reason: "no manifest was read from the indexer (it holds 0 modules)", bothPhases: true}, nil},
|
||||
{"a module whose manifest digest is missing fails both phases, named",
|
||||
map[string]string{"mod-l": `{"module":"mod-l","version":"1"}`, "mod-m": mManifest},
|
||||
controllerHolds, want{reason: "manifest of mod-m could not be read", bothPhases: true}, func(b *fakeBus) {
|
||||
delete(b.states[grants.ReadManifests], delivery.ManifestDigest([]byte(mManifest)))
|
||||
}},
|
||||
{"a manifest mesh-sdk refuses fails both phases, named",
|
||||
map[string]string{"mod-l": `{"module":"mod-l","version":"1"}`, "mod-r": `{"module":"mod-r","version":"1","waits":["mesh-controller.checked"]}`},
|
||||
controllerHolds, want{reason: "manifest of mod-r could not be read", bothPhases: true}, nil},
|
||||
{"a lease already handed away leaves nothing to write",
|
||||
map[string]string{},
|
||||
handedAway, want{handedAway: true, reason: "there is nothing to write: the lease is handed to the operator's hand-over"}},
|
||||
handedAway, want{handedAway: true, reason: "there is nothing to write: the lease is handed to the operator's hand-over"}, nil},
|
||||
} {
|
||||
t.Run(row.name, func(t *testing.T) {
|
||||
b := newFakeBus()
|
||||
@@ -67,6 +83,9 @@ func TestTheLeaseCheckJudgesEveryDependencyOnTheController(t *testing.T) {
|
||||
b.manifestOf(m, raw)
|
||||
}
|
||||
b.put(grants.ReadLease, grants.LeaseKey, row.lease)
|
||||
if row.setup != nil {
|
||||
row.setup(b)
|
||||
}
|
||||
b.streams = []map[string]any{
|
||||
{"name": "KV_mesh-controller_assignments", "consumers": 3, "messages": 2991, "subjects_held": 40, "last": "2026-10-30T12:55:13Z"},
|
||||
{"name": "KV_mesh-controller_rollouts", "consumers": 0, "messages": 6652},
|
||||
@@ -102,6 +121,9 @@ func TestTheLeaseCheckJudgesEveryDependencyOnTheController(t *testing.T) {
|
||||
if row.want.reason != "" && !slices.ContainsFunc(got.Check.Reasons, func(r string) bool { return strings.Contains(r, row.want.reason) }) {
|
||||
t.Errorf("reasons %q name nothing like %q", got.Check.Reasons, row.want.reason)
|
||||
}
|
||||
if row.want.bothPhases && !slices.ContainsFunc(got.Before.Reasons, func(r string) bool { return strings.Contains(r, row.want.reason) }) {
|
||||
t.Errorf("reasons before the delivery %q name nothing like %q", got.Before.Reasons, row.want.reason)
|
||||
}
|
||||
if got.Lease.HandedAway != row.want.handedAway {
|
||||
t.Errorf("lease handed away %v (%s), want %v", got.Lease.HandedAway, got.Lease.Says, row.want.handedAway)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user