From 8f8f7dff235bc1138f6cbad50d8f111726b4f1bb Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 00:00:25 +0000 Subject: [PATCH 1/8] Keep the assignments and each module's declared config on the bus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mesh-config resolves a module's config values per node from which module runs where and what each registered module declares, and both lived only in the controller's store (hq issue 652, design 57 F3). The controller now keeps two record mirrors, mesh-controller_assignments (.) and mesh-controller_declared-config (), built from the store by the lease holder every ten seconds and written only where a value differs, by compare-and-set, with one assignment-changed or declared-config-changed per write and a purge said as removed (ADR 0289, design 54 §2-3). A module reads them through `reads` as mesh-controller.; the writers table keeps every other writer out. Neither holds a secret: an assignment names its node and module, a declaration is what the public manifest declares. Co-Authored-By: Claude Opus 5.5 --- cmd/mesh-controller/push.go | 3 + cmd/mesh-controller/record_mirrors.go | 105 +++++++++++++++++++++ internal/broker/controller_buckets.go | 60 +++++++++++- internal/broker/controller_buckets_test.go | 31 +++++- internal/broker/log_test.go | 8 +- internal/broker/states_agreement_test.go | 7 ++ internal/broker/streams.go | 5 +- internal/broker/writers.go | 9 ++ internal/broker/writers_test.go | 3 + internal/catalogue/seats.go | 4 +- internal/catalogue/state.go | 13 +++ internal/catalogue/state_test.go | 15 +++ internal/inventory/mirrors.go | 100 ++++++++++++++++++++ internal/link/events.go | 5 + testdata/store-tests | 1 + 15 files changed, 362 insertions(+), 7 deletions(-) create mode 100644 cmd/mesh-controller/record_mirrors.go create mode 100644 internal/inventory/mirrors.go diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index 05bfc4c5..ccdd28f3 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -264,6 +264,9 @@ func serve(ctx context.Context) (err error) { // The facts snapshot a merge check is fed (novox/hq to-be 45 §9): kept current while this // controller holds the lease, read by the build seat from the artifact store. go exportingFacts(ctx, open, bus.Conn.ConnectedServerVersion) + // The record mirrors (novox/hq ADR 0289, design 57 F3): the assignments and each registered module's + // declared config on the bus, written from the store only when a record changed, by the lease holder. + go mirroringRecords(ctx, inv, bus.Conn, bus) // Every call carries the lease's epoch, and its record is written only under the lease (novox/hq // to-be 45 §6). link.Calls.UnderLease(func() (uint64, error) { return theLease.epoch(ctx) }) diff --git a/cmd/mesh-controller/record_mirrors.go b/cmd/mesh-controller/record_mirrors.go new file mode 100644 index 00000000..2f25352e --- /dev/null +++ b/cmd/mesh-controller/record_mirrors.go @@ -0,0 +1,105 @@ +package main + +import ( + "context" + "fmt" + "time" + + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/recordmirror" +) + +// The controller's record mirrors on the bus (novox/hq ADR 0289, design 54, design 57 F3): the assignments, +// and each registered module's declared config, which mesh-config resolves a module's config values from. +// +// **Built from the store, on a tick of their own, by the lease holder alone.** A record changes through a +// verb that runs as a command of its own, a reconcile, a merge's registration or a pending assignment made, +// so no one call site sees every change; what every change has in common is that it is committed to the +// store. So the serving controller reads the store every mirrorEvery and writes only what differs from what +// the bucket holds (ADR 0289 §1): a quiet mesh writes nothing, and a change rolled back is never read. A +// mirror is therefore at most mirrorEvery behind its store. + +// mirrorEvery is how often the mirrors are compared with the store: two small reads of the store. +const mirrorEvery = 10 * time.Second + +// mirroringRecords keeps the mirrors current for as long as this controller serves: ctx ends when it stops. +func mirroringRecords(ctx context.Context, inv *inventory.Inventory, conn *nats.Conn, announce recordmirror.Announcer) { + api, err := jetstream.New(conn) + if err != nil { + fmt.Printf("the record mirrors are not kept: %v\n", err) + return + } + holds := func() bool { + epoch, err := theLease.epoch(ctx) + return err == nil && epoch > 0 + } + epoch := func() uint64 { + e, _ := theLease.epoch(ctx) + return e + } + sources := map[string]func(context.Context) (map[string][]byte, error){ + recordmirror.Assignments.Name: inv.AssignmentsMirror, + recordmirror.DeclaredConfig.Name: inv.DeclaredConfigMirror, + } + writers := map[string]*recordmirror.Writer{} + failing := map[string]string{} + tick := time.NewTicker(mirrorEvery) + defer tick.Stop() + for { + if holds() { + for _, m := range recordmirror.Mirrors() { + why := mirrorOnce(ctx, api, m, writers, sources[m.Name], announce, epoch, holds) + if why != failing[m.Name] { + if why != "" { + fmt.Printf("the %s mirror cannot be kept: %s\n", m.Name, why) + } else { + fmt.Printf("the %s mirror is kept again\n", m.Name) + } + failing[m.Name] = why + } + } + } + select { + case <-ctx.Done(): + return + case <-tick.C: + } + } +} + +// mirrorOnce brings one mirror to what the store holds, and answers why it could not; empty when it could. +func mirrorOnce(ctx context.Context, api jetstream.JetStream, m recordmirror.Mirror, writers map[string]*recordmirror.Writer, + source func(context.Context) (map[string][]byte, error), announce recordmirror.Announcer, epoch func() uint64, + holds func() bool) string { + reading, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + w := writers[m.Name] + if w == nil { + kv, err := api.KeyValue(reading, m.Bucket) + if err != nil { + return fmt.Sprintf("the bucket %s cannot be read: %v", m.Bucket, err) + } + w = &recordmirror.Writer{Mirror: m, KV: kv, Announce: announce, Epoch: epoch, Holds: holds} + writers[m.Name] = w + } + want, err := source(reading) + if err != nil { + return fmt.Sprintf("the store cannot be read: %v", err) + } + lost := w.LostEvents + r, err := w.Sync(reading, want) + if r.Written+r.Removed > 0 { + fmt.Printf("the %s mirror moved: %d written, %d removed\n", m.Name, r.Written, r.Removed) + } + if w.LostEvents > lost { + fmt.Printf("the %s mirror lost %d event(s): the bucket holds the change, its notice was refused twice\n", + m.Name, w.LostEvents-lost) + } + if err != nil { + return err.Error() + } + return "" +} diff --git a/internal/broker/controller_buckets.go b/internal/broker/controller_buckets.go index 69bc81ad..fe87dc4d 100644 --- a/internal/broker/controller_buckets.go +++ b/internal/broker/controller_buckets.go @@ -40,6 +40,39 @@ var ( AskedBucket = BucketName(ControllerSeat, "asked") ) +// The controller's record mirrors (novox/hq ADR 0289, design 54, design 57 package F3): a bucket per kind of +// record, one key per record, written from the store by the lease holder only when the record changed, with +// one event on the controller's seat per write. Read by whatever names them in `reads` as +// `mesh-controller.` (ADR 0201 §1), written by nobody else. **No secret in either** (ADR 0201): an +// assignment's value names its module and node; a module's declared config is what its manifest, which is +// public, declares. +// +// AssignmentsBucket holds which module is assigned to which node, keyed `.`; and +// DeclaredConfigBucket each registered module's config declarations as its manifest holds them, keyed by +// the module: what mesh-config resolves a config value from (ADR 0348 clauses 2 and 4). +var ( + AssignmentsBucket = BucketName(ControllerSeat, MirrorAssignments) + DeclaredConfigBucket = BucketName(ControllerSeat, MirrorDeclaredConfig) +) + +// The mirrors' local names, as a reader names them in `reads` after `mesh-controller.`. +const ( + MirrorAssignments = "assignments" + MirrorDeclaredConfig = "declared-config" +) + +// ControllerMirrors are the local names of the controller's record mirrors, which a module may read. +func ControllerMirrors() []string { return []string{MirrorAssignments, MirrorDeclaredConfig} } + +// The mirrors' caps (design 54 §5): one value per key, no age, 64 KiB a value, 64 MiB a bucket. +const ( + MirrorValueBytes = 64 << 10 + MirrorBytes = 64 << 20 + // MirrorMarkerTTL is the longest a purge's marker is kept (design 54 §2): the bucket allows a + // per-message age for it, and the writer gives each marker a day. + MirrorMarkerTTL = 24 * time.Hour +) + // AskedKeptFor is how long an ask is kept after it was made: a month, as the router keeps its own. const AskedKeptFor = 30 * 24 * time.Hour @@ -66,12 +99,14 @@ const ( // IsControllerBucket says a bucket is the controller's own, not a module's state nothing declares. func IsControllerBucket(bucket string) bool { return bucket == CallsBucket || bucket == HandActsBucket || bucket == ConditionsBucket || - bucket == ConditionHistoryBucket || bucket == LeaseBucket || bucket == AskedBucket + bucket == ConditionHistoryBucket || bucket == LeaseBucket || bucket == AskedBucket || + bucket == AssignmentsBucket || bucket == DeclaredConfigBucket } // ControllerBuckets are the controller's own buckets, in the order they are asserted. func ControllerBuckets() []string { - return []string{LeaseBucket, CallsBucket, HandActsBucket, ConditionsBucket, ConditionHistoryBucket, AskedBucket} + return []string{LeaseBucket, CallsBucket, HandActsBucket, ConditionsBucket, ConditionHistoryBucket, AskedBucket, + AssignmentsBucket, DeclaredConfigBucket} } // ControllerBucketsAsserter is what raising the controller's buckets needs of a connection. @@ -168,6 +203,27 @@ func (j *JetStream) EnsureControllerBuckets() error { }); err != nil { return fmt.Errorf("asserting bucket %s: %w", AskedBucket, err) } + // **The record mirrors: no age** (design 54 §5). A key is removed when its record is, by a purge whose + // marker lives a day; a key that expired would be a record the bus forgot while the store still holds it. + for _, m := range []struct{ bucket, why string }{ + {AssignmentsBucket, "which module is assigned to which node, keyed . (novox/hq ADR 0289, " + + "design 54): written by the controller alone from its store when an assignment changes, read through `reads`"}, + {DeclaredConfigBucket, "each registered module's config declarations as its manifest holds them, keyed by " + + "the module (novox/hq ADR 0348, design 57 F3): written by the controller alone from its store when a " + + "registration changes them, read through `reads`"}, + } { + if _, err := js.CreateOrUpdateKeyValue(ctx, jetstream.KeyValueConfig{ + Bucket: m.bucket, + Description: m.why, + History: 1, + MaxValueSize: MirrorValueBytes, + MaxBytes: MirrorBytes, + Storage: jetstream.FileStorage, + LimitMarkerTTL: MirrorMarkerTTL, + }); err != nil { + return fmt.Errorf("asserting bucket %s: %w", m.bucket, err) + } + } return nil } diff --git a/internal/broker/controller_buckets_test.go b/internal/broker/controller_buckets_test.go index 804f1c5e..1e8c7a69 100644 --- a/internal/broker/controller_buckets_test.go +++ b/internal/broker/controller_buckets_test.go @@ -21,7 +21,8 @@ func TestTheControllerMayWriteEveryBucketItWrites(t *testing.T) { if err != nil { t.Fatal(err) } - want := []string{"$KV." + CallsBucket + ".>", "$KV." + HandActsBucket + ".>"} + want := []string{"$KV." + CallsBucket + ".>", "$KV." + HandActsBucket + ".>", + "$KV." + AssignmentsBucket + ".>", "$KV." + DeclaredConfigBucket + ".>"} for _, seat := range seatsTheControllerAsks { if hasCancelledSet(seat) { want = append(want, "$KV."+CancelledSetName(seat)+".>") @@ -37,6 +38,34 @@ func TestTheControllerMayWriteEveryBucketItWrites(t *testing.T) { } } +// **A module reads a record mirror through `reads`, and never writes it** (ADR 0201 §1, design 54 §5, +// design 57 F3): granted the direct reads and watches of the bucket, and refused at composition a grant to +// write it, whose writer is the controller. +func TestAModuleReadsTheControllersMirrorsAndWritesNone(t *testing.T) { + reader := Principal{Kind: KindModule, Node: "one", Module: "mesh-config", PasswordHash: "x", + Reads: []string{"mesh-controller." + MirrorAssignments, "mesh-controller." + MirrorDeclaredConfig}} + p, err := PermissionsFor(reader) + if err != nil { + t.Fatal(err) + } + for _, bucket := range []string{AssignmentsBucket, DeclaredConfigBucket} { + for _, subject := range []string{"$JS.API.STREAM.INFO.KV_" + bucket, "$JS.API.DIRECT.GET.KV_" + bucket + ".>", + "$JS.API.CONSUMER.CREATE.KV_" + bucket + ".>"} { + if !slices.Contains(p.Publish, subject) { + t.Errorf("a module reading %s may not publish %s", bucket, subject) + } + } + for _, subject := range p.Publish { + if SubjectsOverlap(subject, "$KV."+bucket+".>") { + t.Errorf("a module reading %s may write it through %s", bucket, subject) + } + } + if err := CheckWriters(reader, []string{"$KV." + bucket + ".>"}); err == nil { + t.Errorf("a module granted a write of %s would compose", bucket) + } + } +} + // **A machine's node tools may say they are there, as that machine and no other** (novox/hq to-be 45 // S11), and the controller may hear the bus's advisories and ask who answers — read-only, named. func TestTheWatchedSignalsMayBeSaidAndHeard(t *testing.T) { diff --git a/internal/broker/log_test.go b/internal/broker/log_test.go index 3119fb03..95043e1c 100644 --- a/internal/broker/log_test.go +++ b/internal/broker/log_test.go @@ -172,7 +172,9 @@ func TestRaisingLogsReportsWhatNothingDeclares(t *testing.T) { // **No path of the mesh deletes or purges a log or a bucket, or recreates one** (novox/hq ADR 0297 §2, // ADR 0201 §15–16): no code of this repository outside its tests calls the client's stream or bucket // delete, or purges a stream, but for the controller lease's own stream, which purges its own history -// below a sequence (internal/lease). A new call is a decision, not a refactor. +// below a sequence (internal/lease), and the controller's record mirrors, which purge the key of a record +// the store no longer holds (internal/recordmirror; novox/hq ADR 0289, design 54 §2). A new call is a decision, +// not a refactor. func TestNoPathDeletesALogOrABucket(t *testing.T) { removers := map[string]bool{"DeleteStream": true, "DeleteKeyValue": true, "PurgeStream": true, "DeleteObjectStore": true} @@ -197,7 +199,9 @@ func TestNoPathDeletesALogOrABucket(t *testing.T) { return true } name := sel.Sel.Name - if removers[name] || (name == "Purge" && !strings.Contains(filepath.ToSlash(path), "internal/lease/")) { + decided := strings.Contains(filepath.ToSlash(path), "internal/lease/") || + strings.Contains(filepath.ToSlash(path), "internal/recordmirror/") + if removers[name] || (name == "Purge" && !decided) { found = append(found, path+": "+name) } return true diff --git a/internal/broker/states_agreement_test.go b/internal/broker/states_agreement_test.go index 179203ef..e29a8992 100644 --- a/internal/broker/states_agreement_test.go +++ b/internal/broker/states_agreement_test.go @@ -34,6 +34,8 @@ func TestTheFactsTheGrantPermitsAreTheFactsTheMeshStates(t *testing.T) { states = append(states, link.KeyChecked) // And a walk kept in a new state, for the delivery it walks (novox/hq ADR 0239). states = append(states, link.KeyPlanMoved) + // And a key of a record mirror written or removed (novox/hq ADR 0289, design 54 §3; design 57 F3). + states = append(states, link.KeyAssignmentChanged, link.KeyDeclaredConfigChanged) for _, event := range states { if !slices.Contains(broker.ControllerStates, event) { t.Errorf("the mesh states %q and its account may not publish it", event) @@ -42,6 +44,11 @@ func TestTheFactsTheGrantPermitsAreTheFactsTheMeshStates(t *testing.T) { if len(broker.ControllerStates) != len(states) { t.Errorf("the grant permits %v, which is more than the mesh states", broker.ControllerStates) } + // The record mirrors a module may read are the ones the bus holds (design 54 §5, design 57 F3). + if !slices.Equal(catalogue.ControllerMirrors, broker.ControllerMirrors()) { + t.Errorf("the catalogue lets a module read the controller's mirrors %v and the bus holds %v", + catalogue.ControllerMirrors, broker.ControllerMirrors()) + } // **And the seat says it.** A seat carries the protocol of its role (novox/hq ADR 0129), so the // facts the control plane states are the seat's `emits` — which is what lets anything else declare // that it consumes them, and what the subject-agreement check reads to know they have an owner. diff --git a/internal/broker/streams.go b/internal/broker/streams.go index 124dfa86..3c286a19 100644 --- a/internal/broker/streams.go +++ b/internal/broker/streams.go @@ -313,7 +313,10 @@ var ControllerStates = []string{"applied", "refused", "built-before", // the pull request's status, and anybody else may read. "checked", // And a walk kept in a new state (novox/hq ADR 0239): what the delivery it walks reads its steps from. - "plan-moved"} + "plan-moved", + // And a key of a record mirror written or removed (novox/hq ADR 0289, design 54 §3): the assignments and + // each registered module's declared config (design 57 F3). + "assignment-changed", "declared-config-changed"} // BusAdvisories are what the bus server says about the mesh's own account that the controller // reads (novox/hq to-be 45 §3, S9): a durable consumer that handed a message over as often as it diff --git a/internal/broker/writers.go b/internal/broker/writers.go index 6099dfb0..b5e83824 100644 --- a/internal/broker/writers.go +++ b/internal/broker/writers.go @@ -158,6 +158,15 @@ var WritersTable = []WriterRow{ {State: "a message a consumer gave up on", Writer: "controller", KeptIn: "the bus, the stream " + DeadLettersStream, Others: "read, delivered again or dropped through the controller's dead-letters verb", Subjects: []string{deadLetterPrefix + ">", againPrefix + ">"}, Writes: isController}, + // The record mirrors (novox/hq ADR 0289 §9, design 54 §2): rows of this table, so a grant that lets + // anything else write them is refused at composition. Built so far: the assignments and each registered + // module's declared config (design 57 F3). + {State: "the assignments mirror", Writer: "controller (lease holder), from its store, only on a change", + KeptIn: "key-value " + AssignmentsBucket, Others: "read through `reads` as mesh-controller." + MirrorAssignments, + Subjects: kvOf(AssignmentsBucket), Writes: isController}, + {State: "the declared-config mirror", Writer: "controller (lease holder), from its store, only on a change", + KeptIn: "key-value " + DeclaredConfigBucket, Others: "read through `reads` as mesh-controller." + MirrorDeclaredConfig, + Subjects: kvOf(DeclaredConfigBucket), Writes: isController}, } // CheckWriters refuses a grant that lets a principal publish on a subject the writers table gives diff --git a/internal/broker/writers_test.go b/internal/broker/writers_test.go index 124de422..7ec91c5f 100644 --- a/internal/broker/writers_test.go +++ b/internal/broker/writers_test.go @@ -34,6 +34,9 @@ var designRows = []string{ "the operator-channel's open messages", "the facts snapshot", "a message a consumer gave up on", + // The record mirrors, rows of to-be 45 §1's table by design 54 §2 (ADR 0289 §9). + "the assignments mirror", + "the declared-config mirror", } func TestTheWritersTableIsTheDesigns(t *testing.T) { diff --git a/internal/catalogue/seats.go b/internal/catalogue/seats.go index 68c7291a..8602d507 100644 --- a/internal/catalogue/seats.go +++ b/internal/catalogue/seats.go @@ -107,7 +107,9 @@ var defaultSeats = append([]Seat{ // A pull request's merge check, judged (novox/hq to-be 45 §9). "checked", // A walk kept in a new state, for the delivery it walks (novox/hq ADR 0239). - "plan-moved"}, + "plan-moved", + // A key of a record mirror written or removed (novox/hq ADR 0289, design 54 §3; design 57 F3). + "assignment-changed", "declared-config-changed"}, Serves: ControllerVerbs}, // The store's first verbs (novox/hq ADR 0159): the smallest set that makes the store askable, // served by whichever module holds the seat with tools of these names. diff --git a/internal/catalogue/state.go b/internal/catalogue/state.go index 1944b778..42a14266 100644 --- a/internal/catalogue/state.go +++ b/internal/catalogue/state.go @@ -124,6 +124,14 @@ func StateProblems(m Manifest) []string { return problems } +// ControllerMirrors are the controller's record mirrors a module may read, each `mesh-controller.` +// (novox/hq ADR 0289, design 54 §5, design 57 F3). **The controller keeps them, not its manifest's `state`**: +// they are asserted with the controller's own buckets, with options of their own, and written by the +// controller's account alone (the writers table); declared as a module's state, they would be asserted a +// second way and granted to the module's own principal as well. Named here so a read of one is a read of +// kept state. The bus's list is broker.ControllerMirrors; a test keeps the two agreeing. +var ControllerMirrors = []string{"assignments", "declared-config"} + // StateReadsNothingDeclares is every read across a catalogue whose owner is present and declares no // such state. An absent owner says nothing — a module may be installed long before the one whose // state it reads, as a consumer may before its emitter (design 32 §1). @@ -136,6 +144,11 @@ func StateReadsNothingDeclares(manifests []Manifest) []string { } declared[m.Module] = own } + if own, present := declared[ControllerSeatName]; present { + for _, name := range ControllerMirrors { + own[name] = true + } + } var problems []string for _, m := range manifests { for _, r := range m.Reads { diff --git a/internal/catalogue/state_test.go b/internal/catalogue/state_test.go index 98eba5f9..e82f9711 100644 --- a/internal/catalogue/state_test.go +++ b/internal/catalogue/state_test.go @@ -67,6 +67,21 @@ func TestAReadNamesStateItsOwnerKeeps(t *testing.T) { } } +// **The controller's record mirrors are kept state** (design 54 §5, design 57 F3): a module may read +// `mesh-controller.assignments` and `mesh-controller.declared-config` although the controller's manifest +// declares no `state`; a read of anything else of the controller's is still named. +func TestAReadOfTheControllersMirrorsIsAReadOfKeptState(t *testing.T) { + controller := Manifest{Module: ControllerSeatName} + reader := Manifest{Module: "mesh-config", Reads: []string{"mesh-controller.assignments", "mesh-controller.declared-config"}} + if p := StateReadsNothingDeclares([]Manifest{controller, reader}); len(p) != 0 { + t.Fatalf("a read of the controller's mirrors was refused: %v", p) + } + other := Manifest{Module: "other", Reads: []string{"mesh-controller.calls"}} + if p := StateReadsNothingDeclares([]Manifest{controller, other}); len(p) != 1 { + t.Fatalf("a read of the controller's own calls was not named: %v", p) + } +} + // **Across the whole catalogue**: every state name is local, and every read whose owner is present // names state that owner keeps. func TestEveryManifestsStateIsLocalAndEveryReadIsKept(t *testing.T) { diff --git a/internal/inventory/mirrors.go b/internal/inventory/mirrors.go new file mode 100644 index 00000000..5d878a0f --- /dev/null +++ b/internal/inventory/mirrors.go @@ -0,0 +1,100 @@ +package inventory + +import ( + "bytes" + "context" + "encoding/json" + "time" +) + +// The values of the controller's record mirrors, built from the store (novox/hq ADR 0289, design 54 §1, +// design 57 package F3). +// +// **Each value is built from a named list of fields, and nothing else** (design 54 §1): no epoch, no time +// of writing and no time of the last look, so any lease holder builds the same bytes from the same record +// and a pass over unchanged records writes nothing (ADR 0289 §1, §4). The encoding is fixed: the fields in +// their struct's order, a declaration's maps sorted. + +// AssignmentMirrored is one assignment as the assignments mirror holds it: the module, the node, and since +// when it is assigned there. Design 54 §1 also lists whether it is pending, the builds sent and running and +// the seats and provisions it holds there; those join with issue 419, which builds the rest of design 54. +type AssignmentMirrored struct { + Node string `json:"node"` + Module string `json:"module"` + Assigned time.Time `json:"assigned"` +} + +// DeclaredConfigMirrored is one registered module's config declarations as the declared-config mirror holds +// them: the module, and its manifest's `settings` exactly as the manifest holds them (each key's kind, +// default and why today), carried whole so a declaration that gains fields (ADR 0348 clause 4) is mirrored +// without a change here. An empty object when the module declares none: registered, declaring nothing. +type DeclaredConfigMirrored struct { + Module string `json:"module"` + Settings json.RawMessage `json:"settings"` +} + +// AssignmentsMirror is every assignment's mirrored value, keyed `.`. +func (i *Inventory) AssignmentsMirror(ctx context.Context) (map[string][]byte, error) { + rows, err := i.store.Pool().Query(ctx, + `select n.name, a.module, a.assigned from assignment a join node n on n.id = a.node`) + if err != nil { + return nil, err + } + defer rows.Close() + out := map[string][]byte{} + for rows.Next() { + var a AssignmentMirrored + if err := rows.Scan(&a.Node, &a.Module, &a.Assigned); err != nil { + return nil, err + } + a.Assigned = a.Assigned.UTC() + value, err := json.Marshal(a) + if err != nil { + return nil, err + } + out[a.Node+"."+a.Module] = value + } + return out, rows.Err() +} + +// DeclaredConfigMirror is every registered module's mirrored config declarations, keyed by the module. +func (i *Inventory) DeclaredConfigMirror(ctx context.Context) (map[string][]byte, error) { + rows, err := i.store.Pool().Query(ctx, + `select name, coalesce(manifest->'settings', '{}'::jsonb) from module`) + if err != nil { + return nil, err + } + defer rows.Close() + out := map[string][]byte{} + for rows.Next() { + var ( + d DeclaredConfigMirrored + raw []byte + ) + if err := rows.Scan(&d.Module, &raw); err != nil { + return nil, err + } + settings, err := canonical(raw) + if err != nil { + return nil, err + } + d.Settings = settings + value, err := json.Marshal(d) + if err != nil { + return nil, err + } + out[d.Module] = value + } + return out, rows.Err() +} + +// canonical is a JSON document encoded the one way: objects' keys sorted, numbers as written. +func canonical(raw []byte) ([]byte, error) { + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + var v any + if err := dec.Decode(&v); err != nil { + return nil, err + } + return json.Marshal(v) +} diff --git a/internal/link/events.go b/internal/link/events.go index 48b7ece6..9d89dcfc 100644 --- a/internal/link/events.go +++ b/internal/link/events.go @@ -81,6 +81,11 @@ const ( // steps from. Said after every save of the serving controller; a delivery's holder also asks for the // walks it follows, so a save made elsewhere is found by comparison, never lost. KeyPlanMoved = "plan-moved" + // KeyAssignmentChanged and KeyDeclaredConfigChanged: a key of the controller's assignments or declared-config + // mirror was written or removed (novox/hq ADR 0289, design 54 §3, design 57 F3): which key, how and at which + // revision of the bucket — never the value, which the bucket holds. + KeyAssignmentChanged = "assignment-changed" + KeyDeclaredConfigChanged = "declared-config-changed" ) // Applied is what a machine now runs, as the mesh states it. diff --git a/testdata/store-tests b/testdata/store-tests index bbfc1467..b7dc4922 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -61,3 +61,4 @@ ./cmd/mesh-controller TestAProviderElsewhereServesItsOwnComposedName ./cmd/mesh-controller TestAReusedCheckBuildIsRegisteredByItsDigestAndGatedAsABuiltOne ./internal/inventory TestAReportOfABusyContainerIsKeptAndReadBack +./internal/recordmirror TestNatsAnAssignmentAndARegistrationEachChangeOneKeyAndRaiseOneEvent -- 2.54.0 From 176a3d4fdf3b0bc4057289c71fcf2f1fc3f00276 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 00:31:17 +0000 Subject: [PATCH 2/8] Write a record mirror only where its store differs, one event per write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The writer behind the assignments and declared-config mirrors (hq issue 652, ADR 0289, design 54 §2-3): filled from the bucket on its first pass, it compares each value byte for byte, writes by compare-and-set, purges a key whose record is gone with a marker that lives a day, and says each write once after it succeeded. Co-Authored-By: Claude Opus 5.5 --- internal/recordmirror/mirror.go | 330 ++++++++++++++++++++++++++++++++ 1 file changed, 330 insertions(+) create mode 100644 internal/recordmirror/mirror.go diff --git a/internal/recordmirror/mirror.go b/internal/recordmirror/mirror.go new file mode 100644 index 00000000..ed9b7a48 --- /dev/null +++ b/internal/recordmirror/mirror.go @@ -0,0 +1,330 @@ +// Package recordmirror writes the controller's record mirrors (novox/hq ADR 0289, design 54): a bucket per kind +// of record, one key per record, written from the store only when the record changed, and one event on the +// controller's seat per write. +// +// **The store stays the record.** A mirror is built from what the store committed, so a change rolled back +// is never written. The writer keeps the last value it wrote for each key, filled from the bucket on its +// first pass, and compares byte for byte: a pass over unchanged records writes nothing and says nothing +// (ADR 0289 §1). Each write names the revision it last saw (compare-and-set), so a former lease holder that +// wrote in between is noticed, never overwritten blind. +// +// **The bucket is the authority, the event a notice** (design 54 §3): the event is sent after the write +// succeeded, so there is no event for a value the bucket does not hold. A refused event is tried once more +// and then counted as lost; a watcher that must not miss a change watches the bucket. +// +// What this package does not do yet is design 54 §4's audit (probe D17 and its healer H6), which compares a +// mirror with its store behind the writer's back; novox/hq issue 419 carries it. +package recordmirror + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "regexp" + "sort" + "time" + + "github.com/nats-io/nats.go/jetstream" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/link" +) + +// A Mirror is one kind of record the controller keeps on the bus: its local name, its bucket and the event +// each write raises on the controller's seat. +type Mirror struct { + Name string + Bucket string + Event string +} + +// The mirrors this package writes (novox/hq design 57, package F3). The assignments mirror is design 54's, +// keyed `.`; the declared config is each registered module's config declarations as its +// manifest holds them, keyed by the module, which mesh-config resolves a module's config values from +// (ADR 0348 clauses 2 and 4). +var ( + Assignments = Mirror{Name: broker.MirrorAssignments, Bucket: broker.AssignmentsBucket, + Event: link.KeyAssignmentChanged} + DeclaredConfig = Mirror{Name: broker.MirrorDeclaredConfig, Bucket: broker.DeclaredConfigBucket, + Event: link.KeyDeclaredConfigChanged} +) + +// Mirrors is every mirror this package writes, in the order they are written. +func Mirrors() []Mirror { return []Mirror{Assignments, DeclaredConfig} } + +// The changes an event says. +const ( + Added = "added" + Changed = "changed" + Removed = "removed" +) + +// Event is what one write says on the controller's seat (design 54 §3): which mirror and key, what kind of +// change, the names of the top-level fields that changed, the bucket's revision of the write, the lease's +// epoch and when. Never the value: a watcher reads it from the bucket at that revision or later. +type Event struct { + Mirror string `json:"mirror"` + Key string `json:"key"` + Change string `json:"change"` + Fields []string `json:"fields,omitempty"` + Revision uint64 `json:"revision"` + Epoch uint64 `json:"epoch,omitempty"` + At time.Time `json:"at"` +} + +// Announcer says an event on a seat; link.OverNATS is one, and so is link.Bus. +type Announcer interface { + PublishSeatEvent(ctx context.Context, seat, event string, body []byte) error +} + +// MarkerTTL is how long the marker a purge leaves is kept (design 54 §2): a day, so the markers of removed +// records do not pile up in a bucket that has no age. +const MarkerTTL = 24 * time.Hour + +// Writer writes one mirror. Not safe for use by two goroutines at once: one controller's one loop owns it. +type Writer struct { + Mirror Mirror + KV jetstream.KeyValue + Announce Announcer + // Epoch is the lease's epoch the events carry; nil carries none. + Epoch func() uint64 + // Holds says this controller still holds the lease; nil is always. A write refused by compare-and-set + // is made again only while it does (design 54 §2). + Holds func() bool + // Now is the clock the events are dated by; nil is time.Now. + Now func() time.Time + + known map[string]kept + filled bool + // LostEvents counts the events refused twice since the writer was made. + LostEvents int +} + +type kept struct { + value []byte + revision uint64 +} + +// Result is what one pass did: keys written, keys removed, and keys refused with why. +type Result struct { + Written int + Removed int + Refused int +} + +// Sync makes the bucket hold want, one key per record: writes each value that differs from what the key +// holds, purges each key no record is left for, and raises one event per write. A key the bus cannot hold +// is refused alone, and the others are written; the error then names it. +func (w *Writer) Sync(ctx context.Context, want map[string][]byte) (Result, error) { + var r Result + if !w.filled { + if err := w.fill(ctx); err != nil { + return r, fmt.Errorf("reading the %s mirror: %w", w.Mirror.Name, err) + } + } + var problems []error + keys := make([]string, 0, len(want)) + for k := range want { + keys = append(keys, k) + } + sort.Strings(keys) + for _, key := range keys { + value := want[key] + before, had := w.known[key] + if had && bytes.Equal(before.value, value) { + continue + } + if err := validKey(key); err != nil { + r.Refused++ + problems = append(problems, err) + continue + } + revision, wrote, err := w.put(ctx, key, value, before, had) + if err != nil { + problems = append(problems, fmt.Errorf("writing %s: %w", key, err)) + continue + } + w.known[key] = kept{value: append([]byte(nil), value...), revision: revision} + if !wrote { + continue + } + r.Written++ + change, fields := Added, []string(nil) + if had { + change, fields = Changed, changedFields(before.value, value) + } + w.say(ctx, key, change, fields, revision) + } + gone := make([]string, 0) + for key := range w.known { + if _, ok := want[key]; !ok { + gone = append(gone, key) + } + } + sort.Strings(gone) + for _, key := range gone { + k := w.known[key] + err := w.KV.Purge(ctx, key, jetstream.LastRevision(k.revision), jetstream.PurgeTTL(MarkerTTL)) + if refusedByRevision(err) { + // Read it again: purged already, or moved by another writer. + if e, gerr := w.KV.Get(ctx, key); errors.Is(gerr, jetstream.ErrKeyNotFound) { + delete(w.known, key) + continue + } else if gerr == nil && w.holds() { + err = w.KV.Purge(ctx, key, jetstream.LastRevision(e.Revision()), jetstream.PurgeTTL(MarkerTTL)) + } + } + if err != nil { + problems = append(problems, fmt.Errorf("removing %s: %w", key, err)) + continue + } + delete(w.known, key) + r.Removed++ + revision := uint64(0) + if status, err := w.KV.Status(ctx); err == nil { + // The purge's own sequence is the stream's last one only when nothing wrote in between, which no + // other writer may; said as the bucket's revision after it. + if s, ok := status.(interface{ StreamInfo() *jetstream.StreamInfo }); ok { + revision = s.StreamInfo().State.LastSeq + } + } + w.say(ctx, key, Removed, nil, revision) + } + return r, errors.Join(problems...) +} + +// put writes one value by compare-and-set and answers its revision, and whether this call wrote it (false +// when the bucket already held it). +func (w *Writer) put(ctx context.Context, key string, value []byte, before kept, had bool) (uint64, bool, error) { + var ( + revision uint64 + err error + ) + if had { + revision, err = w.KV.Update(ctx, key, value, before.revision) + } else { + revision, err = w.KV.Create(ctx, key, value) + } + if !refusedByRevision(err) { + return revision, err == nil, err + } + // Refused: another writer in between, or this writer's own write whose acknowledgement was lost. + e, gerr := w.KV.Get(ctx, key) + switch { + case errors.Is(gerr, jetstream.ErrKeyNotFound): + if !w.holds() { + return 0, false, errors.New("the key moved and this controller no longer holds the lease") + } + revision, err = w.KV.Create(ctx, key, value) + return revision, err == nil, err + case gerr != nil: + return 0, false, gerr + case bytes.Equal(e.Value(), value): + return e.Revision(), false, nil + case !w.holds(): + return 0, false, errors.New("the key moved and this controller no longer holds the lease") + } + revision, err = w.KV.Update(ctx, key, value, e.Revision()) + return revision, err == nil, err +} + +// say raises one write's event, tried once more when refused and then counted as lost. +func (w *Writer) say(ctx context.Context, key, change string, fields []string, revision uint64) { + if w.Announce == nil { + return + } + e := Event{Mirror: w.Mirror.Name, Key: key, Change: change, Fields: fields, Revision: revision, At: w.now().UTC()} + if w.Epoch != nil { + e.Epoch = w.Epoch() + } + body, err := json.Marshal(e) + if err != nil { + w.LostEvents++ + return + } + for try := 0; try < 2; try++ { + if err = w.Announce.PublishSeatEvent(ctx, broker.ControllerSeat, w.Mirror.Event, body); err == nil { + return + } + } + w.LostEvents++ +} + +// fill reads what the bucket holds now, so a controller started again writes only what differs. +func (w *Writer) fill(ctx context.Context) error { + w.known = map[string]kept{} + watch, err := w.KV.WatchAll(ctx, jetstream.IgnoreDeletes()) + if err != nil { + return err + } + defer func() { _ = watch.Stop() }() + for { + select { + case <-ctx.Done(): + return ctx.Err() + case e := <-watch.Updates(): + if e == nil { + w.filled = true + return nil + } + w.known[e.Key()] = kept{value: append([]byte(nil), e.Value()...), revision: e.Revision()} + } + } +} + +func (w *Writer) holds() bool { return w.Holds == nil || w.Holds() } + +func (w *Writer) now() time.Time { + if w.Now == nil { + return time.Now() + } + return w.Now() +} + +// refusedByRevision is the server refusing a compare-and-set: the key is not at the revision named. +func refusedByRevision(err error) bool { + if err == nil { + return false + } + if errors.Is(err, jetstream.ErrKeyExists) { + return true + } + var apiErr *jetstream.APIError + return errors.As(err, &apiErr) && apiErr.ErrorCode == jetstream.JSErrCodeStreamWrongLastSequence +} + +// keyChars is what a bucket's key may be made of. +var keyChars = regexp.MustCompile(`^[-/_=.a-zA-Z0-9]+$`) + +// validKey refuses a key the bus cannot hold, naming it. +func validKey(key string) error { + if key == "" || !keyChars.MatchString(key) || key[0] == '.' || key[len(key)-1] == '.' { + return fmt.Errorf("%q cannot be a key of a bucket: letters, digits, '-', '_', '=', '/' and '.', not at "+ + "either end", key) + } + return nil +} + +// changedFields names the top-level fields whose values differ between two encoded objects, sorted; nil +// when either is not an object. +func changedFields(before, after []byte) []string { + var a, b map[string]json.RawMessage + if json.Unmarshal(before, &a) != nil || json.Unmarshal(after, &b) != nil || a == nil || b == nil { + return nil + } + var out []string + for k, v := range b { + if old, ok := a[k]; !ok || !bytes.Equal(old, v) { + out = append(out, k) + } + } + for k := range a { + if _, ok := b[k]; !ok { + out = append(out, k) + } + } + sort.Strings(out) + return out +} -- 2.54.0 From ea5cc6fc3536d10e1730900d87f04070f05a72b8 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 00:32:29 +0000 Subject: [PATCH 3/8] Prove a record mirror against a real bus and the store An assignment and a registration each change exactly one key and raise exactly one event; the same records written again change nothing (hq issue 652, the package's done-when). Listed as a store test, so the merge check runs it. Co-Authored-By: Claude Opus 5.5 --- internal/recordmirror/mirror_store_test.go | 158 +++++++++++++ internal/recordmirror/mirror_test.go | 261 +++++++++++++++++++++ 2 files changed, 419 insertions(+) create mode 100644 internal/recordmirror/mirror_store_test.go create mode 100644 internal/recordmirror/mirror_test.go diff --git a/internal/recordmirror/mirror_store_test.go b/internal/recordmirror/mirror_store_test.go new file mode 100644 index 00000000..c0601a7c --- /dev/null +++ b/internal/recordmirror/mirror_store_test.go @@ -0,0 +1,158 @@ +package recordmirror + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" +) + +// **Design 57's F3, end to end** (novox/hq issue 652): from the controller's store to a real bus. An +// assignment changes exactly one key of the assignments mirror and raises exactly one `assignment-changed`; +// a registration changes exactly one key of the declared-config mirror and raises exactly one +// `declared-config-changed`; and the same records written again change nothing and say nothing — a module +// registered again unchanged included, although the store's registration time moved. +func TestNatsAnAssignmentAndARegistrationEachChangeOneKeyAndRaiseOneEvent(t *testing.T) { + inv := inventory.ForTest(t) + ctx := t.Context() + js, api := aMirrorBus(t) + assignments := aWriter(t, js, api, Assignments) + declared := aWriter(t, js, api, DeclaredConfig) + heardAssignments := listen(t, js.Conn(), "mesh.seat."+broker.ControllerSeat+".event."+Assignments.Event) + heardDeclared := listen(t, js.Conn(), "mesh.seat."+broker.ControllerSeat+".event."+DeclaredConfig.Event) + + pass := func() (Result, Result) { + t.Helper() + a, err := inv.AssignmentsMirror(ctx) + if err != nil { + t.Fatal(err) + } + ra, err := assignments.Sync(ctx, a) + if err != nil { + t.Fatal(err) + } + d, err := inv.DeclaredConfigMirror(ctx) + if err != nil { + t.Fatal(err) + } + rd, err := declared.Sync(ctx, d) + if err != nil { + t.Fatal(err) + } + return ra, rd + } + + if _, err := inv.AddNode(ctx, "anchor"); err != nil { + t.Fatal(err) + } + ui := catalogue.Manifest{Module: "ui", Version: "1", Settings: map[string]catalogue.SettingDeclaration{ + "font-size": {Kind: catalogue.KindPreference, Default: 14, Why: "readable on the desktop"}}} + if err := inv.RegisterModule(ctx, ui, inventory.Source{}); err != nil { + t.Fatal(err) + } + pass() + assignmentWrites, declaredWrites := writes(t, api, Assignments.Bucket), writes(t, api, DeclaredConfig.Bucket) + assignmentsSaid, declaredSaid := len(settle(heardAssignments, 0)), len(settle(heardDeclared, 1)) + + // **A registration**: a module new to the catalogue. + if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "notes", Version: "1"}, inventory.Source{}); err != nil { + t.Fatal(err) + } + ra, rd := pass() + if ra != (Result{}) || rd != (Result{Written: 1}) { + t.Errorf("a registration wrote assignments %+v and declared config %+v", ra, rd) + } + if got := writes(t, api, DeclaredConfig.Bucket) - declaredWrites; got != 1 { + t.Errorf("a registration changed %d keys of the declared config", got) + } + if got := writes(t, api, Assignments.Bucket) - assignmentWrites; got != 0 { + t.Errorf("a registration changed %d keys of the assignments", got) + } + said := settle(heardDeclared, declaredSaid+1)[declaredSaid:] + if len(said) != 1 || said[0].Key != "notes" || said[0].Change != Added { + t.Errorf("a registration raised %+v", said) + } + declaredWrites, declaredSaid = writes(t, api, DeclaredConfig.Bucket), declaredSaid+len(said) + + // **An assignment.** + if _, err := inv.Assign(ctx, "anchor", "ui"); err != nil { + t.Fatal(err) + } + ra, rd = pass() + if ra != (Result{Written: 1}) || rd != (Result{}) { + t.Errorf("an assignment wrote assignments %+v and declared config %+v", ra, rd) + } + if got := writes(t, api, Assignments.Bucket) - assignmentWrites; got != 1 { + t.Errorf("an assignment changed %d keys of the assignments", got) + } + said = settle(heardAssignments, assignmentsSaid+1)[assignmentsSaid:] + if len(said) != 1 || said[0].Key != "anchor.ui" || said[0].Change != Added { + t.Errorf("an assignment raised %+v", said) + } + assignmentWrites, assignmentsSaid = writes(t, api, Assignments.Bucket), assignmentsSaid+len(said) + e, err := assignments.KV.Get(ctx, "anchor.ui") + if err != nil { + t.Fatal(err) + } + var value inventory.AssignmentMirrored + if err := json.Unmarshal(e.Value(), &value); err != nil || value.Node != "anchor" || value.Module != "ui" || + value.Assigned.IsZero() { + t.Errorf("the assignment is mirrored as %s (%v)", e.Value(), err) + } + + // **The same records written again change nothing**: an assignment made again, and a module registered + // again with the manifest it had. + if _, err := inv.Assign(ctx, "anchor", "ui"); err != nil { + t.Fatal(err) + } + if err := inv.RegisterModule(ctx, ui, inventory.Source{}); err != nil { + t.Fatal(err) + } + if ra, rd := pass(); ra != (Result{}) || rd != (Result{}) { + t.Errorf("unchanged records wrote assignments %+v and declared config %+v", ra, rd) + } + if writes(t, api, Assignments.Bucket) != assignmentWrites || writes(t, api, DeclaredConfig.Bucket) != declaredWrites { + t.Error("unchanged records changed a key") + } + if a, d := settle(heardAssignments, assignmentsSaid+1), settle(heardDeclared, declaredSaid+1); len(a) != assignmentsSaid || + len(d) != declaredSaid { + t.Errorf("unchanged records raised %+v and %+v", a[assignmentsSaid:], d[declaredSaid:]) + } + + // **A declaration changed by a registration**: one key, one event, naming the field that moved. + ui.Version = "2" + ui.Settings["font-size"] = catalogue.SettingDeclaration{Kind: catalogue.KindPreference, Default: 16, + Why: "readable on the desktop"} + if err := inv.RegisterModule(ctx, ui, inventory.Source{}); err != nil { + t.Fatal(err) + } + if ra, rd := pass(); ra != (Result{}) || rd != (Result{Written: 1}) { + t.Errorf("a changed declaration wrote assignments %+v and declared config %+v", ra, rd) + } + said = settle(heardDeclared, declaredSaid+1)[declaredSaid:] + if len(said) != 1 || said[0].Key != "ui" || said[0].Change != Changed || strings.Join(said[0].Fields, ",") != "settings" { + t.Errorf("a changed declaration raised %+v", said) + } + got, err := declared.KV.Get(ctx, "ui") + if err != nil { + t.Fatal(err) + } + if want := `{"module":"ui","settings":{"font-size":{"default":16,"kind":"preference","why":"readable on the desktop"}}}`; string(got.Value()) != want { + t.Errorf("the declaration is mirrored as %s, want %s", got.Value(), want) + } + + // **An unassignment** removes its key, said once. + if err := inv.Unassign(ctx, "anchor", "ui"); err != nil { + t.Fatal(err) + } + if ra, _ := pass(); ra != (Result{Removed: 1}) { + t.Errorf("an unassignment did %+v", ra) + } + said = settle(heardAssignments, assignmentsSaid+1)[assignmentsSaid:] + if len(said) != 1 || said[0].Key != "anchor.ui" || said[0].Change != Removed { + t.Errorf("an unassignment raised %+v", said) + } +} diff --git a/internal/recordmirror/mirror_test.go b/internal/recordmirror/mirror_test.go new file mode 100644 index 00000000..41f0c2ac --- /dev/null +++ b/internal/recordmirror/mirror_test.go @@ -0,0 +1,261 @@ +package recordmirror + +import ( + "encoding/json" + "errors" + "slices" + "sync" + "testing" + "time" + + "github.com/nats-io/nats.go" + "github.com/nats-io/nats.go/jetstream" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/link" + "github.com/novox/mesh-controller/internal/testbus" +) + +// A record mirror against a real bus (novox/hq design 54 §2, §3; ADR 0289): a change of a record changes +// exactly one key and raises exactly one event; a pass over unchanged records writes nothing and says +// nothing; a record gone is a purge said as `removed`. + +// aMirrorBus is a bus of this test alone with the mesh's streams and the controller's buckets, as the +// controller asserts them at its start, and a counter of the writes made to one bucket. +func aMirrorBus(t *testing.T) (*broker.JetStream, jetstream.JetStream) { + t.Helper() + js, err := broker.Dial(testbus.URL(t)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(js.Close) + if err := broker.AssertMeshStreams(js); err != nil { + t.Fatal(err) + } + if err := js.EnsureControllerBuckets(); err != nil { + t.Fatal(err) + } + api, err := jetstream.New(js.Conn()) + if err != nil { + t.Fatal(err) + } + return js, api +} + +// heard counts what is said on one subject, each body kept. +type heard struct { + mu sync.Mutex + bodies []Event +} + +func (h *heard) all() []Event { + h.mu.Lock() + defer h.mu.Unlock() + return append([]Event(nil), h.bodies...) +} + +func listen(t *testing.T, conn *nats.Conn, subject string) *heard { + t.Helper() + h := &heard{} + sub, err := conn.Subscribe(subject, func(m *nats.Msg) { + var e Event + if err := json.Unmarshal(m.Data, &e); err != nil { + t.Errorf("an event that is not one: %s", m.Data) + return + } + h.mu.Lock() + h.bodies = append(h.bodies, e) + h.mu.Unlock() + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = sub.Unsubscribe() }) + if err := conn.Flush(); err != nil { + t.Fatal(err) + } + return h +} + +// writes is how many messages the bucket's stream has taken: every put and every purge is one. +func writes(t *testing.T, api jetstream.JetStream, bucket string) uint64 { + t.Helper() + s, err := api.Stream(t.Context(), "KV_"+bucket) + if err != nil { + t.Fatal(err) + } + info, err := s.Info(t.Context()) + if err != nil { + t.Fatal(err) + } + return info.State.LastSeq +} + +// settle waits until n events are heard or a second passed, and answers what was heard. +func settle(h *heard, n int) []Event { + deadline := time.Now().Add(time.Second) + for time.Now().Before(deadline) && len(h.all()) < n { + time.Sleep(10 * time.Millisecond) + } + // And a moment more, so an event too many is heard too. + time.Sleep(100 * time.Millisecond) + return h.all() +} + +func aWriter(t *testing.T, js *broker.JetStream, api jetstream.JetStream, m Mirror) *Writer { + t.Helper() + kv, err := api.KeyValue(t.Context(), m.Bucket) + if err != nil { + t.Fatal(err) + } + return &Writer{Mirror: m, KV: kv, Announce: link.OverNATS{Conn: js.Conn(), JS: js.Context()}, + Epoch: func() uint64 { return 7 }} +} + +func TestNatsAMirrorChangesOneKeyAndRaisesOneEventPerChange(t *testing.T) { + for _, m := range []Mirror{Assignments, DeclaredConfig} { + t.Run(m.Name, func(t *testing.T) { + js, api := aMirrorBus(t) + events := listen(t, js.Conn(), "mesh.seat."+broker.ControllerSeat+".event."+m.Event) + w := aWriter(t, js, api, m) + ctx := t.Context() + + first := map[string][]byte{"anchor.gitea": []byte(`{"a":1}`)} + if _, err := w.Sync(ctx, first); err != nil { + t.Fatal(err) + } + before := writes(t, api, m.Bucket) + heardBefore := len(settle(events, 1)) + + // One record changes, one is added: two keys, two events, nothing else. + second := map[string][]byte{"anchor.gitea": []byte(`{"a":2}`), "anchor.forgejo": []byte(`{"a":1}`)} + r, err := w.Sync(ctx, second) + if err != nil { + t.Fatal(err) + } + if r.Written != 2 || r.Removed != 0 { + t.Errorf("a pass over one changed and one new record wrote %+v", r) + } + if got := writes(t, api, m.Bucket) - before; got != 2 { + t.Errorf("the bucket took %d writes for two changed records", got) + } + said := settle(events, heardBefore+2)[heardBefore:] + if len(said) != 2 { + t.Fatalf("two changes raised %d events: %+v", len(said), said) + } + changes := map[string]Event{} + for _, e := range said { + changes[e.Key] = e + } + if e := changes["anchor.gitea"]; e.Change != Changed || !slices.Equal(e.Fields, []string{"a"}) || + e.Mirror != m.Name || e.Epoch != 7 || e.Revision == 0 { + t.Errorf("the change of anchor.gitea was said as %+v", e) + } + if e := changes["anchor.forgejo"]; e.Change != Added { + t.Errorf("the new anchor.forgejo was said as %+v", e) + } + + // **An unchanged pass writes nothing**, and says nothing (ADR 0289 §1). + heardBefore = len(events.all()) + before = writes(t, api, m.Bucket) + r, err = w.Sync(ctx, second) + if err != nil { + t.Fatal(err) + } + if r != (Result{}) { + t.Errorf("a pass over unchanged records did %+v", r) + } + if got := writes(t, api, m.Bucket) - before; got != 0 { + t.Errorf("a pass over unchanged records wrote the bucket %d times", got) + } + if said := settle(events, heardBefore+1)[heardBefore:]; len(said) != 0 { + t.Errorf("a pass over unchanged records raised %+v", said) + } + + // **A record gone is a purge**, one write and one `removed`. + before = writes(t, api, m.Bucket) + r, err = w.Sync(ctx, map[string][]byte{"anchor.gitea": []byte(`{"a":2}`)}) + if err != nil { + t.Fatal(err) + } + if r.Removed != 1 || r.Written != 0 { + t.Errorf("a record gone did %+v", r) + } + if got := writes(t, api, m.Bucket) - before; got != 1 { + t.Errorf("a record gone wrote the bucket %d times", got) + } + if _, err := w.KV.Get(ctx, "anchor.forgejo"); !errors.Is(err, jetstream.ErrKeyNotFound) { + t.Errorf("the key of a record gone is still read: %v", err) + } + said = settle(events, heardBefore+1)[heardBefore:] + if len(said) != 1 || said[0].Change != Removed || said[0].Key != "anchor.forgejo" { + t.Errorf("a record gone was said as %+v", said) + } + }) + } +} + +// **A controller started again writes nothing for what the bucket already holds** (design 54 §2: the writer's +// memory is filled from the bucket), and a key nobody holds a record for is removed. +func TestNatsAWriterStartedAgainWritesOnlyWhatDiffers(t *testing.T) { + js, api := aMirrorBus(t) + ctx := t.Context() + first := aWriter(t, js, api, Assignments) + held := map[string][]byte{"anchor.gitea": []byte(`{"a":1}`), "anchor.forgejo": []byte(`{"a":1}`)} + if _, err := first.Sync(ctx, held); err != nil { + t.Fatal(err) + } + before := writes(t, api, Assignments.Bucket) + again := aWriter(t, js, api, Assignments) + r, err := again.Sync(ctx, held) + if err != nil { + t.Fatal(err) + } + if r != (Result{}) || writes(t, api, Assignments.Bucket) != before { + t.Errorf("a writer started again over unchanged records did %+v", r) + } +} + +// **Another writer in between is no lost value** (design 54 §2, compare-and-set): a write refused because the +// key moved reads it again, takes the revision when it already holds the value, and otherwise writes again. +func TestNatsAWriteRefusedByCompareAndSetReadsAgain(t *testing.T) { + js, api := aMirrorBus(t) + ctx := t.Context() + w := aWriter(t, js, api, Assignments) + if _, err := w.Sync(ctx, map[string][]byte{"anchor.gitea": []byte(`{"a":1}`)}); err != nil { + t.Fatal(err) + } + // Behind its back, as a former lease holder would. + if _, err := w.KV.Put(ctx, "anchor.gitea", []byte(`{"a":9}`)); err != nil { + t.Fatal(err) + } + r, err := w.Sync(ctx, map[string][]byte{"anchor.gitea": []byte(`{"a":2}`)}) + if err != nil { + t.Fatal(err) + } + if r.Written != 1 { + t.Errorf("a refused write was not made again: %+v", r) + } + e, err := w.KV.Get(ctx, "anchor.gitea") + if err != nil || string(e.Value()) != `{"a":2}` { + t.Fatalf("the bucket holds %v, %v", e, err) + } +} + +// **A key the bus cannot hold is refused alone**, and the others are written. +func TestAKeyTheBusCannotHoldIsRefusedAlone(t *testing.T) { + if validKey("anchor.gitea") != nil || validKey("bad key") == nil || validKey(".lead") == nil || + validKey("") == nil || validKey("trail.") == nil { + t.Error("the keys a bucket may hold are not told apart") + } +} + +func TestTheFieldsThatChangedAreNamed(t *testing.T) { + got := changedFields([]byte(`{"a":1,"b":{"x":1},"c":"same"}`), []byte(`{"a":2,"b":{"x":1},"d":true,"c":"same"}`)) + if !slices.Equal(got, []string{"a", "d"}) { + t.Errorf("the fields that changed are %v", got) + } + if got := changedFields(nil, []byte(`{"a":1}`)); got != nil { + t.Errorf("an added key names fields %v", got) + } +} -- 2.54.0 From 3218fc3c1bdc3337d55d1e74d2ea3402155e2240 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 00:33:05 +0000 Subject: [PATCH 4/8] Hold the composed grant and the captured genesis template to the mirrors The controller's composition gains the two mirror buckets and their events, so the golden composition moves, and the genesis template is captured again from mesh-host's branch of this package, whose installer user list carries them. Co-Authored-By: Claude Opus 5.5 --- internal/broker/testdata/composed.conf | 2 +- testdata/beside/CAPTURED | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/broker/testdata/composed.conf b/internal/broker/testdata/composed.conf index 8270aaf3..3476c950 100644 --- a/internal/broker/testdata/composed.conf +++ b/internal/broker/testdata/composed.conf @@ -24,7 +24,7 @@ accounts { jetstream: enabled users = [ { user: "controller", password: "$2a$11$cccccccccccccccccccccc", permissions: { - publish: { allow: ["$JS.ACK.CONTROL.controller.>", "$JS.ACK.DEAD_LETTER_NOTICES.controller.>", "$JS.ACK.EVENTS.controller.>", "$JS.API.>", "$KV.SEAT_MESH_BUILD_MACHINE_cancelled.>", "$KV.SEAT_NODE_BUILD_AGENT_cancelled.>", "$KV.mesh-controller_asked.>", "$KV.mesh-controller_calls.>", "$KV.mesh-controller_condition-history.>", "$KV.mesh-controller_conditions.>", "$KV.mesh-controller_hand-acts.>", "$KV.mesh-controller_lease.>", "$SRV.INFO", "_INBOX.enrol.>", "mesh.again.>", "mesh.assignment.>", "mesh.events.dead.>", "mesh.mod.*.tool.>", "mesh.node.>", "mesh.seat.mesh-build-machine.accept.>", "mesh.seat.mesh-build-machine.tool.>", "mesh.seat.mesh-controller.event.applied", "mesh.seat.mesh-controller.event.built-before", "mesh.seat.mesh-controller.event.checked", "mesh.seat.mesh-controller.event.condition-changed", "mesh.seat.mesh-controller.event.condition-cleared", "mesh.seat.mesh-controller.event.condition-raised", "mesh.seat.mesh-controller.event.doctor-heartbeat", "mesh.seat.mesh-controller.event.healer-acted", "mesh.seat.mesh-controller.event.plan-moved", "mesh.seat.mesh-controller.event.refused", "mesh.seat.mesh-controller.event.rolled-back", "mesh.seat.mesh-controller.event.secret-replaced", "mesh.seat.mesh-controller.tool.plans", "mesh.seat.mesh-delivery.tool.close", "mesh.seat.mesh-delivery.tool.groups", "mesh.seat.mesh-delivery.tool.release", "mesh.seat.mesh-delivery.tool.stalled", "mesh.seat.mesh-delivery.tool.stop", "mesh.seat.mesh-delivery.tool.times", "mesh.seat.node-backup.tool.backed-up.*", "mesh.seat.node-backup.tool.now.*", "mesh.seat.node-build-agent.accept.>", "mesh.seat.node-build-agent.tool.>", "mesh.seat.node-intrusion-prevention.tool.banned.*", "mesh.seat.node-launcher.tool.secret.*", "mesh.seat.node-service-manager.tool.restart.*"] } + publish: { allow: ["$JS.ACK.CONTROL.controller.>", "$JS.ACK.DEAD_LETTER_NOTICES.controller.>", "$JS.ACK.EVENTS.controller.>", "$JS.API.>", "$KV.SEAT_MESH_BUILD_MACHINE_cancelled.>", "$KV.SEAT_NODE_BUILD_AGENT_cancelled.>", "$KV.mesh-controller_asked.>", "$KV.mesh-controller_assignments.>", "$KV.mesh-controller_calls.>", "$KV.mesh-controller_condition-history.>", "$KV.mesh-controller_conditions.>", "$KV.mesh-controller_declared-config.>", "$KV.mesh-controller_hand-acts.>", "$KV.mesh-controller_lease.>", "$SRV.INFO", "_INBOX.enrol.>", "mesh.again.>", "mesh.assignment.>", "mesh.events.dead.>", "mesh.mod.*.tool.>", "mesh.node.>", "mesh.seat.mesh-build-machine.accept.>", "mesh.seat.mesh-build-machine.tool.>", "mesh.seat.mesh-controller.event.applied", "mesh.seat.mesh-controller.event.assignment-changed", "mesh.seat.mesh-controller.event.built-before", "mesh.seat.mesh-controller.event.checked", "mesh.seat.mesh-controller.event.condition-changed", "mesh.seat.mesh-controller.event.condition-cleared", "mesh.seat.mesh-controller.event.condition-raised", "mesh.seat.mesh-controller.event.declared-config-changed", "mesh.seat.mesh-controller.event.doctor-heartbeat", "mesh.seat.mesh-controller.event.healer-acted", "mesh.seat.mesh-controller.event.plan-moved", "mesh.seat.mesh-controller.event.refused", "mesh.seat.mesh-controller.event.rolled-back", "mesh.seat.mesh-controller.event.secret-replaced", "mesh.seat.mesh-controller.tool.plans", "mesh.seat.mesh-delivery.tool.close", "mesh.seat.mesh-delivery.tool.groups", "mesh.seat.mesh-delivery.tool.release", "mesh.seat.mesh-delivery.tool.stalled", "mesh.seat.mesh-delivery.tool.stop", "mesh.seat.mesh-delivery.tool.times", "mesh.seat.node-backup.tool.backed-up.*", "mesh.seat.node-backup.tool.now.*", "mesh.seat.node-build-agent.accept.>", "mesh.seat.node-build-agent.tool.>", "mesh.seat.node-intrusion-prevention.tool.banned.*", "mesh.seat.node-launcher.tool.secret.*", "mesh.seat.node-service-manager.tool.restart.*"] } subscribe: { allow: ["$JS.API.>", "$JS.EVENT.ADVISORY.CONSUMER.DELETED.>", "$JS.EVENT.ADVISORY.CONSUMER.MAX_DELIVERIES.>", "$SRV.INFO", "$SRV.INFO.mesh-controller", "$SRV.INFO.mesh-controller.>", "$SRV.PING", "$SRV.PING.mesh-controller", "$SRV.PING.mesh-controller.>", "$SRV.STATS", "$SRV.STATS.mesh-controller", "$SRV.STATS.mesh-controller.>", "_DELIVER.controller", "_DELIVER.controller.>", "_INBOX.controller.>", "mesh.control.>", "mesh.mod.*.event.provisioner.failing", "mesh.mod.*.event.provisioner.recovered", "mesh.mod.*.event.provisioner.retirement", "mesh.mod.gitea.event.pull.merged", "mesh.mod.gitea.event.pull.updated", "mesh.mod.mesh-catalog.event.catching-up", "mesh.mod.mesh-catalog.event.upgraded", "mesh.seat.mesh-build-machine.event.built", "mesh.seat.mesh-controller.tool.>", "mesh.seat.node-build-agent.event.built", "mesh.seat.operator-channel.event.decided.mesh-controller"] } allow_responses: { max: 1, ttl: "1m" } } } diff --git a/testdata/beside/CAPTURED b/testdata/beside/CAPTURED index 5e6e1f87..cf215bef 100644 --- a/testdata/beside/CAPTURED +++ b/testdata/beside/CAPTURED @@ -8,7 +8,7 @@ private addresses. That is acceptable while mesh-controller is private; if it ev a second place to clean besides the catalogue itself. mesh-catalog e0bc919d4945bb5f13f971f74643a0b714cf45da modules/*/module.json, modules/nats/Dockerfile -mesh-host 7a41e120635b3f7b6cb94cca8b3fc4093e4e6110 examples/foundation-first-node.lock +mesh-host c8920622516548978d92e734f3991a262f98a742 examples/foundation-first-node.lock mesh-sdk 5933eb757d443293631be2bed27a96288c2348d8 conformance/events/module-event.json To move them, from this repository's root, with the two repositories checked out beside it: -- 2.54.0 From 8563b90918fb153902b611608968e4d8370766be Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 00:34:24 +0000 Subject: [PATCH 5/8] Capture the genesis template from mesh-host's branch of this package The template at mesh-host c8920622 carries the controller's grant to write the two record mirrors and say their events; the genesis template test holds the controller's composition to it. Co-Authored-By: Claude Opus 5.5 --- .../examples/foundation-first-node.lock | 108 +++++++++++++----- 1 file changed, 79 insertions(+), 29 deletions(-) diff --git a/testdata/beside/mesh-host/examples/foundation-first-node.lock b/testdata/beside/mesh-host/examples/foundation-first-node.lock index 289e3e2a..5e05f7f3 100644 --- a/testdata/beside/mesh-host/examples/foundation-first-node.lock +++ b/testdata/beside/mesh-host/examples/foundation-first-node.lock @@ -28,11 +28,12 @@ // image, and this file is applied on a machine with no mesh to ask about anything (ADR 0006). The // registry prefix is the lab's; the digests are upstream's. // -// The store's data is a NAMED VOLUME, not a directory on the machine. A directory the host -// creates is owned by root, and the database runs as somebody else inside the container — so it -// could not write, and the container crash-looped. A named volume lets the image set up its own -// ownership, and outlives the container, which is what you want for the thing holding the mesh's -// state. +// **The store is raised where the `postgres` module keeps it** (novox/hq ADR 0338): the container +// `postgres`, on the module's pgvector image, its data in the directory /var/lib/mesh-store owned by +// the image's postgres user, its superuser password read from the module's own-secret file — so the +// store step adopts it as it runs. Pinned upstream by the module's own digest, not through the lab's +// registry prefix, because the two must be the same reference (mesh-host internal/bootstrap holds +// them to each other against the module's manifest). { "declaration": 1, @@ -83,17 +84,28 @@ "boot": "enabled", "restart-on": ["base-filter"] }, + // The store's data is a directory owned by the image's postgres user (999), made before the + // container starts: a directory the runtime made would be root's, and the database could not + // write it. Measured on the module's image 2026-10-13: it initialises a new store on a directory + // made so (novox/hq ADR 0338 §5). + { + "id": "store-data", + "type": "directory", + "path": "/var/lib/mesh-store", + "mode": "0700", + "owner": "999:70" + }, { "id": "store", "type": "container", - "name": "mesh-store", - "image": "192.0.2.250:5000/postgres@sha256:7abf537131b66ed5af448d90653abf1679b0c7e9a1f07efdd4c3108a401b259a", + "name": "postgres", + "image": "pgvector/pgvector@sha256:cf134a767f474095eeba57e0117be8e568e011a63f33fbf252f14c9b760f8e6f", "env": { "POSTGRES_PASSWORD": "bootstrap", "PGDATA": "/var/lib/postgresql/data/pgdata" }, "ports": ["5432:5432"], - "volumes": ["mesh-store-data:/var/lib/postgresql/data"] + "volumes": ["/var/lib/mesh-store:/var/lib/postgresql/data"] }, // Over TCP, not the socket. While the store initialises it runs a temporary server on the // socket ONLY, then stops it and starts the real one — so a socket check passes, the action @@ -102,21 +114,21 @@ { "id": "store-ready", "type": "action", - "in": "mesh-store", + "in": "postgres", "command": ["sh", "-c", "for i in $(seq 1 180); do pg_isready -h 127.0.0.1 -U postgres >/dev/null 2>&1 && exit 0; sleep 1; done; echo 'the store did not answer within 180s; its own last words follow'; pg_isready -h 127.0.0.1 -U postgres; tail -n 20 /var/lib/postgresql/data/log/*.log 2>/dev/null; exit 1"], "verify": ["pg_isready", "-h", "127.0.0.1", "-U", "postgres"] }, { "id": "inventory-database", "type": "action", - "in": "mesh-store", + "in": "postgres", "command": ["sh", "-c", "psql -U postgres -c 'CREATE DATABASE inventory'"], "verify": ["sh", "-c", "psql -U postgres -lqt | cut -d'|' -f1 | grep -qw inventory"] }, { "id": "identity-database", "type": "action", - "in": "mesh-store", + "in": "postgres", "command": ["sh", "-c", "psql -U postgres -c 'CREATE DATABASE identity'"], "verify": ["sh", "-c", "psql -U postgres -lqt | cut -d'|' -f1 | grep -qw identity"] }, @@ -126,20 +138,44 @@ { "id": "licences-database", "type": "action", - "in": "mesh-store", + "in": "postgres", "command": ["sh", "-c", "psql -U postgres -c 'CREATE DATABASE licences'"], "verify": ["sh", "-c", "psql -U postgres -lqt | cut -d'|' -f1 | grep -qw licences"] }, { "id": "context-schemas", "type": "action", - "command": ["docker", "run", "--rm", "--network", "container:mesh-store", + "command": ["docker", "run", "--rm", "--network", "container:postgres", "-e", "MESH_STORE_INVENTORY=postgres://postgres:bootstrap@127.0.0.1:5432/inventory?sslmode=disable", "-e", "MESH_STORE_IDENTITY=postgres://postgres:bootstrap@127.0.0.1:5432/identity?sslmode=disable", "-e", "MESH_STORE_LICENCES=postgres://postgres:bootstrap@127.0.0.1:5432/licences?sslmode=disable", "192.0.2.250:5000/mesh-controller@sha256:c67db38439ff0aee242b467486765467bb95801f52175fc5727cc4e437338ace", "migrate"], - "verify": ["sh", "-c", "docker exec mesh-store psql -U postgres -d inventory -tAc \"select to_regclass('public.node')\" | grep -qx node && docker exec mesh-store psql -U postgres -d identity -tAc \"select to_regclass('public.signing_key')\" | grep -qx signing_key && docker exec mesh-store psql -U postgres -d licences -tAc \"select to_regclass('public.licence')\" | grep -qx licence"] + "verify": ["sh", "-c", "docker exec postgres psql -U postgres -d inventory -tAc \"select to_regclass('public.node')\" | grep -qx node && docker exec postgres psql -U postgres -d identity -tAc \"select to_regclass('public.signing_key')\" | grep -qx signing_key && docker exec postgres psql -U postgres -d licences -tAc \"select to_regclass('public.licence')\" | grep -qx licence"] + }, + // **The bus is raised where the `nats` module keeps it** (novox/hq ADR 0333 §1): the container + // `mesh-broker-nats`, its configuration and user list in /var/lib/nats-module/conf, its streams in + // /var/lib/mesh-broker-nats and its certificate in /var/lib/mesh-broker-tls — the names and paths the + // catalogue's module declares. So adopting it as that module at the broker step keeps it: the + // module's built image takes the place of the upstream one below, once, on the same streams, users + // and certificate, and nothing else moves. mesh-host's internal/bootstrap holds the two to each other + // against the module's own manifest (busLayout), at the broker step and in its tests. + // + // Directories on the machine, not volumes the runtime names: the module mounts these paths, and the + // control plane, once it runs as the mesh's process, reads the certificate at its path on the machine. + // The certificate's directory is readable by everyone because the certificate is public and the control + // plane runs as nobody; its key is written 0600 by the program that makes it. + { + "id": "bus-tls-dir", + "type": "directory", + "path": "/var/lib/mesh-broker-tls", + "mode": "0755" + }, + { + "id": "bus-data-dir", + "type": "directory", + "path": "/var/lib/mesh-broker-nats", + "mode": "0700" }, { "id": "bus-certificate", @@ -151,49 +187,63 @@ // it, and asking nothing of the image it writes into. Self-signed on purpose: a host pins // this server's exact certificate (novox/hq ADR 0004), and at this moment there is no mesh // to ask an authority of. - // `--user 0:0` because the volume is root's and this image runs as nobody, which is right - // for the long-running control plane and wrong for a one-shot writing into a fresh volume. - "command": ["docker", "run", "--rm", "--user", "0:0", "-v", "mesh-broker-tls:/tls", + // `--user 0:0` because the directory is root's and this image runs as nobody, which is right + // for the long-running control plane and wrong for a one-shot writing into a fresh directory. + "command": ["docker", "run", "--rm", "--user", "0:0", "-v", "/var/lib/mesh-broker-tls:/tls", "192.0.2.250:5000/mesh-controller@sha256:c67db38439ff0aee242b467486765467bb95801f52175fc5727cc4e437338ace", "broker", "certificate", "--into", "/tls"], - "verify": ["docker", "run", "--rm", "--user", "0:0", "-v", "mesh-broker-tls:/tls", + "verify": ["docker", "run", "--rm", "--user", "0:0", "-v", "/var/lib/mesh-broker-tls:/tls", "192.0.2.250:5000/mesh-controller@sha256:c67db38439ff0aee242b467486765467bb95801f52175fc5727cc4e437338ace", "broker", "certificate", "--check", "--into", "/tls"] }, + // **Seeds, written only where nothing is** (create-once). The bus's configuration and user list are + // the bundle's until the bus is adopted, and the module's from then on (ADR 0333 §4): an installer + // run again after the adoption applies this bundle again, and an ordinary file would put the + // bundle's one user back over every user the mesh composed, and leave the node unable to reach its + // bus. Until the adoption the installer places the composed list over the seed (enrol.go). { "id": "bus-conf-dir", "type": "directory", - "path": "/var/lib/mesh-bus-conf", + "path": "/var/lib/nats-module/conf", "mode": "0700" }, { "id": "bus-conf", "type": "file", - "path": "/var/lib/mesh-bus-conf/nats.conf", + "path": "/var/lib/nats-module/conf/nats.conf", "mode": "0644", - "content": "port: 4222\nhttp: 127.0.0.1:8222\n\ntls {\n cert_file: \"/tls/tls.crt\"\n key_file: \"/tls/tls.key\"\n}\n\njetstream {\n store_dir: \"/data\"\n}\n\ninclude accounts.conf\n" + "create-once": true, + "content": "port: 4222\nhttp: 0.0.0.0:8222\n\ntls {\n cert_file: \"/tls/tls.crt\"\n key_file: \"/tls/tls.key\"\n}\n\njetstream {\n store_dir: \"/data\"\n}\n\ninclude accounts.conf\n" }, { "id": "bus-accounts", "type": "file", - "path": "/var/lib/mesh-bus-conf/accounts.conf", + "path": "/var/lib/nats-module/conf/accounts.conf", "mode": "0600", - "content": "// The first user list, carried by the installer because at genesis there is no mesh to\n// compose one. A bootstrap credential, rotated with the store's and replaced by the\n// controller's own composition from its first start onward.\naccounts {\n MESH {\n jetstream: enabled\n users = [\n { user: \"controller\", password: \"$2a$10$AHqJgOifIVbU41KmATiMhuXFs8xa7Wl2HuN4UVBCXdN2jIQzjqApy\", permissions: {\n publish: { allow: [\"$JS.ACK.CONTROL.controller.>\", \"$JS.ACK.EVENTS.controller.>\", \"$JS.ACK.DEAD_LETTER_NOTICES.controller.>\", \"$JS.API.>\", \"$KV.mesh-controller_asked.>\", \"$KV.mesh-controller_calls.>\", \"$KV.mesh-controller_hand-acts.>\", \"$KV.mesh-controller_conditions.>\", \"$KV.mesh-controller_condition-history.>\", \"$KV.mesh-controller_lease.>\", \"$KV.SEAT_MESH_BUILD_MACHINE_cancelled.>\", \"$KV.SEAT_NODE_BUILD_AGENT_cancelled.>\", \"_INBOX.enrol.>\", \"mesh.assignment.>\", \"mesh.mod.*.tool.>\", \"mesh.node.>\", \"mesh.seat.mesh-build-machine.accept.>\", \"mesh.seat.node-build-agent.accept.>\", \"mesh.seat.mesh-build-machine.tool.>\", \"mesh.seat.node-build-agent.tool.>\", \"mesh.seat.mesh-controller.event.applied\", \"mesh.seat.mesh-controller.event.built-before\", \"mesh.seat.mesh-controller.event.refused\", \"mesh.seat.mesh-controller.event.rolled-back\", \"mesh.seat.mesh-controller.event.checked\", \"mesh.seat.mesh-controller.event.plan-moved\", \"mesh.seat.mesh-controller.event.condition-raised\", \"mesh.seat.mesh-controller.event.condition-changed\", \"mesh.seat.mesh-controller.event.condition-cleared\", \"mesh.seat.mesh-controller.event.doctor-heartbeat\", \"mesh.seat.mesh-controller.event.secret-replaced\", \"mesh.seat.mesh-controller.event.healer-acted\", \"$SRV.INFO\", \"mesh.seat.node-intrusion-prevention.tool.banned.*\", \"mesh.seat.node-backup.tool.backed-up.*\", \"mesh.seat.node-backup.tool.now.*\", \"mesh.seat.mesh-delivery.tool.stalled\", \"mesh.seat.mesh-delivery.tool.close\", \"mesh.seat.node-launcher.tool.secret.*\", \"mesh.seat.mesh-delivery.tool.release\", \"mesh.seat.mesh-delivery.tool.stop\", \"mesh.seat.mesh-delivery.tool.times\", \"mesh.seat.mesh-delivery.tool.groups\", \"mesh.seat.node-service-manager.tool.restart.*\", \"mesh.seat.mesh-controller.tool.plans\", \"mesh.events.dead.>\", \"mesh.again.>\"] }\n subscribe: { allow: [\"$JS.API.>\", \"$JS.EVENT.ADVISORY.CONSUMER.MAX_DELIVERIES.>\", \"$JS.EVENT.ADVISORY.CONSUMER.DELETED.>\", \"_DELIVER.controller\", \"_DELIVER.controller.>\", \"_INBOX.controller.>\", \"mesh.control.>\", \"mesh.mod.*.event.provisioner.failing\", \"mesh.mod.*.event.provisioner.recovered\", \"mesh.mod.*.event.provisioner.retirement\", \"mesh.mod.gitea.event.pull.merged\", \"mesh.mod.gitea.event.pull.updated\", \"mesh.mod.mesh-catalog.event.catching-up\", \"mesh.mod.mesh-catalog.event.upgraded\", \"mesh.seat.mesh-build-machine.event.built\", \"mesh.seat.node-build-agent.event.built\", \"mesh.seat.operator-channel.event.decided.mesh-controller\", \"mesh.seat.mesh-controller.tool.>\", \"$SRV.PING\", \"$SRV.INFO\", \"$SRV.PING.mesh-controller\", \"$SRV.PING.mesh-controller.>\", \"$SRV.INFO.mesh-controller\", \"$SRV.INFO.mesh-controller.>\", \"$SRV.STATS\", \"$SRV.STATS.mesh-controller\", \"$SRV.STATS.mesh-controller.>\"] }\n allow_responses: { max: 1, ttl: \"1m\" }\n } }\n ]\n }\n}\n" + "create-once": true, + "content": "// The first user list, carried by the installer because at genesis there is no mesh to\n// compose one. A bootstrap credential, rotated with the store's and replaced by the\n// controller's own composition from its first start onward.\naccounts {\n MESH {\n jetstream: enabled\n users = [\n { user: \"controller\", password: \"$2a$10$AHqJgOifIVbU41KmATiMhuXFs8xa7Wl2HuN4UVBCXdN2jIQzjqApy\", permissions: {\n publish: { allow: [\"$JS.ACK.CONTROL.controller.>\", \"$JS.ACK.EVENTS.controller.>\", \"$JS.ACK.DEAD_LETTER_NOTICES.controller.>\", \"$JS.API.>\", \"$KV.mesh-controller_asked.>\", \"$KV.mesh-controller_assignments.>\", \"$KV.mesh-controller_calls.>\", \"$KV.mesh-controller_hand-acts.>\", \"$KV.mesh-controller_conditions.>\", \"$KV.mesh-controller_condition-history.>\", \"$KV.mesh-controller_declared-config.>\", \"$KV.mesh-controller_lease.>\", \"$KV.SEAT_MESH_BUILD_MACHINE_cancelled.>\", \"$KV.SEAT_NODE_BUILD_AGENT_cancelled.>\", \"_INBOX.enrol.>\", \"mesh.assignment.>\", \"mesh.mod.*.tool.>\", \"mesh.node.>\", \"mesh.seat.mesh-build-machine.accept.>\", \"mesh.seat.node-build-agent.accept.>\", \"mesh.seat.mesh-build-machine.tool.>\", \"mesh.seat.node-build-agent.tool.>\", \"mesh.seat.mesh-controller.event.applied\", \"mesh.seat.mesh-controller.event.built-before\", \"mesh.seat.mesh-controller.event.refused\", \"mesh.seat.mesh-controller.event.rolled-back\", \"mesh.seat.mesh-controller.event.checked\", \"mesh.seat.mesh-controller.event.plan-moved\", \"mesh.seat.mesh-controller.event.assignment-changed\", \"mesh.seat.mesh-controller.event.declared-config-changed\", \"mesh.seat.mesh-controller.event.condition-raised\", \"mesh.seat.mesh-controller.event.condition-changed\", \"mesh.seat.mesh-controller.event.condition-cleared\", \"mesh.seat.mesh-controller.event.doctor-heartbeat\", \"mesh.seat.mesh-controller.event.secret-replaced\", \"mesh.seat.mesh-controller.event.healer-acted\", \"$SRV.INFO\", \"mesh.seat.node-intrusion-prevention.tool.banned.*\", \"mesh.seat.node-backup.tool.backed-up.*\", \"mesh.seat.node-backup.tool.now.*\", \"mesh.seat.mesh-delivery.tool.stalled\", \"mesh.seat.mesh-delivery.tool.close\", \"mesh.seat.node-launcher.tool.secret.*\", \"mesh.seat.mesh-delivery.tool.release\", \"mesh.seat.mesh-delivery.tool.stop\", \"mesh.seat.mesh-delivery.tool.times\", \"mesh.seat.mesh-delivery.tool.groups\", \"mesh.seat.node-service-manager.tool.restart.*\", \"mesh.seat.mesh-controller.tool.plans\", \"mesh.events.dead.>\", \"mesh.again.>\"] }\n subscribe: { allow: [\"$JS.API.>\", \"$JS.EVENT.ADVISORY.CONSUMER.MAX_DELIVERIES.>\", \"$JS.EVENT.ADVISORY.CONSUMER.DELETED.>\", \"_DELIVER.controller\", \"_DELIVER.controller.>\", \"_INBOX.controller.>\", \"mesh.control.>\", \"mesh.mod.*.event.provisioner.failing\", \"mesh.mod.*.event.provisioner.recovered\", \"mesh.mod.*.event.provisioner.retirement\", \"mesh.mod.gitea.event.pull.merged\", \"mesh.mod.gitea.event.pull.updated\", \"mesh.mod.mesh-catalog.event.catching-up\", \"mesh.mod.mesh-catalog.event.upgraded\", \"mesh.seat.mesh-build-machine.event.built\", \"mesh.seat.node-build-agent.event.built\", \"mesh.seat.operator-channel.event.decided.mesh-controller\", \"mesh.seat.mesh-controller.tool.>\", \"$SRV.PING\", \"$SRV.INFO\", \"$SRV.PING.mesh-controller\", \"$SRV.PING.mesh-controller.>\", \"$SRV.INFO.mesh-controller\", \"$SRV.INFO.mesh-controller.>\", \"$SRV.STATS\", \"$SRV.STATS.mesh-controller\", \"$SRV.STATS.mesh-controller.>\"] }\n allow_responses: { max: 1, ttl: \"1m\" }\n } }\n ]\n }\n}\n" }, + // The upstream server, until the broker step adopts it: the `nats` module's own image, built by the + // mesh, takes its place there once, on these same directories (ADR 0333 §3), as the bus's planned + // step (ADR 0236). The release is the one the module's image is built on (its NATS_BASE, 2.11.17), + // pinned upstream by that same digest: the adoption changes no server release, so it can be undone + // by putting the old image back and is said reversible. The published port is the + // node's bus port, 5671 unless given, which every enrolment token names; the installer gives the + // module the setting that maps its declared 4222 to it (ADR 0333 §2). { "id": "broker", "type": "container", - "name": "mesh-broker", - "image": "192.0.2.250:5000/nats@sha256:b83efabe3e7def1e0a4a31ec6e078999bb17c80363f881df35edc70fcb6bb927", + "name": "mesh-broker-nats", + "image": "nats@sha256:e4bf19f15fd3218814a4e3c9e0064e1334bd8aa20d5984b9f1a0afd084f8cc00", "ports": ["5671:4222", "127.0.0.1:8222:8222"], - "volumes": ["mesh-broker-data:/data", "mesh-broker-tls:/tls:ro", "/var/lib/mesh-bus-conf:/etc/nats:ro"], + "volumes": ["/var/lib/mesh-broker-nats:/data", "/var/lib/nats-module/conf:/etc/nats:ro", "/var/lib/mesh-broker-tls:/tls:ro"], "args": ["-c", "/etc/nats/nats.conf", "-js"] }, { "id": "broker-ready", "type": "action", - "command": ["sh", "-c", "for i in $(seq 1 60); do docker run --rm --network host --entrypoint sh 192.0.2.250:5000/nats@sha256:b83efabe3e7def1e0a4a31ec6e078999bb17c80363f881df35edc70fcb6bb927 -c 'nc -z 127.0.0.1 5671' >/dev/null 2>&1 && exit 0; sleep 1; done; exit 1"], - "verify": ["sh", "-c", "docker run --rm --network host --entrypoint sh 192.0.2.250:5000/nats@sha256:b83efabe3e7def1e0a4a31ec6e078999bb17c80363f881df35edc70fcb6bb927 -c 'nc -z 127.0.0.1 5671'"] + "command": ["sh", "-c", "for i in $(seq 1 60); do docker run --rm --network host --entrypoint sh nats@sha256:e4bf19f15fd3218814a4e3c9e0064e1334bd8aa20d5984b9f1a0afd084f8cc00 -c 'nc -z 127.0.0.1 5671' >/dev/null 2>&1 && exit 0; sleep 1; done; exit 1"], + "verify": ["sh", "-c", "docker run --rm --network host --entrypoint sh nats@sha256:e4bf19f15fd3218814a4e3c9e0064e1334bd8aa20d5984b9f1a0afd084f8cc00 -c 'nc -z 127.0.0.1 5671'"] }, { "id": "control-plane", @@ -202,7 +252,7 @@ "image": "192.0.2.250:5000/mesh-controller@sha256:c67db38439ff0aee242b467486765467bb95801f52175fc5727cc4e437338ace", "network": "host", "args": ["serve"], - "volumes": ["mesh-broker-tls:/broker-tls:ro"], + "volumes": ["/var/lib/mesh-broker-tls:/broker-tls:ro"], "env": { "MESH_STORE_INVENTORY": "postgres://postgres:bootstrap@127.0.0.1:5432/inventory?sslmode=disable", "MESH_STORE_IDENTITY": "postgres://postgres:bootstrap@127.0.0.1:5432/identity?sslmode=disable", -- 2.54.0 From 69367aa786427326d48acf270186a28d5f9a33ee Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 01:01:00 +0000 Subject: [PATCH 6/8] Correct a mirror written behind the writer's back, and write none without the lease MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of mesh-controller#315 (hq issue 652): the writer trusted the bucket as it read it once, so a former lease holder's write stayed for good; it wrote without the lease, asking only after a refused compare-and-set; and the declared config carried a manifest's declarations whole, so a field the manifest gains would reach the bus unreviewed (ADR 0289 §9, §10, §12). Every pass now reads the bucket and writes back the store's value where it differs, asks for the lease before the pass and before every write, and a declaration is mirrored as its listed fields (kind, default, why), with a test that fails on a declaration field the list does not name. Co-Authored-By: Claude Opus 5.5 --- internal/inventory/mirrors.go | 40 +++++++++--- internal/inventory/record_mirrors_test.go | 41 ++++++++++++ internal/recordmirror/mirror.go | 42 ++++++++----- internal/recordmirror/mirror_store_test.go | 2 +- internal/recordmirror/mirror_test.go | 72 +++++++++++++++++++++- 5 files changed, 172 insertions(+), 25 deletions(-) create mode 100644 internal/inventory/record_mirrors_test.go diff --git a/internal/inventory/mirrors.go b/internal/inventory/mirrors.go index 5d878a0f..c33db1c0 100644 --- a/internal/inventory/mirrors.go +++ b/internal/inventory/mirrors.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "time" ) @@ -25,14 +26,27 @@ type AssignmentMirrored struct { } // DeclaredConfigMirrored is one registered module's config declarations as the declared-config mirror holds -// them: the module, and its manifest's `settings` exactly as the manifest holds them (each key's kind, -// default and why today), carried whole so a declaration that gains fields (ADR 0348 clause 4) is mirrored -// without a change here. An empty object when the module declares none: registered, declaring nothing. +// them: the module, and each key its manifest's `settings` declares with that declaration's listed fields. +// An empty object when the module declares none: registered, declaring nothing. type DeclaredConfigMirrored struct { - Module string `json:"module"` - Settings json.RawMessage `json:"settings"` + Module string `json:"module"` + Settings map[string]DeclaredMirrored `json:"settings"` } +// DeclaredMirrored is one key's declaration as the mirror holds it: **its listed fields and no other** +// (ADR 0289 §12). Today a declaration is kind, default and why (catalogue.SettingDeclaration); a field the +// manifest gains reaches the bus only once it is added here, deliberately, and a test fails until it is +// listed or refused (ADR 0348 clause 4's fields arrive with package K1). A default is never a secret: the +// catalogue refuses a default for a key whose name says secret. +type DeclaredMirrored struct { + Kind string `json:"kind"` + Default json.RawMessage `json:"default"` + Why string `json:"why"` +} + +// DeclaredMirroredFields are the fields of a declaration the mirror carries, by their manifest names. +var DeclaredMirroredFields = []string{"kind", "default", "why"} + // AssignmentsMirror is every assignment's mirrored value, keyed `.`. func (i *Inventory) AssignmentsMirror(ctx context.Context) (map[string][]byte, error) { rows, err := i.store.Pool().Query(ctx, @@ -74,11 +88,19 @@ func (i *Inventory) DeclaredConfigMirror(ctx context.Context) (map[string][]byte if err := rows.Scan(&d.Module, &raw); err != nil { return nil, err } - settings, err := canonical(raw) - if err != nil { - return nil, err + var declared map[string]DeclaredMirrored + if err := json.Unmarshal(raw, &declared); err != nil { + return nil, fmt.Errorf("the declarations of %s cannot be read: %w", d.Module, err) + } + d.Settings = map[string]DeclaredMirrored{} + for key, decl := range declared { + if len(decl.Default) > 0 { + if decl.Default, err = canonical(decl.Default); err != nil { + return nil, err + } + } + d.Settings[key] = decl } - d.Settings = settings value, err := json.Marshal(d) if err != nil { return nil, err diff --git a/internal/inventory/record_mirrors_test.go b/internal/inventory/record_mirrors_test.go new file mode 100644 index 00000000..db2e9ba2 --- /dev/null +++ b/internal/inventory/record_mirrors_test.go @@ -0,0 +1,41 @@ +package inventory + +import ( + "reflect" + "slices" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// jsonFields is the names a struct's fields are encoded under. +func jsonFields(t reflect.Type) []string { + var out []string + for i := 0; i < t.NumField(); i++ { + name, _, _ := strings.Cut(t.Field(i).Tag.Get("json"), ",") + if name != "" && name != "-" { + out = append(out, name) + } + } + return out +} + +// **A declaration reaches the bus with its listed fields and no other** (ADR 0289 §12): the mirror's value +// carries exactly DeclaredMirroredFields, and every field a manifest's declaration has is on that list. A +// field a declaration gains (package K1 widens it, ADR 0348 clause 4) fails here until it is listed — read, +// judged not a secret and carried — so nothing reaches the declared-config mirror unreviewed. +func TestTheDeclaredConfigMirrorCarriesOnlyItsListedFields(t *testing.T) { + if got := jsonFields(reflect.TypeOf(DeclaredMirrored{})); !slices.Equal(got, DeclaredMirroredFields) { + t.Errorf("the mirrored declaration carries %v, its list is %v", got, DeclaredMirroredFields) + } + for _, field := range jsonFields(reflect.TypeOf(catalogue.SettingDeclaration{})) { + if !slices.Contains(DeclaredMirroredFields, field) { + t.Errorf("a declaration has the field %q, which the declared-config mirror does not list: list it "+ + "in DeclaredMirrored (and say why it is no secret), or say here why it stays off the bus", field) + } + } + if got := jsonFields(reflect.TypeOf(DeclaredConfigMirrored{})); !slices.Equal(got, []string{"module", "settings"}) { + t.Errorf("the mirrored module carries %v", got) + } +} diff --git a/internal/recordmirror/mirror.go b/internal/recordmirror/mirror.go index ed9b7a48..3e6032e5 100644 --- a/internal/recordmirror/mirror.go +++ b/internal/recordmirror/mirror.go @@ -3,10 +3,14 @@ // controller's seat per write. // // **The store stays the record.** A mirror is built from what the store committed, so a change rolled back -// is never written. The writer keeps the last value it wrote for each key, filled from the bucket on its -// first pass, and compares byte for byte: a pass over unchanged records writes nothing and says nothing -// (ADR 0289 §1). Each write names the revision it last saw (compare-and-set), so a former lease holder that -// wrote in between is noticed, never overwritten blind. +// is never written. Every pass reads what the bucket holds and compares it byte for byte with what the store +// says: a pass over unchanged records writes nothing and says nothing (ADR 0289 §1), and a value somebody +// else wrote behind the writer's back (a former lease holder) differs from the store and is put right on +// the next pass. Each write names the revision it read (compare-and-set), so a write in between is noticed, +// never overwritten blind. +// +// **Only the lease holder writes** (ADR 0289 §9, design 54 §2): a pass begins by asking whether this +// controller holds the lease, and asks again before every write; without it, it writes nothing. // // **The bucket is the authority, the event a notice** (design 54 §3): the event is sent after the write // succeeded, so there is no event for a value the bucket does not hold. A refused event is tried once more @@ -83,6 +87,9 @@ type Announcer interface { // records do not pile up in a bucket that has no age. const MarkerTTL = 24 * time.Hour +// ErrNotHeld is a pass refused because this controller does not hold the lease: it wrote nothing. +var ErrNotHeld = errors.New("this controller does not hold the lease, so it writes no mirror") + // Writer writes one mirror. Not safe for use by two goroutines at once: one controller's one loop owns it. type Writer struct { Mirror Mirror @@ -90,14 +97,13 @@ type Writer struct { Announce Announcer // Epoch is the lease's epoch the events carry; nil carries none. Epoch func() uint64 - // Holds says this controller still holds the lease; nil is always. A write refused by compare-and-set - // is made again only while it does (design 54 §2). + // Holds says this controller holds the lease. Asked before every pass and every write; a writer + // without it writes nothing (design 54 §2). Required: nil holds nothing. Holds func() bool // Now is the clock the events are dated by; nil is time.Now. Now func() time.Time - known map[string]kept - filled bool + known map[string]kept // LostEvents counts the events refused twice since the writer was made. LostEvents int } @@ -119,10 +125,13 @@ type Result struct { // is refused alone, and the others are written; the error then names it. func (w *Writer) Sync(ctx context.Context, want map[string][]byte) (Result, error) { var r Result - if !w.filled { - if err := w.fill(ctx); err != nil { - return r, fmt.Errorf("reading the %s mirror: %w", w.Mirror.Name, err) - } + if !w.holds() { + return r, ErrNotHeld + } + // What the bucket holds now, every pass: a value written behind this writer's back differs from the + // store's and is written again below. + if err := w.fill(ctx); err != nil { + return r, fmt.Errorf("reading the %s mirror: %w", w.Mirror.Name, err) } var problems []error keys := make([]string, 0, len(want)) @@ -141,6 +150,9 @@ func (w *Writer) Sync(ctx context.Context, want map[string][]byte) (Result, erro problems = append(problems, err) continue } + if !w.holds() { + return r, errors.Join(append(problems, ErrNotHeld)...) + } revision, wrote, err := w.put(ctx, key, value, before, had) if err != nil { problems = append(problems, fmt.Errorf("writing %s: %w", key, err)) @@ -166,6 +178,9 @@ func (w *Writer) Sync(ctx context.Context, want map[string][]byte) (Result, erro sort.Strings(gone) for _, key := range gone { k := w.known[key] + if !w.holds() { + return r, errors.Join(append(problems, ErrNotHeld)...) + } err := w.KV.Purge(ctx, key, jetstream.LastRevision(k.revision), jetstream.PurgeTTL(MarkerTTL)) if refusedByRevision(err) { // Read it again: purged already, or moved by another writer. @@ -266,7 +281,6 @@ func (w *Writer) fill(ctx context.Context) error { return ctx.Err() case e := <-watch.Updates(): if e == nil { - w.filled = true return nil } w.known[e.Key()] = kept{value: append([]byte(nil), e.Value()...), revision: e.Revision()} @@ -274,7 +288,7 @@ func (w *Writer) fill(ctx context.Context) error { } } -func (w *Writer) holds() bool { return w.Holds == nil || w.Holds() } +func (w *Writer) holds() bool { return w.Holds != nil && w.Holds() } func (w *Writer) now() time.Time { if w.Now == nil { diff --git a/internal/recordmirror/mirror_store_test.go b/internal/recordmirror/mirror_store_test.go index c0601a7c..11e8de47 100644 --- a/internal/recordmirror/mirror_store_test.go +++ b/internal/recordmirror/mirror_store_test.go @@ -140,7 +140,7 @@ func TestNatsAnAssignmentAndARegistrationEachChangeOneKeyAndRaiseOneEvent(t *tes if err != nil { t.Fatal(err) } - if want := `{"module":"ui","settings":{"font-size":{"default":16,"kind":"preference","why":"readable on the desktop"}}}`; string(got.Value()) != want { + if want := `{"module":"ui","settings":{"font-size":{"kind":"preference","default":16,"why":"readable on the desktop"}}}`; string(got.Value()) != want { t.Errorf("the declaration is mirrored as %s, want %s", got.Value(), want) } diff --git a/internal/recordmirror/mirror_test.go b/internal/recordmirror/mirror_test.go index 41f0c2ac..31f67594 100644 --- a/internal/recordmirror/mirror_test.go +++ b/internal/recordmirror/mirror_test.go @@ -109,7 +109,7 @@ func aWriter(t *testing.T, js *broker.JetStream, api jetstream.JetStream, m Mirr t.Fatal(err) } return &Writer{Mirror: m, KV: kv, Announce: link.OverNATS{Conn: js.Conn(), JS: js.Context()}, - Epoch: func() uint64 { return 7 }} + Epoch: func() uint64 { return 7 }, Holds: func() bool { return true }} } func TestNatsAMirrorChangesOneKeyAndRaisesOneEventPerChange(t *testing.T) { @@ -242,6 +242,76 @@ func TestNatsAWriteRefusedByCompareAndSetReadsAgain(t *testing.T) { } } +// **A value written behind the writer's back is put right on the next pass** (ADR 0289 §10): a former lease +// holder's write differs from the store, and the store's value is written again, said as one change. +func TestNatsAForeignWriteIsCorrectedOnTheNextPass(t *testing.T) { + js, api := aMirrorBus(t) + ctx := t.Context() + events := listen(t, js.Conn(), "mesh.seat."+broker.ControllerSeat+".event."+Assignments.Event) + w := aWriter(t, js, api, Assignments) + store := map[string][]byte{"anchor.gitea": []byte(`{"a":1}`)} + if _, err := w.Sync(ctx, store); err != nil { + t.Fatal(err) + } + heardBefore := len(settle(events, 1)) + if _, err := w.KV.Put(ctx, "anchor.gitea", []byte(`{"a":9}`)); err != nil { + t.Fatal(err) + } + r, err := w.Sync(ctx, store) + if err != nil { + t.Fatal(err) + } + if r != (Result{Written: 1}) { + t.Errorf("a pass after a foreign write did %+v", r) + } + e, err := w.KV.Get(ctx, "anchor.gitea") + if err != nil || string(e.Value()) != `{"a":1}` { + t.Fatalf("the bucket still holds %v, %v", e, err) + } + if said := settle(events, heardBefore+1)[heardBefore:]; len(said) != 1 || said[0].Change != Changed { + t.Errorf("the correction was said as %+v", said) + } + // And put right, it stays: the next pass writes nothing. + if r, err := w.Sync(ctx, store); err != nil || r != (Result{}) { + t.Errorf("a pass after the correction did %+v, %v", r, err) + } +} + +// **A writer without the lease writes nothing** (ADR 0289 §9, design 54 §2): not a new key, not a change, not +// a removal, and it says so. +func TestNatsAWriterWithoutTheLeaseWritesNothing(t *testing.T) { + js, api := aMirrorBus(t) + ctx := t.Context() + held := aWriter(t, js, api, Assignments) + if _, err := held.Sync(ctx, map[string][]byte{"anchor.gitea": []byte(`{"a":1}`)}); err != nil { + t.Fatal(err) + } + before := writes(t, api, Assignments.Bucket) + for _, holds := range []func() bool{nil, func() bool { return false }} { + w := aWriter(t, js, api, Assignments) + w.Holds = holds + r, err := w.Sync(ctx, map[string][]byte{"anchor.gitea": []byte(`{"a":2}`), "anchor.forgejo": []byte(`{"a":1}`)}) + if !errors.Is(err, ErrNotHeld) || r != (Result{}) { + t.Errorf("a writer without the lease did %+v, %v", r, err) + } + r, err = w.Sync(ctx, map[string][]byte{}) + if !errors.Is(err, ErrNotHeld) || r != (Result{}) { + t.Errorf("a writer without the lease removed: %+v, %v", r, err) + } + } + // And one that loses it mid-pass stops before its next write. + asked := 0 + w := aWriter(t, js, api, Assignments) + w.Holds = func() bool { asked++; return asked <= 2 } + r, err := w.Sync(ctx, map[string][]byte{"anchor.a": []byte(`{}`), "anchor.b": []byte(`{}`), "anchor.gitea": []byte(`{"a":1}`)}) + if !errors.Is(err, ErrNotHeld) || r.Written != 1 { + t.Errorf("a writer that lost the lease mid-pass did %+v, %v", r, err) + } + if got := writes(t, api, Assignments.Bucket) - before; got != 1 { + t.Errorf("writers without the lease wrote the bucket %d times", got) + } +} + // **A key the bus cannot hold is refused alone**, and the others are written. func TestAKeyTheBusCannotHoldIsRefusedAlone(t *testing.T) { if validKey("anchor.gitea") != nil || validKey("bad key") == nil || validKey(".lead") == nil || -- 2.54.0 From 35c7662f895f57b0092939f67e6a48ac88386ed8 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 01:26:29 +0000 Subject: [PATCH 7/8] Check the mirrors as ADR 0356 says, and prove a removal asks for the lease Reviews 251 of mesh-controller#315 and ADR 0356 v4 (hq issue 652): a removal without the lease removes nothing; a module no longer registered loses its declared-config key with its event; a listed mirror is never reported as undeclared state; only the audit and a mirror's own writer name a mirror's bucket; and the mirrors' list and the writers table agree. The store test's line moves where it merges with the integration branch's own new lines. Co-Authored-By: Claude Opus 5.5 --- internal/broker/controller_buckets_test.go | 53 ++++++++++++++ internal/recordmirror/mirror_store_test.go | 20 ++++++ internal/recordmirror/mirror_test.go | 25 +++++++ internal/recordmirror/readers_test.go | 82 ++++++++++++++++++++++ testdata/store-tests | 2 +- 5 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 internal/recordmirror/readers_test.go diff --git a/internal/broker/controller_buckets_test.go b/internal/broker/controller_buckets_test.go index 1e8c7a69..194227e2 100644 --- a/internal/broker/controller_buckets_test.go +++ b/internal/broker/controller_buckets_test.go @@ -125,3 +125,56 @@ func TestWhatTheControllerAskedIsBounded(t *testing.T) { t.Errorf("history %d, age %s, bytes %d", status.History(), status.TTL(), info.Config.MaxBytes) } } + +// **A listed mirror is no undeclared bucket** (ADR 0356 §3, ADR 0201 §16): the controller's list of its mirrors +// stands for their declaration, so the report of state nothing declares never names one. +func TestAListedMirrorIsNotReportedAsUndeclared(t *testing.T) { + names := []string{"gone_state"} + for _, m := range ControllerMirrors() { + names = append(names, BucketName(ControllerSeat, m)) + } + undeclared, err := RaiseBuckets(fakeBuckets{names: names}, nil) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(undeclared, []string{"gone_state"}) { + t.Fatalf("the report of undeclared state names %v", undeclared) + } +} + +// **The mirrors' list and the writers table agree** (ADR 0356 §3, to-be 45 §1): every listed mirror is a row of +// the table whose one writer is the controller, on its bucket alone; and every row that writes a bucket of the +// controller's named as a mirror is on the list. +func TestTheMirrorsListAndTheWritersTableAgree(t *testing.T) { + controller := Principal{Kind: KindController} + module := Principal{Kind: KindModule, Module: ControllerSeat, Node: "one"} + rowed := map[string]bool{} + for _, row := range WritersTable { + if !strings.HasSuffix(row.State, " mirror") { + continue + } + for _, subject := range row.Subjects { + bucket := strings.TrimSuffix(strings.TrimPrefix(subject, "$KV."), ".>") + rowed[bucket] = true + if row.Writes == nil || !row.Writes(controller, subject) || row.Writes(module, subject) { + t.Errorf("%q is not written by the controller alone", row.State) + } + } + } + listed := map[string]bool{} + for _, m := range ControllerMirrors() { + bucket := BucketName(ControllerSeat, m) + listed[bucket] = true + if !rowed[bucket] { + t.Errorf("the mirror %s is listed and no row of the writers table writes it", bucket) + } + if !IsControllerBucket(bucket) || !slices.Contains(ControllerBuckets(), bucket) { + t.Errorf("the mirror %s is listed and not asserted as the controller's own", bucket) + } + } + for bucket := range rowed { + if !listed[bucket] { + t.Errorf("the writers table writes the mirror %s, which the list does not name", bucket) + } + } +} diff --git a/internal/recordmirror/mirror_store_test.go b/internal/recordmirror/mirror_store_test.go index 11e8de47..391f6b93 100644 --- a/internal/recordmirror/mirror_store_test.go +++ b/internal/recordmirror/mirror_store_test.go @@ -155,4 +155,24 @@ func TestNatsAnAssignmentAndARegistrationEachChangeOneKeyAndRaiseOneEvent(t *tes if len(said) != 1 || said[0].Key != "anchor.ui" || said[0].Change != Removed { t.Errorf("an unassignment raised %+v", said) } + + // **A module no longer registered** has its key removed, said once as `removed` (ADR 0356 §4). + if err := inv.ForgetModule(ctx, "notes"); err != nil { + t.Fatal(err) + } + declaredWrites = writes(t, api, DeclaredConfig.Bucket) + declaredSaid = len(heardDeclared.all()) + if _, rd := pass(); rd != (Result{Removed: 1}) { + t.Errorf("a module forgotten did %+v", rd) + } + if got := writes(t, api, DeclaredConfig.Bucket) - declaredWrites; got != 1 { + t.Errorf("a module forgotten changed %d keys of the declared config", got) + } + if _, err := declared.KV.Get(ctx, "notes"); err == nil { + t.Error("a module forgotten keeps its key") + } + said = settle(heardDeclared, declaredSaid+1)[declaredSaid:] + if len(said) != 1 || said[0].Key != "notes" || said[0].Change != Removed { + t.Errorf("a module forgotten raised %+v", said) + } } diff --git a/internal/recordmirror/mirror_test.go b/internal/recordmirror/mirror_test.go index 31f67594..9d987e37 100644 --- a/internal/recordmirror/mirror_test.go +++ b/internal/recordmirror/mirror_test.go @@ -312,6 +312,31 @@ func TestNatsAWriterWithoutTheLeaseWritesNothing(t *testing.T) { } } +// **A removal asks for the lease too** (ADR 0289 §9): a writer that held the lease when its pass began and lost +// it before its first removal purges nothing, and says it does not hold the lease. +func TestNatsARemovalWithoutTheLeaseRemovesNothing(t *testing.T) { + js, api := aMirrorBus(t) + ctx := t.Context() + held := aWriter(t, js, api, Assignments) + if _, err := held.Sync(ctx, map[string][]byte{"anchor.gitea": []byte(`{"a":1}`)}); err != nil { + t.Fatal(err) + } + before := writes(t, api, Assignments.Bucket) + asked := 0 + w := aWriter(t, js, api, Assignments) + w.Holds = func() bool { asked++; return asked == 1 } + r, err := w.Sync(ctx, map[string][]byte{}) + if !errors.Is(err, ErrNotHeld) || r != (Result{}) || asked != 2 { + t.Errorf("a removal without the lease did %+v, %v (the lease asked %d times)", r, err, asked) + } + if got := writes(t, api, Assignments.Bucket) - before; got != 0 { + t.Errorf("a removal without the lease wrote the bucket %d times", got) + } + if _, err := w.KV.Get(ctx, "anchor.gitea"); err != nil { + t.Errorf("the key was removed without the lease: %v", err) + } +} + // **A key the bus cannot hold is refused alone**, and the others are written. func TestAKeyTheBusCannotHoldIsRefusedAlone(t *testing.T) { if validKey("anchor.gitea") != nil || validKey("bad key") == nil || validKey(".lead") == nil || diff --git a/internal/recordmirror/readers_test.go b/internal/recordmirror/readers_test.go new file mode 100644 index 00000000..c57a7b4c --- /dev/null +++ b/internal/recordmirror/readers_test.go @@ -0,0 +1,82 @@ +package recordmirror + +import ( + "go/ast" + "go/parser" + "go/token" + "io/fs" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/broker" +) + +// mayNameAMirror are the files of the controller that may name a record mirror's bucket outside their tests: +// where the buckets are defined, asserted and granted (internal/broker), the list a reader names in `reads` +// (internal/catalogue/state.go), and each mirror's own writer (this package and the loop that runs it). The +// audit of a mirror against its store (ADR 0289 §4, issue 419) joins this list when it is built. +var mayNameAMirror = []string{ + "cmd/mesh-controller/record_mirrors.go", + "internal/broker/controller_buckets.go", + "internal/broker/writers.go", + "internal/catalogue/state.go", +} + +// **Nothing in the controller reads a mirror to decide anything** (ADR 0289 §19 as ADR 0356 §2 changes it): no +// file of the controller's packages outside its tests names a mirror's bucket, by its Go name or its bus name, +// but the audit and a mirror's own writer. A reader elsewhere fails here, naming the file and what it named. +func TestOnlyTheAuditAndAMirrorsWriterReadAMirror(t *testing.T) { + names := []string{"AssignmentsBucket", "DeclaredConfigBucket", "MirrorAssignments", "MirrorDeclaredConfig", + "ControllerMirrors", "Assignments", "DeclaredConfig", "Mirrors"} + var buckets []string + for _, m := range broker.ControllerMirrors() { + buckets = append(buckets, broker.BucketName(broker.ControllerSeat, m)) + } + root := filepath.Join("..", "..") + var found []string + for _, dir := range []string{"cmd", "internal"} { + err := filepath.WalkDir(filepath.Join(root, dir), func(path string, e fs.DirEntry, err error) error { + if err != nil || e.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return err + } + rel, err := filepath.Rel(root, path) + if err != nil { + return err + } + rel = filepath.ToSlash(rel) + if strings.HasPrefix(rel, "internal/recordmirror/") || slices.Contains(mayNameAMirror, rel) { + return nil + } + f, err := parser.ParseFile(token.NewFileSet(), path, nil, 0) + if err != nil { + return err + } + ast.Inspect(f, func(n ast.Node) bool { + switch n := n.(type) { + case *ast.SelectorExpr: + pkg, ok := n.X.(*ast.Ident) + if ok && (pkg.Name == "broker" || pkg.Name == "recordmirror") && slices.Contains(names, n.Sel.Name) { + found = append(found, rel+": "+pkg.Name+"."+n.Sel.Name) + } + case *ast.BasicLit: + for _, b := range buckets { + if n.Kind == token.STRING && strings.Contains(n.Value, b) { + found = append(found, rel+": "+n.Value) + } + } + } + return true + }) + return nil + }) + if err != nil { + t.Fatal(err) + } + } + if len(found) > 0 { + t.Fatalf("a package of the controller reads a record mirror, which only the audit and the mirror's "+ + "writer may (ADR 0289 §19, ADR 0356 §2):\n %s", strings.Join(found, "\n ")) + } +} diff --git a/testdata/store-tests b/testdata/store-tests index b7dc4922..2213aab7 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -49,6 +49,7 @@ ./internal/inventory TestACredentialIssuedOutsideIsNeverMade ./internal/inventory TestOneAcceptForAProviderIsSealedToEveryConsumer ./internal/inventory TestASharedOwnSecretIssuedOutsideIsNeverMade +./internal/recordmirror TestNatsAnAssignmentAndARegistrationEachChangeOneKeyAndRaiseOneEvent ./cmd/mesh-controller TestOneAcceptForAProviderIsPlacedForEveryConsumer ./cmd/mesh-controller TestReplay164 ./cmd/mesh-controller TestAnApprovedLayerOpensTheWalkThatSendsIt @@ -61,4 +62,3 @@ ./cmd/mesh-controller TestAProviderElsewhereServesItsOwnComposedName ./cmd/mesh-controller TestAReusedCheckBuildIsRegisteredByItsDigestAndGatedAsABuiltOne ./internal/inventory TestAReportOfABusyContainerIsKeptAndReadBack -./internal/recordmirror TestNatsAnAssignmentAndARegistrationEachChangeOneKeyAndRaiseOneEvent -- 2.54.0 From 7a691edd3a6ce14ba7b33bb4cfaea186d1f26b91 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 01:40:33 +0000 Subject: [PATCH 8/8] Find a bare or aliased mirror reader, and say what the reader test misses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review 257 of mesh-controller#315 (hq issue 652): the test that only the audit and a mirror's writer read a mirror (ADR 0289 §19, ADR 0356 §2) matched only names qualified as broker. or recordmirror., so a reader inside package broker, or one importing broker under another name, passed. It now matches by import path and bare names in the defining packages, proves it on planted readers, and names what it cannot see. Co-Authored-By: Claude Opus 5.5 --- internal/recordmirror/readers_test.go | 117 ++++++++++++++++++++++++-- 1 file changed, 108 insertions(+), 9 deletions(-) diff --git a/internal/recordmirror/readers_test.go b/internal/recordmirror/readers_test.go index c57a7b4c..26f2e806 100644 --- a/internal/recordmirror/readers_test.go +++ b/internal/recordmirror/readers_test.go @@ -5,8 +5,10 @@ import ( "go/parser" "go/token" "io/fs" + "os" "path/filepath" "slices" + "strconv" "strings" "testing" @@ -24,17 +26,87 @@ var mayNameAMirror = []string{ "internal/catalogue/state.go", } +// The packages whose names stand for a mirror, by import path, and the names in each. +const ( + brokerPath = "github.com/novox/mesh-controller/internal/broker" + recordmirrorPath = "github.com/novox/mesh-controller/internal/recordmirror" +) + +var mirrorNames = map[string][]string{ + brokerPath: {"AssignmentsBucket", "DeclaredConfigBucket", "MirrorAssignments", "MirrorDeclaredConfig", + "ControllerMirrors"}, + recordmirrorPath: {"Assignments", "DeclaredConfig", "Mirrors"}, +} + // **Nothing in the controller reads a mirror to decide anything** (ADR 0289 §19 as ADR 0356 §2 changes it): no -// file of the controller's packages outside its tests names a mirror's bucket, by its Go name or its bus name, -// but the audit and a mirror's own writer. A reader elsewhere fails here, naming the file and what it named. +// file of the controller's packages outside its tests names a mirror's bucket but the audit and a mirror's own +// writer. A reader elsewhere fails here, naming the file and what it named. +// +// **What it catches**: a name that stands for a mirror, qualified by the package it comes from under any +// local name the file imports it as (`broker.AssignmentsBucket`, `b.AssignmentsBucket`); the same names bare +// in a file of package broker or package recordmirror; and a string literal holding a mirror's bucket name. +// **What it does not catch**: a bucket name built at run time, such as `BucketName(ControllerSeat, +// "assignments")` or literals concatenated into one, and a mirror reached through a value passed in from an +// allowed file. Those are deliberate evasions rather than ordinary code; a reviewer judges them. func TestOnlyTheAuditAndAMirrorsWriterReadAMirror(t *testing.T) { - names := []string{"AssignmentsBucket", "DeclaredConfigBucket", "MirrorAssignments", "MirrorDeclaredConfig", - "ControllerMirrors", "Assignments", "DeclaredConfig", "Mirrors"} var buckets []string for _, m := range broker.ControllerMirrors() { buckets = append(buckets, broker.BucketName(broker.ControllerSeat, m)) } root := filepath.Join("..", "..") + found := mirrorReaders(t, root, buckets) + if len(found) > 0 { + t.Fatalf("a package of the controller reads a record mirror, which only the audit and the mirror's "+ + "writer may (ADR 0289 §19, ADR 0356 §2):\n %s", strings.Join(found, "\n ")) + } +} + +// **The reader test sees what it says it sees**: in a planted tree, each planted reader is named, and an allowed +// file and a selector on a value are not. +func TestTheReaderTestFindsPlantedReaders(t *testing.T) { + root := t.TempDir() + plant := func(path, src string) { + full := filepath.Join(root, path) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(src), 0o644); err != nil { + t.Fatal(err) + } + } + plant("cmd/mesh-controller/qualified.go", "package main\n\nimport \""+brokerPath+"\"\n\nvar a = broker.AssignmentsBucket\n") + plant("internal/x/aliased.go", "package x\n\nimport b \""+brokerPath+"\"\n\nvar a = b.DeclaredConfigBucket\n") + plant("internal/broker/zz_reader.go", "package broker\n\nfunc zzR() string { return AssignmentsBucket }\n") + plant("internal/inventory/literal.go", "package inventory\n\nvar a = \"mesh-controller_declared-config\"\n") + plant("internal/y/writer.go", "package y\n\nimport \""+recordmirrorPath+"\"\n\nvar a = recordmirror.Assignments\n") + // Allowed: a mirror's definition and its writer. + plant("internal/broker/controller_buckets.go", "package broker\n\nvar AssignmentsBucket = \"x\"\n") + plant("internal/broker/other.go", "package broker\n\nfunc other() string { return \"nothing\" }\n") + // A selector on a value (v.AssignmentsBucket) is no use of the mirror, but declaring a field of a mirror's + // name in package broker is named, on the safe side; and a bare use inside a call under a selector is found. + plant("internal/broker/field.go", "package broker\n\ntype s struct{ AssignmentsBucket string }\n\nfunc f(v s) string { return v.AssignmentsBucket }\n") + plant("internal/broker/nested.go", "package broker\n\nfunc g() int { return len(h(DeclaredConfigBucket).x) }\n") + found := mirrorReaders(t, root, []string{"mesh-controller_assignments", "mesh-controller_declared-config"}) + want := []string{ + "cmd/mesh-controller/qualified.go: broker.AssignmentsBucket", + "internal/broker/field.go: AssignmentsBucket", + "internal/broker/nested.go: DeclaredConfigBucket", + "internal/broker/zz_reader.go: AssignmentsBucket", + "internal/inventory/literal.go: \"mesh-controller_declared-config\"", + "internal/x/aliased.go: b.DeclaredConfigBucket", + "internal/y/writer.go: recordmirror.Assignments", + } + slices.Sort(found) + if !slices.Equal(found, want) { + t.Errorf("the planted readers were found as\n %s\nwant\n %s", strings.Join(found, "\n "), + strings.Join(want, "\n ")) + } +} + +// mirrorReaders is every place under root's cmd and internal, outside tests and the allowed files, that names a +// mirror. +func mirrorReaders(t *testing.T, root string, buckets []string) []string { + t.Helper() var found []string for _, dir := range []string{"cmd", "internal"} { err := filepath.WalkDir(filepath.Join(root, dir), func(path string, e fs.DirEntry, err error) error { @@ -53,13 +125,43 @@ func TestOnlyTheAuditAndAMirrorsWriterReadAMirror(t *testing.T) { if err != nil { return err } + // Each import's local name, mapped to the package it stands for. + imported := map[string]string{} + for _, imp := range f.Imports { + p, _ := strconv.Unquote(imp.Path.Value) + if _, ok := mirrorNames[p]; !ok { + continue + } + local := filepath.Base(p) + if imp.Name != nil { + local = imp.Name.Name + } + imported[local] = p + } + // A file of the package that defines the names may use them bare. + var bare []string + switch f.Name.Name { + case "broker": + bare = mirrorNames[brokerPath] + case "recordmirror": + bare = mirrorNames[recordmirrorPath] + } ast.Inspect(f, func(n ast.Node) bool { switch n := n.(type) { case *ast.SelectorExpr: pkg, ok := n.X.(*ast.Ident) - if ok && (pkg.Name == "broker" || pkg.Name == "recordmirror") && slices.Contains(names, n.Sel.Name) { + if !ok { + return true + } + if p, imp := imported[pkg.Name]; imp && slices.Contains(mirrorNames[p], n.Sel.Name) { found = append(found, rel+": "+pkg.Name+"."+n.Sel.Name) } + // Neither a selector's own name nor its package is a bare use. + return false + case *ast.Ident: + if slices.Contains(bare, n.Name) { + found = append(found, rel+": "+n.Name) + } case *ast.BasicLit: for _, b := range buckets { if n.Kind == token.STRING && strings.Contains(n.Value, b) { @@ -75,8 +177,5 @@ func TestOnlyTheAuditAndAMirrorsWriterReadAMirror(t *testing.T) { t.Fatal(err) } } - if len(found) > 0 { - t.Fatalf("a package of the controller reads a record mirror, which only the audit and the mirror's "+ - "writer may (ADR 0289 §19, ADR 0356 §2):\n %s", strings.Join(found, "\n ")) - } + return found } -- 2.54.0