From 7530c2aeaccd793342524776bfee4b3fa2e54bbf Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 00:41:06 +0000 Subject: [PATCH 1/6] Read placement from a layer's Placement, never from its config values (issue 651) ADR 0348 clause 11: places, accesses and kept networks are placement, not config. Today's mergeable files still take them as said, so nothing composes differently. Co-Authored-By: Claude Opus 5.5 --- internal/catalogue/placement.go | 67 +++++++++++++++++++++++++- internal/catalogue/setting_defaults.go | 3 +- internal/catalogue/settings.go | 16 ++++-- 3 files changed, 80 insertions(+), 6 deletions(-) diff --git a/internal/catalogue/placement.go b/internal/catalogue/placement.go index 400c5b3f..bc8bf03d 100644 --- a/internal/catalogue/placement.go +++ b/internal/catalogue/placement.go @@ -34,6 +34,69 @@ const PlacesSetting = "places" // AccessesSetting is the settings key that says where a module's accesses are on this node, by id. const AccessesSetting = "accesses" +// **Placement is not config** (novox/hq ADR 0348 clause 11, issue 651). `places`, `accesses` and +// `networks` are facts of an assignment the controller decides, held beside the assignment and apart +// from its settings: a Layer carries them in Placement, never in Values, and only Places, AccessPlaces +// and KeptNetworks read them. A person still says them as keys of a layer through `settings`; LayerOf +// splits such a layer, so what they type is routed to the placement and nothing they do changes. + +// placementKeys are the keys of a layer that are placement, not config. +var placementKeys = map[string]bool{PlacesSetting: true, AccessesSetting: true, NetworksSetting: true} + +// SplitPlacement divides a layer as a person says it into its config values and its placement. Each is +// nil when it holds nothing; the values themselves are not copied. +func SplitPlacement(values map[string]any) (config, placement map[string]any) { + for key, value := range values { + if placementKeys[key] { + if placement == nil { + placement = map[string]any{} + } + placement[key] = value + continue + } + if config == nil { + config = map[string]any{} + } + config[key] = value + } + return config, placement +} + +// LayerOf is a layer as a person says it — config values and placement in one map — split into the two. +func LayerOf(from string, values map[string]any) Layer { + config, placement := SplitPlacement(values) + if config == nil { + config = map[string]any{} + } + return Layer{From: from, Values: config, Placement: placement} +} + +// Whole is the layer as a person says it: its values and its placement in one map, a new map. +func (l Layer) Whole() map[string]any { + out := make(map[string]any, len(l.Values)+len(l.Placement)) + for k, v := range l.Values { + out[k] = v + } + for k, v := range l.Placement { + out[k] = v + } + return out +} + +// asSaid is the layers with placement folded back into the values, for what reads a layer whole as it +// did while placement was kept in it: today's mergeable file, which takes every key (ADR 0326 §3), and +// the judgement that no value holds a line end. +func asSaid(layers []Layer) []Layer { + out := make([]Layer, len(layers)) + for i, l := range layers { + out[i] = l + if len(l.Placement) > 0 { + out[i] = Layer{From: l.From, Values: l.Whole(), Default: l.Default} + } + } + return out +} + // Placement is what an assignment says about one of a module's directories. type Placement struct { // Path is where the directory is on this machine. Absolute. @@ -208,7 +271,7 @@ func Places(m Manifest, layers []Layer) (map[string]Placement, error) { } out := map[string]Placement{} for _, layer := range layers { - raw, ok := layer.Values[PlacesSetting] + raw, ok := layer.Placement[PlacesSetting] if !ok { continue } @@ -278,7 +341,7 @@ func AccessPlaces(m Manifest, layers []Layer) (map[string]string, error) { } out := map[string]string{} for _, layer := range layers { - raw, ok := layer.Values[AccessesSetting] + raw, ok := layer.Placement[AccessesSetting] if !ok { continue } diff --git a/internal/catalogue/setting_defaults.go b/internal/catalogue/setting_defaults.go index 1bc72f67..3f158e02 100644 --- a/internal/catalogue/setting_defaults.go +++ b/internal/catalogue/setting_defaults.go @@ -207,7 +207,8 @@ func Effective(m Manifest, layers []Layer) []SettingSource { Default: d.Default, HasDefault: true} } for _, layer := range layers { - for key, v := range layer.Values { + // As the layer is said, placement too: `settings` answers it as a person set it. + for key, v := range layer.Whole() { s, ok := byKey[key] if !ok { s = &SettingSource{Key: key} diff --git a/internal/catalogue/settings.go b/internal/catalogue/settings.go index e20b61a2..45c992e8 100644 --- a/internal/catalogue/settings.go +++ b/internal/catalogue/settings.go @@ -37,6 +37,11 @@ type Layer struct { // Default marks the layer a module's own defaults make (novox/hq ADR 0262). Marked rather than // recognised by From, which is a node's name for a node's layer, and a node may be called anything. Default bool + // Placement is the assignment's placement at this layer's scope — `places`, `accesses`, `networks` + // — which the controller holds apart from settings (novox/hq ADR 0348 clause 11, issue 651): read + // by Places, AccessPlaces and KeptNetworks, and never from Values. LayerOf splits a layer as a person + // says it into the two. + Placement map[string]any } // ApplySettings produces a resource's final content from the module's own and the layers over it. @@ -45,6 +50,10 @@ type Layer struct { // module first, then this node's. A node that differs from the rest is expressed by differing, // rather than by repeating everything the rest already say. func ApplySettings(resource map[string]any, layers []Layer) (map[string]any, error) { + // Today's form reaches a mergeable file with every key of the layer as it was said, placement too + // (novox/hq ADR 0326 §3, ADR 0348 clause 11: a module not yet moved keeps it), so a file composes + // byte for byte as it did while placement was kept in the layer. + layers = asSaid(layers) how, _ := resource["merge"].(string) taken, names := takenKeys(resource) if how == "" { @@ -395,7 +404,7 @@ func UnusedSettings(m Manifest, layers []Layer) []string { } var unused []string for _, layer := range layers { - for key := range layer.Values { + for key := range layer.Whole() { if lands[key] { continue } @@ -483,7 +492,7 @@ func KeptNetworks(m Manifest, layers []Layer, adopted bool) (map[string][]string } out := map[string][]string{} for _, layer := range layers { - raw, ok := layer.Values[NetworksSetting] + raw, ok := layer.Placement[NetworksSetting] if !ok { continue } @@ -543,7 +552,8 @@ var networkName = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_.-]*$`) // refused where it is stored (SetSettings, with UnusedSettings) and said where a plan is read, // and never costs a module its place. func JudgeSettings(m Manifest, layers []Layer, adopted bool) error { - if err := settingsHoldOneLine(m.Module, layers); err != nil { + // Placement is judged for line ends as it always was: a path is written into files as well. + if err := settingsHoldOneLine(m.Module, asSaid(layers)); err != nil { return err } // Which keys each mergeable file takes is the definition's to say right (novox/hq ADR 0326); -- 2.54.0 From 02b542a0592eaadbeb3c123858d1846dc692c744 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 00:42:37 +0000 Subject: [PATCH 2/6] Keep a layer's placement beside it, and say the layer whole (issue 651) The settings verb's path splits a layer it is given into its config values and its placement, so what a person types still works; a layer reads back and goes into the history whole. Co-Authored-By: Claude Opus 5.5 --- internal/inventory/catalogue.go | 97 ++++++++++++++++++++++++++------- internal/inventory/unseen.go | 28 +++++++--- 2 files changed, 99 insertions(+), 26 deletions(-) diff --git a/internal/inventory/catalogue.go b/internal/inventory/catalogue.go index 6ce3740c..31cc597f 100644 --- a/internal/inventory/catalogue.go +++ b/internal/inventory/catalogue.go @@ -839,7 +839,18 @@ func (i *Inventory) KeepSettings(ctx context.Context, nodeName, module string, v } func (i *Inventory) setSettings(ctx context.Context, nodeName, module string, values map[string]any, judge bool, setBy string) error { - raw, err := json.Marshal(values) + // **Placement is not config** (novox/hq ADR 0348 clause 11, issue 651): the layer is said whole, as + // it always was, and its placement keys are kept beside the assignment, never in the settings row. + // Both are written in one transaction, and the layer a set replaces is the whole of both. + config, placement := catalogue.SplitPlacement(values) + if config == nil { + config = map[string]any{} + } + raw, err := json.Marshal(config) + if err != nil { + return err + } + placed, err := placementJSON(placement) if err != nil { return err } @@ -881,6 +892,9 @@ func (i *Inventory) setSettings(ctx context.Context, nodeName, module string, va do update set values = excluded.values, set_at = now(), set_by = excluded.set_by`, module, raw, nullable(setBy)); err != nil { return wrapModule(err, module) } + if err := keepPlacement(ctx, tx, nil, module, placed, setBy); err != nil { + return err + } return tx.Commit(ctx) } node, err := i.NodeByName(ctx, nodeName) @@ -912,6 +926,9 @@ func (i *Inventory) setSettings(ctx context.Context, nodeName, module string, va if err != nil { return wrapModule(err, module) } + if err := keepPlacement(ctx, tx, &node.ID, module, placed, setBy); err != nil { + return err + } // A given port replaces what the mesh assigned for that port: the assignment is given back, // so the number is free for the next module rather than held for ever in the name of a port // that now lives elsewhere. @@ -942,21 +959,15 @@ func (i *Inventory) judgeSettings(ctx context.Context, nodeName, module string, return err } where, from, adopted = nodeName, nodeName, node.Adopted - var meshWide []byte - err = i.store.Pool().QueryRow(ctx, - `select values from settings where module = $1 and node is null`, module).Scan(&meshWide) - if err != nil && !errors.Is(err, pgx.ErrNoRows) { + under, has, err := i.Layer(ctx, "", module) + if err != nil { return err } - if len(meshWide) > 0 { - var under map[string]any - if err := json.Unmarshal(meshWide, &under); err != nil { - return err - } - layers = append(layers, catalogue.Layer{From: catalogue.MeshWideLayer, Values: under}) + if has { + layers = append(layers, catalogue.LayerOf(catalogue.MeshWideLayer, under)) } } - layers = append(layers, catalogue.Layer{From: from, Values: values}) + layers = append(layers, catalogue.LayerOf(from, values)) if err := catalogue.JudgeSettings(m, layers, adopted); err != nil { return fmt.Errorf("refused: %s on %s cannot compose with the layer %q — %w", module, where, from, err) } @@ -1089,7 +1100,8 @@ func refuseGivenCollisions(ctx context.Context, tx pgx.Tx, nodeID any, node, mod } func wrapModule(err error, module string) error { - if err != nil && strings.Contains(err.Error(), "settings_module_fkey") { + if err != nil && (strings.Contains(err.Error(), "settings_module_fkey") || + strings.Contains(err.Error(), "placement_module_fkey")) { return fmt.Errorf("%w: %s", ErrNoSuchModule, module) } return err @@ -1130,6 +1142,11 @@ func (i *Inventory) ClearSettingsBy(ctx context.Context, nodeName, module, clear `delete from settings where module = $1 and node is not distinct from $2::uuid`, module, nodeID); err != nil { return err } + // And its placement, which is the same layer as a person says it (novox/hq ADR 0348 clause 11). + if _, err := tx.Exec(ctx, + `delete from placement where module = $1 and node is not distinct from $2::uuid`, module, nodeID); err != nil { + return err + } return tx.Commit(ctx) } @@ -1142,10 +1159,13 @@ func (i *Inventory) SettingsFor(ctx context.Context, nodeName, module string) ([ if err != nil { return nil, err } + // Each layer's placement beside it, from the placement the controller holds (novox/hq ADR 0348 + // clause 11): a layer's Values hold its config values only, its Placement where things are. rows, err := i.store.Pool().Query(ctx, - `select node is null, values from settings - where module = $1 and (node is null or node = $2) - order by node is null desc`, module, node.ID) + `select s.node is null, s.values, p.values from settings s + left join placement p on p.module = s.module and p.node is not distinct from s.node + where s.module = $1 and (s.node is null or s.node = $2) + order by s.node is null desc`, module, node.ID) if err != nil { return nil, err } @@ -1154,23 +1174,62 @@ func (i *Inventory) SettingsFor(ctx context.Context, nodeName, module string) ([ var layers []catalogue.Layer for rows.Next() { var meshWide bool - var raw []byte - if err := rows.Scan(&meshWide, &raw); err != nil { + var raw, placed []byte + if err := rows.Scan(&meshWide, &raw, &placed); err != nil { return nil, err } values := map[string]any{} if err := json.Unmarshal(raw, &values); err != nil { return nil, err } + var placement map[string]any + if len(placed) > 0 { + if err := json.Unmarshal(placed, &placement); err != nil { + return nil, err + } + } from := nodeName if meshWide { from = catalogue.MeshWideLayer } - layers = append(layers, catalogue.Layer{From: from, Values: values}) + layers = append(layers, catalogue.Layer{From: from, Values: values, Placement: placement}) } return layers, rows.Err() } +// placementJSON is a layer's placement as the placement table keeps it, or nil when it has none. +func placementJSON(placement map[string]any) ([]byte, error) { + if len(placement) == 0 { + return nil, nil + } + return json.Marshal(placement) +} + +// keepPlacement writes one layer's placement beside its settings row, in the write's transaction: the +// placement the layer says, or none when it says none — a set replaces the layer whole, placement too. +func keepPlacement(ctx context.Context, tx pgx.Tx, nodeID *string, module string, placed []byte, setBy string) error { + if placed == nil { + _, err := tx.Exec(ctx, + `delete from placement where module = $1 and node is not distinct from $2::uuid`, module, nodeID) + return err + } + var err error + if nodeID == nil { + _, err = tx.Exec(ctx, + `insert into placement (node, module, values, set_by) values (null, $1, $2, $3) + on conflict (module) where node is null + do update set values = excluded.values, set_at = now(), set_by = excluded.set_by`, + module, placed, nullable(setBy)) + } else { + _, err = tx.Exec(ctx, + `insert into placement (node, module, values, set_by) values ($1, $2, $3, $4) + on conflict (node, module) where node is not null + do update set values = excluded.values, set_at = now(), set_by = excluded.set_by`, + *nodeID, module, placed, nullable(setBy)) + } + return wrapModule(err, module) +} + // PinProvision records which provider a machine gets a provision from: a module, and the node it // runs on — both, always (novox/hq #258). A provision comes from a module, and the same module on // two machines is two answers, so neither half alone says which. diff --git a/internal/inventory/unseen.go b/internal/inventory/unseen.go index 8acbf13b..023fa315 100644 --- a/internal/inventory/unseen.go +++ b/internal/inventory/unseen.go @@ -20,9 +20,11 @@ func (i *Inventory) Layer(ctx context.Context, nodeName, module string) (map[str if err != nil { return nil, false, err } + // Said whole, as it was set: its config values and the placement kept beside them (novox/hq ADR 0348 + // clause 11), so a person reading it before setting it sees every key they may keep or remove. var raw []byte - err = i.store.Pool().QueryRow(ctx, - `select values from settings where module = $1 and node is not distinct from $2::uuid`, + err = i.store.Pool().QueryRow(ctx, `select `+layerWhole+` from settings s + where s.module = $1 and s.node is not distinct from $2::uuid`, module, nodeID).Scan(&raw) if errors.Is(err, pgx.ErrNoRows) { return nil, false, nil @@ -46,6 +48,9 @@ type PastLayer struct { ReplacedBy string // SetBy is who had set the layer, when it was kept (novox/hq ADR 0277); empty for one set before that was kept. SetBy string + // Why is why it went, where that was said: "moved to placement, ADR 0348 clause 11" for a layer whose + // placement keys migration 0097 moved out of it (ReplacedBy "moved"). + Why string } // LayerOrigin is who set a layer and when (novox/hq ADR 0277): empty words for a layer set before who set it was @@ -79,7 +84,7 @@ func (i *Inventory) SettingsHistory(ctx context.Context, nodeName, module string return nil, err } rows, err := i.store.Pool().Query(ctx, - `select values, set_at, replaced_at, replaced_by, set_by from settings_history + `select values, set_at, replaced_at, replaced_by, set_by, why from settings_history where module = $1 and node is not distinct from $2::uuid order by replaced_at desc`, module, nodeID) if err != nil { @@ -90,13 +95,16 @@ func (i *Inventory) SettingsHistory(ctx context.Context, nodeName, module string for rows.Next() { var raw []byte var p PastLayer - var by *string - if err := rows.Scan(&raw, &p.SetAt, &p.ReplacedAt, &p.ReplacedBy, &by); err != nil { + var by, why *string + if err := rows.Scan(&raw, &p.SetAt, &p.ReplacedAt, &p.ReplacedBy, &by, &why); err != nil { return nil, err } if by != nil { p.SetBy = *by } + if why != nil { + p.Why = *why + } if err := json.Unmarshal(raw, &p.Values); err != nil { return nil, err } @@ -107,9 +115,15 @@ func (i *Inventory) SettingsHistory(ctx context.Context, nodeName, module string // keepReplacedSQL copies a layer into the history before it is replaced or cleared; run in the same // transaction as the write where there is one, so a write that fails leaves no history of it. +// +// The layer is kept whole, as it was set: its config values and its placement (novox/hq ADR 0348 clause 11). const keepReplacedSQL = `insert into settings_history (node, module, values, set_at, replaced_by, set_by) - select node, module, values, set_at, $3, set_by from settings - where module = $1 and node is not distinct from $2::uuid` + select s.node, s.module, ` + layerWhole + `, s.set_at, $3, s.set_by from settings s + where s.module = $1 and s.node is not distinct from $2::uuid` + +// layerWhole is a settings row `s` with the placement kept beside it folded back in: the layer as it was set. +const layerWhole = `s.values || coalesce((select p.values from placement p + where p.module = s.module and p.node is not distinct from s.node), '{}'::jsonb)` // layerNode is the node id of a layer, nil for the whole mesh's. func (i *Inventory) layerNode(ctx context.Context, nodeName string) (*string, error) { -- 2.54.0 From 85b85a3fa990e85c6703016c679d1ea2b02bf6d8 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 00:43:40 +0000 Subject: [PATCH 3/6] Say a layer whole where the mesh reports it, and give tests their placement (issue 651) The facts snapshot, the take preview and the settings history read a layer as it was set; tests that place give the layer through LayerOf. Co-Authored-By: Claude Opus 5.5 --- cmd/mesh-controller/adoption.go | 5 +++-- cmd/mesh-controller/facts.go | 4 ++-- cmd/mesh-controller/mesh_for_test.go | 16 +++++++++++++++- cmd/mesh-controller/modules.go | 4 ++++ internal/catalogue/data_test.go | 2 +- internal/catalogue/setting_defaults_test.go | 2 +- internal/catalogue/settings_judge_test.go | 14 +++++++------- internal/catalogue/terminal_settings_test.go | 14 +++++++------- testdata/store-tests | 3 +++ 9 files changed, 43 insertions(+), 21 deletions(-) diff --git a/cmd/mesh-controller/adoption.go b/cmd/mesh-controller/adoption.go index 411ae05d..afa90a07 100644 --- a/cmd/mesh-controller/adoption.go +++ b/cmd/mesh-controller/adoption.go @@ -424,8 +424,9 @@ func comparisonOf(module string, c comparison, opts takeOptions) (preview string } // And its settings on this machine, composed against its definition (rule 1, rule 6). for _, layer := range c.layers { - keys := make([]string, 0, len(layer.Values)) - for k := range layer.Values { + said := layer.Whole() + keys := make([]string, 0, len(said)) + for k := range said { keys = append(keys, k) } sort.Strings(keys) diff --git a/cmd/mesh-controller/facts.go b/cmd/mesh-controller/facts.go index 50b891cf..af04f202 100644 --- a/cmd/mesh-controller/facts.go +++ b/cmd/mesh-controller/facts.go @@ -348,11 +348,11 @@ func gatherFacts(ctx context.Context, open *stores, busVersion string) (snapshot if layer.From == catalogue.MeshWideLayer { if !meshWide[module] { meshWide[module] = true - f.Settings = append(f.Settings, snapshot.Settings{Module: module, Values: scrub.Values(layer.Values)}) + f.Settings = append(f.Settings, snapshot.Settings{Module: module, Values: scrub.Values(layer.Whole())}) } continue } - m.Settings = append(m.Settings, snapshot.Settings{Module: module, Values: scrub.Values(layer.Values)}) + m.Settings = append(m.Settings, snapshot.Settings{Module: module, Values: scrub.Values(layer.Whole())}) } } m.Declaration = declarationFacts(ctx, open, n.Name, gens, gensErr, scrub) diff --git a/cmd/mesh-controller/mesh_for_test.go b/cmd/mesh-controller/mesh_for_test.go index b2e76be4..eea81d13 100644 --- a/cmd/mesh-controller/mesh_for_test.go +++ b/cmd/mesh-controller/mesh_for_test.go @@ -33,7 +33,21 @@ import ( func aMesh(t *testing.T) *stores { t.Helper() inventory.ForTest(t) // raises the store, migrates it, and points the environment at it - licences.ForTest(t) // planning reaches this one too, by name and never by connection + return aMeshOnTheStore(t) +} + +// aMeshBefore is aMesh on a store migrated only below migration `number`, with the function that runs the +// rest: for a test of what a migration that moves data does to what the mesh composes (novox/hq issue 651). +func aMeshBefore(t *testing.T, number int) (*stores, func()) { + t.Helper() + _, migrate := inventory.ForTestBefore(t, number) + return aMeshOnTheStore(t), migrate +} + +// aMeshOnTheStore is the mesh aMesh makes, on the store the environment points at. +func aMeshOnTheStore(t *testing.T) *stores { + t.Helper() + licences.ForTest(t) // planning reaches this one too, by name and never by connection open, err := openStores(t.Context()) if err != nil { diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index 66804655..dc3ea06c 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -537,6 +537,10 @@ func settingsCommand(ctx context.Context, args []string) error { if p.SetBy != "" { by = "; " + p.SetBy } + if p.Why != "" { + // A layer whose placement moved out of it (migration 0097): still the layer, as it stands. + by += "; " + p.Why + } fmt.Printf("%s on %s, until %s (%s%s):\n %s\n", positionals[0], where, p.ReplacedAt.Local().Format("2006-01-02 15:04:05"), p.ReplacedBy, by, shown) } diff --git a/internal/catalogue/data_test.go b/internal/catalogue/data_test.go index fde16ca2..d60f3356 100644 --- a/internal/catalogue/data_test.go +++ b/internal/catalogue/data_test.go @@ -173,7 +173,7 @@ func TestTheBackupHoldersLinesAreDerivedFromTheData(t *testing.T) { {"id":"meta","path":"${dir:meta}","class":"rebuildable"}]}, "resources":[{"id":"meta","type":"directory","mode":"0700"}]}`) facts := map[string]string{"account-home": "/home/op"} - with := Rendering{Settings: SettingsBy{"media": {{From: "node", Values: map[string]any{AccessesSetting: map[string]any{"films": "/tank/films"}}}}}} + with := Rendering{Settings: SettingsBy{"media": {LayerOf("node", map[string]any{AccessesSetting: map[string]any{"films": "/tank/films"}})}}} backup, _, err := seatContributions([]Manifest{pg, agent, media}, Manifest{}, BackupSeat+":"+BackupKindBackup, with, facts, nil) if err != nil { t.Fatal(err) diff --git a/internal/catalogue/setting_defaults_test.go b/internal/catalogue/setting_defaults_test.go index f23fb5e1..d5e50dad 100644 --- a/internal/catalogue/setting_defaults_test.go +++ b/internal/catalogue/setting_defaults_test.go @@ -427,7 +427,7 @@ func TestALeftOutModulesUnplaceableBackupLineCostsOnlyThatLine(t *testing.T) { r := anAdoptedAnchor() r.Modules = append(r.Modules, holder, media, placedBadly) with := anchorRendering(false) - with.Settings["notes"] = []Layer{{From: "anchor", Values: map[string]any{PlacesSetting: "not a map"}}} + with.Settings["notes"] = []Layer{LayerOf("anchor", map[string]any{PlacesSetting: "not a map"})} composed, err := r.Compose(with) if err != nil { t.Fatalf("an unplaceable line of a left-out module failed the machine: %v", err) diff --git a/internal/catalogue/settings_judge_test.go b/internal/catalogue/settings_judge_test.go index 50d96222..cbb494c9 100644 --- a/internal/catalogue/settings_judge_test.go +++ b/internal/catalogue/settings_judge_test.go @@ -27,7 +27,7 @@ func TestASettingThatCannotComposeIsRefusedByNameAndLeavesOnlyItsModuleOut(t *te if c.name == "a mesh-wide port" { from = MeshWideLayer } - err := JudgeSettings(web, []Layer{{From: from, Values: c.layer}}, true) + err := JudgeSettings(web, []Layer{LayerOf(from, c.layer)}, true) if err == nil || !strings.Contains(err.Error(), c.refuse) { t.Errorf("%s: judged %v, want %q", c.name, err, c.refuse) } @@ -69,8 +69,8 @@ func TestASettingThatCannotComposeIsRefusedByNameAndLeavesOnlyItsModuleOut(t *te // declaration as the networks it also joins. func TestAKeptNetworkReachesTheContainerOnAnAdoptedMachineOnly(t *testing.T) { r := anAdoptedAnchor() - keep := SettingsBy{"hello-web": {{From: "anchor", - Values: map[string]any{NetworksSetting: map[string]any{"server": []any{"predecessor_default"}}}}}} + keep := SettingsBy{"hello-web": {LayerOf("anchor", + map[string]any{NetworksSetting: map[string]any{"server": []any{"predecessor_default"}}})}} with := anchorRendering(true) with.Settings = keep @@ -107,13 +107,13 @@ func TestAKeptNetworkReachesTheContainerOnAnAdoptedMachineOnly(t *testing.T) { layer Layer want string }{ - {"mesh-wide", Layer{From: MeshWideLayer, Values: map[string]any{NetworksSetting: map[string]any{"server": []any{"x"}}}}, + {"mesh-wide", LayerOf(MeshWideLayer, map[string]any{NetworksSetting: map[string]any{"server": []any{"x"}}}), "a found network is a fact about one machine"}, - {"an unknown container", Layer{From: "anchor", Values: map[string]any{NetworksSetting: map[string]any{"db": []any{"x"}}}}, + {"an unknown container", LayerOf("anchor", map[string]any{NetworksSetting: map[string]any{"db": []any{"x"}}}), `names the container "db", which it does not declare`}, - {"not a list", Layer{From: "anchor", Values: map[string]any{NetworksSetting: map[string]any{"server": "x"}}}, + {"not a list", LayerOf("anchor", map[string]any{NetworksSetting: map[string]any{"server": "x"}}), "is a list of network names"}, - {"not a network name", Layer{From: "anchor", Values: map[string]any{NetworksSetting: map[string]any{"server": []any{"a/b"}}}}, + {"not a network name", LayerOf("anchor", map[string]any{NetworksSetting: map[string]any{"server": []any{"a/b"}}}), "which is not a network name"}, } { _, err := KeptNetworks(web, []Layer{c.layer}, true) diff --git a/internal/catalogue/terminal_settings_test.go b/internal/catalogue/terminal_settings_test.go index ac767cfd..d822a896 100644 --- a/internal/catalogue/terminal_settings_test.go +++ b/internal/catalogue/terminal_settings_test.go @@ -13,22 +13,22 @@ func TestAPlacementOrAnAccessAtTheMachinesOwnIsRefused(t *testing.T) { for _, path := range []string{"/", "/etc", "/etc/", "/etc/sudoers.d", "/usr/bin", "/root", "/var", "/var/lib", "/home", "/home/", "/run", "/run/user/1000", "/var/lib/mesh/x", "/var/lib/mesh-host", "/var/lib/mesh-host/identity", "/srv/../etc", "//etc", "/proc/1", "/sys", "/dev", "/boot/efi", "/bin", "/sbin", "/lib", "/lib64"} { - layers := []Layer{{From: "laptop", Values: map[string]any{PlacesSetting: map[string]any{"data": path}}}} + layers := []Layer{LayerOf("laptop", map[string]any{PlacesSetting: map[string]any{"data": path}})} if _, err := Places(m, layers); err == nil || !strings.Contains(err.Error(), "issue 339") { t.Errorf("a place at %s: %v", path, err) } - layers = []Layer{{From: "laptop", Values: map[string]any{AccessesSetting: map[string]any{"media": path}}}} + layers = []Layer{LayerOf("laptop", map[string]any{AccessesSetting: map[string]any{"media": path}})} if _, err := AccessPlaces(m, layers); err == nil || !strings.Contains(err.Error(), "issue 339") { t.Errorf("an access at %s: %v", path, err) } } for _, path := range []string{"/srv/notes", "/mnt/plex/data", "/storage/media", "/services/media/movies", "/var/lib/notes/data", "/home/restic/repo", "/var/lib/mesh-store"} { - layers := []Layer{{From: "laptop", Values: map[string]any{PlacesSetting: map[string]any{"data": path}}}} + layers := []Layer{LayerOf("laptop", map[string]any{PlacesSetting: map[string]any{"data": path}})} if got, err := Places(m, layers); err != nil || got["data"].Path != path { t.Errorf("a place at %s: %v %v", path, got, err) } - layers = []Layer{{From: "laptop", Values: map[string]any{AccessesSetting: map[string]any{"media": path}}}} + layers = []Layer{LayerOf("laptop", map[string]any{AccessesSetting: map[string]any{"media": path}})} if got, err := AccessPlaces(m, layers); err != nil || got["media"] != path { t.Errorf("an access at %s: %v %v", path, got, err) } @@ -111,17 +111,17 @@ func TestTheRuntimesDataAndAnyHomesSSHAreTheMachinesOwn(t *testing.T) { for _, path := range []string{"/var/spool", "/var/spool/cron", "/var/lib/docker", "/var/lib/docker/volumes", "/var/lib/containers/storage", "/var/lib/containerd", "/var/lib/containerd/io.containerd.snapshotter.v1", "/opt", "/opt/app", "/home/alice/.ssh", "/home/alice/.ssh/keys", "/srv/backup/.ssh", "/root/.ssh"} { - layers := []Layer{{From: "laptop", Values: map[string]any{PlacesSetting: map[string]any{"data": path}}}} + layers := []Layer{LayerOf("laptop", map[string]any{PlacesSetting: map[string]any{"data": path}})} if _, err := Places(m, layers); err == nil || !strings.Contains(err.Error(), "issue 339") { t.Errorf("a place at %s: %v", path, err) } - layers = []Layer{{From: "laptop", Values: map[string]any{AccessesSetting: map[string]any{"media": path}}}} + layers = []Layer{LayerOf("laptop", map[string]any{AccessesSetting: map[string]any{"media": path}})} if _, err := AccessPlaces(m, layers); err == nil || !strings.Contains(err.Error(), "issue 339") { t.Errorf("an access at %s: %v", path, err) } } for _, path := range []string{"/var/lib/dockerish", "/home/alice/ssh", "/home/alice/.sshd-notes", "/storage/media"} { - layers := []Layer{{From: "laptop", Values: map[string]any{PlacesSetting: map[string]any{"data": path}}}} + layers := []Layer{LayerOf("laptop", map[string]any{PlacesSetting: map[string]any{"data": path}})} if _, err := Places(m, layers); err != nil { t.Errorf("a place at %s: %v", path, err) } diff --git a/testdata/store-tests b/testdata/store-tests index bbfc1467..d3af9659 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -61,3 +61,6 @@ ./cmd/mesh-controller TestAProviderElsewhereServesItsOwnComposedName ./cmd/mesh-controller TestAReusedCheckBuildIsRegisteredByItsDigestAndGatedAsABuiltOne ./internal/inventory TestAReportOfABusyContainerIsKeptAndReadBack +./internal/inventory TestPlacementMovesOutOfTheSettingsLayers +./internal/inventory TestASettingRoutesPlacementBesideTheLayer +./cmd/mesh-controller TestADeclarationIsTheSameAfterPlacementMoves -- 2.54.0 From f0af09cfc764f0c80c58d784a1324a7e757dd526 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 00:44:20 +0000 Subject: [PATCH 4/6] Test placement read only from the placement, and raise a store before a migration (issue 651) Co-Authored-By: Claude Opus 5.5 --- internal/catalogue/placement_test.go | 42 ++++++++++++- internal/inventory/fortest.go | 92 ++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 1 deletion(-) diff --git a/internal/catalogue/placement_test.go b/internal/catalogue/placement_test.go index 1b02ca8d..e75d770b 100644 --- a/internal/catalogue/placement_test.go +++ b/internal/catalogue/placement_test.go @@ -23,7 +23,7 @@ func placeable() Manifest { } func placedBy(values map[string]any) Rendering { - return Rendering{Settings: SettingsBy{"arr": {{From: "node anchor", Values: values}}}} + return Rendering{Settings: SettingsBy{"arr": {LayerOf("node anchor", values)}}} } func TestAnAssignmentPlacesDirectoriesAndAccesses(t *testing.T) { @@ -179,3 +179,43 @@ func TestAnUnknownAccessReferenceIsRefusedAtParse(t *testing.T) { t.Fatalf("an access with no id and no path was accepted: %v", err) } } + +// **Placement is read from the placement, never from a layer's values** (novox/hq ADR 0348 clause 11, issue +// 651). A `places` or `accesses` among the config values places nothing; the same in Placement does. A +// mergeable file of today's form still takes every key of the layer as it is said, placement too, so it +// composes as it did while placement was kept in the layer. +func TestPlacementIsReadFromThePlacementOnly(t *testing.T) { + m := placeable() + said := map[string]any{ + PlacesSetting: map[string]any{"config": "/services/arr/config"}, + AccessesSetting: map[string]any{"series": "/storage/media/series", "spool": "/storage/downloads"}, + "log": "warn", + } + among := []Layer{{From: "anchor", Values: said}} + if placed, err := Places(m, among); err != nil || placed != nil { + t.Fatalf("a place among the config values placed %v (%v)", placed, err) + } + if _, _, err := accessesFor(m, among); err == nil { + t.Fatal("an access among the config values placed it") + } + beside := []Layer{LayerOf("anchor", said)} + if beside[0].Values["log"] != "warn" || beside[0].Values[PlacesSetting] != nil || beside[0].Placement[PlacesSetting] == nil { + t.Fatalf("LayerOf split %+v", beside[0]) + } + if placed, err := Places(m, beside); err != nil || placed["config"].Path != "/services/arr/config" { + t.Fatalf("the placement did not place: %v (%v)", placed, err) + } + + file := map[string]any{"id": "runtime", "type": "file", "path": "/x", "content": `{"a":1}`, "merge": "json"} + whole, err := ApplySettings(file, among) + if err != nil { + t.Fatal(err) + } + split, err := ApplySettings(file, beside) + if err != nil { + t.Fatal(err) + } + if whole["content"] != split["content"] || !strings.Contains(split["content"].(string), `"places"`) { + t.Fatalf("today's mergeable file composes %q, was %q", split["content"], whole["content"]) + } +} diff --git a/internal/inventory/fortest.go b/internal/inventory/fortest.go index 77aec9cc..cbd7f73a 100644 --- a/internal/inventory/fortest.go +++ b/internal/inventory/fortest.go @@ -2,6 +2,7 @@ package inventory import ( "context" + "encoding/json" "fmt" "os" "strings" @@ -75,3 +76,94 @@ func (i *Inventory) RemoveNodeForTest(ctx context.Context, name string) (bool, e } return tag.RowsAffected() == 1, nil } + +// ForTestBefore is ForTest with the store migrated only below migration `number`, and a function that runs the +// rest: for a test of a migration that moves data, which seeds the store as the mesh holds it before the +// migration, migrates, and reads what the migration made of it (novox/hq issue 651). +func ForTestBefore(t *testing.T, number int) (*Inventory, func()) { + t.Helper() + admin := os.Getenv("MESH_TEST_POSTGRES") + if admin == "" { + t.Skip("no MESH_TEST_POSTGRES; run `make check` to raise one") + } + name := fmt.Sprintf("invb_%d_%s", time.Now().UnixNano()%1_000_000, + strings.ToLower(strings.NewReplacer("/", "", "-", "").Replace(t.Name()))) + if len(name) > 60 { + name = name[:60] + } + all, err := Migrations() + if err != nil { + t.Fatal(err) + } + var before []store.Migration + for _, m := range all { + if m.Number < number { + before = append(before, m) + } + } + store.CreateMigratedForTest(t, admin, name, before) + busPasswordCostForTest.Store(int32(bcrypt.MinCost)) + cut := strings.LastIndex(admin, "/") + t.Setenv(store.Variable(Name), admin[:cut]+"/"+name+"?sslmode=disable") + inv, err := Open(t.Context()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + inv.Close() + c, err := pgx.Connect(context.Background(), admin) + if err != nil { + return + } + defer c.Close(context.Background()) + _, _ = c.Exec(context.Background(), "drop database if exists "+name+" with (force)") + }) + if err := inv.Ready(t.Context(), 20*time.Second); err != nil { + t.Fatal(err) + } + return inv, func() { + t.Helper() + if _, err := inv.store.Migrate(t.Context(), all); err != nil { + t.Fatal(err) + } + } +} + +// ExecForTest runs one statement against a test's store, for a fixture no verb of the mesh writes: a row as an +// older migration left it. +func (i *Inventory) ExecForTest(t *testing.T, sql string, args ...any) { + t.Helper() + if _, err := i.store.Pool().Exec(t.Context(), sql, args...); err != nil { + t.Fatalf("%s: %v", sql, err) + } +} + +// QueryForTest reads rows of a test's store, each row's columns as JSON text, for a test that checks what a +// migration left. +func (i *Inventory) QueryForTest(t *testing.T, sql string, args ...any) [][]string { + t.Helper() + rows, err := i.store.Pool().Query(t.Context(), sql, args...) + if err != nil { + t.Fatalf("%s: %v", sql, err) + } + defer rows.Close() + var out [][]string + for rows.Next() { + values, err := rows.Values() + if err != nil { + t.Fatal(err) + } + row := make([]string, len(values)) + for k, v := range values { + row[k] = fmt.Sprint(v) + if b, err := json.Marshal(v); err == nil { + row[k] = string(b) + } + } + out = append(out, row) + } + if err := rows.Err(); err != nil { + t.Fatal(err) + } + return out +} -- 2.54.0 From 3048dccd0e51a3c864c50323dc015ead354b4e07 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 00:44:51 +0000 Subject: [PATCH 5/6] Keep the declarations the controller composed before placement moved (issue 651) Written by the integration branch's controller at bfc79c6 from the seed of TestADeclarationIsTheSameAfterPlacementMoves. Co-Authored-By: Claude Opus 5.5 --- .../testdata/placement-moved/anchor.json | 46 +++++++++++++++++++ .../testdata/placement-moved/laptop.json | 37 +++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 cmd/mesh-controller/testdata/placement-moved/anchor.json create mode 100644 cmd/mesh-controller/testdata/placement-moved/laptop.json diff --git a/cmd/mesh-controller/testdata/placement-moved/anchor.json b/cmd/mesh-controller/testdata/placement-moved/anchor.json new file mode 100644 index 00000000..f8f56655 --- /dev/null +++ b/cmd/mesh-controller/testdata/placement-moved/anchor.json @@ -0,0 +1,46 @@ +[ + { + "id": "found.server", + "image": "registry.invalid:5000/library@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "networks": [ + "predecessor_default" + ], + "type": "container" + }, + { + "id": "library.access-storage/downloads", + "mode": "read", + "path": "/storage/downloads", + "type": "access" + }, + { + "id": "library.access-storage/media/series", + "mode": "read", + "path": "/storage/media/series", + "type": "access" + }, + { + "id": "library.config", + "mode": "0750", + "owner": "1001:2000", + "path": "/var/lib/library/config", + "type": "directory" + }, + { + "content": "{\n \"accesses\": {\n \"downloads\": \"/storage/downloads\",\n \"series\": \"/storage/media/series\"\n },\n \"log\": \"warn\",\n \"places\": {\n \"config\": {\n \"owner\": \"1001:2000\",\n \"path\": \"/var/lib/library/config\"\n }\n },\n \"workers\": 4\n}\n", + "id": "library.runtime-config", + "mode": "0640", + "path": "/var/lib/library/config/runtime.json", + "type": "file" + }, + { + "id": "library.server", + "image": "registry.invalid:5000/library@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "type": "container", + "volumes": [ + "/var/lib/library/config:/config", + "/storage/media/series:/series", + "/storage/downloads:/downloads:ro" + ] + } +] diff --git a/cmd/mesh-controller/testdata/placement-moved/laptop.json b/cmd/mesh-controller/testdata/placement-moved/laptop.json new file mode 100644 index 00000000..47884c49 --- /dev/null +++ b/cmd/mesh-controller/testdata/placement-moved/laptop.json @@ -0,0 +1,37 @@ +[ + { + "id": "library.access-home/media/series", + "mode": "read", + "path": "/home/media/series", + "type": "access" + }, + { + "id": "library.access-storage/downloads", + "mode": "read", + "path": "/storage/downloads", + "type": "access" + }, + { + "id": "library.config", + "mode": "0750", + "path": "/var/lib/library/config", + "type": "directory" + }, + { + "content": "{\n \"accesses\": {\n \"downloads\": \"/storage/downloads\",\n \"series\": \"/home/media/series\"\n },\n \"log\": \"warn\"\n}\n", + "id": "library.runtime-config", + "mode": "0640", + "path": "/var/lib/library/config/runtime.json", + "type": "file" + }, + { + "id": "library.server", + "image": "registry.invalid:5000/library@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "type": "container", + "volumes": [ + "/var/lib/library/config:/config", + "/home/media/series:/series", + "/storage/downloads:/downloads:ro" + ] + } +] -- 2.54.0 From 912d76df25f858f52902d734cc9559eb65de75b1 Mon Sep 17 00:00:00 2001 From: mesh-admin Date: Wed, 14 Oct 2026 00:45:58 +0000 Subject: [PATCH 6/6] Move every layer's placement out with migration 0097, and prove no machine is told anything new (issue 651) Co-Authored-By: Claude Opus 5.5 --- cmd/mesh-controller/placement_moved_test.go | 113 +++++++++ .../0097-placement-is-not-config.sql | 46 ++++ internal/inventory/placement_test.go | 222 ++++++++++++++++++ 3 files changed, 381 insertions(+) create mode 100644 cmd/mesh-controller/placement_moved_test.go create mode 100644 internal/inventory/migrations/0097-placement-is-not-config.sql create mode 100644 internal/inventory/placement_test.go diff --git a/cmd/mesh-controller/placement_moved_test.go b/cmd/mesh-controller/placement_moved_test.go new file mode 100644 index 00000000..5831cc56 --- /dev/null +++ b/cmd/mesh-controller/placement_moved_test.go @@ -0,0 +1,113 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// **Placement leaves the settings layers and no machine is told anything new** (novox/hq ADR 0348 clause +// 11, issue 651). A mesh is seeded as the store holds it before migration 0097 — `places`, `accesses` and a +// kept network among the settings, at the mesh's level and a machine's, one layer of placement only — then +// migrated, and every machine's declaration is composed. What each machine is told of the seeded modules is +// byte for byte what the controller before this change composed from the same seed: the golden files were +// written by the integration branch's controller at bfc79c6 running this same test (MESH_WRITE_GOLDEN=1), +// where there was no migration 0097 to run. +func TestADeclarationIsTheSameAfterPlacementMoves(t *testing.T) { + open, migrate := aMeshBefore(t, 97) + ctx := t.Context() + inv := open.inventory + image := "registry.invalid:5000/library@sha256:" + strings.Repeat("a", 64) + register(t, open, catalogue.Manifest{Module: "library", Version: "1", + Accesses: []catalogue.Access{{ID: "series"}, {ID: "downloads", Mode: catalogue.AccessRead}}, + Resources: []map[string]any{ + {"id": "config", "type": "directory", "mode": "0750"}, + {"id": "server", "type": "container", "image": image, + "volumes": []any{"${dir:config}:/config", "${access:series}:/series", "${access:downloads}:/downloads:ro"}}, + // Today's form: a mergeable file naming no keys takes every key of the layer (ADR 0326 §3). + {"id": "runtime-config", "type": "file", "path": "${dir:config}/runtime.json", "mode": "0640", + "content": `{"log":"info"}`, "merge": "json"}, + }}) + register(t, open, catalogue.Manifest{Module: "found", Version: "1", + Resources: []map[string]any{{"id": "server", "type": "container", "image": image}}}) + if err := inv.SetAdopted(ctx, "anchor", true); err != nil { + t.Fatal(err) + } + for _, l := range []struct{ node, module, values string }{ + {"", "library", `{"accesses": {"downloads": "/storage/downloads"}, "log": "warn"}`}, + {"anchor", "library", `{"places": {"config": {"path": "/var/lib/library/config", "owner": "1001:2000"}}, + "accesses": {"series": "/storage/media/series"}, "workers": 4}`}, + {"laptop", "library", `{"accesses": {"series": "/home/media/series"}}`}, + {"anchor", "found", `{"networks": {"server": ["predecessor_default"]}}`}, + } { + if l.node == "" { + inv.ExecForTest(t, `insert into settings (node, module, values) values (null, $1, $2)`, l.module, l.values) + continue + } + inv.ExecForTest(t, `insert into settings (node, module, values) select id, $2, $3 from node where name = $1`, + l.node, l.module, l.values) + } + + migrate() + // Assigned once the store is migrated: an assignment reads the layers, as this controller holds them. + for _, a := range [][]string{{"anchor", "library", "found"}, {"laptop", "library"}} { + if _, err := assign(ctx, open, a[0], a[1:]...); err != nil { + t.Fatalf("assign %v: %v", a, err) + } + } + + gens, err := generators(ctx, open) + if err != nil { + t.Fatal(err) + } + for _, node := range []string{"anchor", "laptop"} { + plan, settings, err := planFor(ctx, open, node) + if err != nil { + t.Fatal(err) + } + with, _, err := renderingFor(ctx, open, node, plan, settings, gens, Allocating) + if err != nil { + t.Fatal(err) + } + composed, err := plan.Compose(with) + if err != nil { + t.Fatal(err) + } + if len(composed.LeftOut) > 0 { + t.Fatalf("%s leaves out %v", node, composed.LeftOut) + } + var told []map[string]any + for _, r := range composed.Resources { + if owner := composed.Owner[r["id"].(string)]; owner == "library" || owner == "found" { + told = append(told, r) + } + } + sort.Slice(told, func(i, j int) bool { return told[i]["id"].(string) < told[j]["id"].(string) }) + got, err := json.MarshalIndent(told, "", " ") + if err != nil { + t.Fatal(err) + } + golden := filepath.Join("testdata", "placement-moved", node+".json") + if os.Getenv("MESH_WRITE_GOLDEN") != "" { + if err := os.MkdirAll(filepath.Dir(golden), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(golden, append(got, '\n'), 0o644); err != nil { + t.Fatal(err) + } + continue + } + want, err := os.ReadFile(golden) + if err != nil { + t.Fatal(err) + } + if string(want) != string(got)+"\n" { + t.Errorf("%s is told something else after placement moved:\n%s\nwas:\n%s", node, got, want) + } + } +} diff --git a/internal/inventory/migrations/0097-placement-is-not-config.sql b/internal/inventory/migrations/0097-placement-is-not-config.sql new file mode 100644 index 00000000..553cf6d6 --- /dev/null +++ b/internal/inventory/migrations/0097-placement-is-not-config.sql @@ -0,0 +1,46 @@ +-- Placement is not config (novox/hq ADR 0348 clause 11, issue 651). +-- +-- Where a module's directories are on a machine (`places`), where the operator's data it reads is +-- (`accesses`) and which found networks a taken container keeps (`networks`) were kept as keys of a +-- settings layer. They are facts of an assignment that the controller decides, never config values, and +-- the config values are leaving for the mesh-config module (ADR 0348 clause 9) while placement stays with +-- the controller. So they get a table of their own beside the assignment, and every settings layer's +-- placement keys move into it, unchanged. +-- +-- Scoped exactly as the layer they came from: one machine's, or the whole mesh's (node null) for a +-- module's every assignment, which is how a mesh-wide `places` or `accesses` layer composes today. The +-- composer reads placement from here only; the `settings` verb still takes the three keys and routes +-- them here, so nothing a person types changes. +create table placement ( + node uuid references node(id) on delete cascade, + module text not null references module(name) on delete cascade, + -- Only `places`, `accesses` and `networks`, as the settings layer held them. + values jsonb not null, + set_at timestamptz not null default now(), + set_by text +); +create unique index placement_for_the_mesh on placement (module) where node is null; +create unique index placement_for_a_node on placement (node, module) where node is not null; + +-- Why a layer went into the history, beside how (`replaced_by`): said for a layer this migration moved. +alter table settings_history add column why text; + +-- Each layer that held a placement key is kept whole in the history first, as a `settings history` reads it. +insert into settings_history (node, module, values, set_at, replaced_by, set_by, why) +select node, module, values, set_at, 'moved', set_by, 'moved to placement, ADR 0348 clause 11' + from settings + where values ?| array['places', 'accesses', 'networks']; + +insert into placement (node, module, values, set_at, set_by) +select node, module, + (select jsonb_object_agg(e.key, e.value) from jsonb_each(values) as e + where e.key in ('places', 'accesses', 'networks')), + set_at, set_by + from settings + where values ?| array['places', 'accesses', 'networks']; + +-- The layer keeps its other keys, when it was set and who set it; a layer that held only placement stays, +-- empty, so a layer that existed still exists. +update settings + set values = values - 'places' - 'accesses' - 'networks' + where values ?| array['places', 'accesses', 'networks']; diff --git a/internal/inventory/placement_test.go b/internal/inventory/placement_test.go new file mode 100644 index 00000000..0483fe7b --- /dev/null +++ b/internal/inventory/placement_test.go @@ -0,0 +1,222 @@ +package inventory + +import ( + "encoding/json" + "reflect" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// placedLibrary is a module with each kind of placement: a directory to place, two accesses, a container that +// may keep a found network. +func placedLibrary() catalogue.Manifest { + return catalogue.Manifest{Module: "library", Version: "1", + Accesses: []catalogue.Access{{ID: "series"}, {ID: "downloads"}}, + Resources: []map[string]any{ + {"id": "config", "type": "directory", "mode": "0750"}, + {"id": "server", "type": "container", + "image": "registry.invalid:5000/library@sha256:" + strings.Repeat("a", 64), + "volumes": []any{"${dir:config}:/config", "${access:series}:/series", "${access:downloads}:/downloads"}}, + {"id": "runtime-config", "type": "file", "path": "${dir:config}/runtime.json", "mode": "0640", + "content": `{"log":"info"}`, "merge": "json"}, + }} +} + +// The layers of the issue (651): a mesh-wide layer with an access and a config value, a node's layer with a +// place, an access and a kept network beside a config value, a node's layer of placement only, and one with +// none. +var placementLayers = []struct { + node string + values string +}{ + {"", `{"accesses": {"downloads": "/storage/downloads"}, "log": "warn"}`}, + {"anchor", `{"places": {"config": {"path": "/var/lib/library/config", "owner": "1001:2000"}}, + "accesses": {"series": "/storage/media/series"}, "networks": {"server": ["predecessor_default"]}, "workers": 4}`}, + {"laptop", `{"accesses": {"series": "/home/media/series"}}`}, + {"spare", `{"log": "debug"}`}, +} + +// **Placement moves out of the settings layers, and every layer reads as it did** (novox/hq ADR 0348 clause +// 11, issue 651). Migration 0097 moves each layer's places, accesses and kept networks beside it: no settings +// row holds a placement key after, every layer the settings verb and composition read is what it was, each +// moved layer is kept whole in the history with why, and a layer without placement is not touched. +func TestPlacementMovesOutOfTheSettingsLayers(t *testing.T) { + inv, migrate := ForTestBefore(t, 97) + ctx := t.Context() + if err := inv.RegisterModule(ctx, placedLibrary(), Source{}); err != nil { + t.Fatal(err) + } + for _, n := range []string{"anchor", "laptop", "spare"} { + if _, err := inv.AddNode(ctx, n); err != nil { + t.Fatal(err) + } + } + if err := inv.SetAdopted(ctx, "anchor", true); err != nil { + t.Fatal(err) + } + // As the mesh holds them before the migration: placement among the settings. + for _, l := range placementLayers { + if l.node == "" { + inv.ExecForTest(t, `insert into settings (node, module, values, set_by) values (null, 'library', $1, 'the operator')`, l.values) + continue + } + inv.ExecForTest(t, `insert into settings (node, module, values) select id, 'library', $2 from node where name = $1`, + l.node, l.values) + } + setAt := inv.QueryForTest(t, `select coalesce(n.name, ''), s.set_at from settings s left join node n on n.id = s.node order by 1`) + + migrate() + + if held := inv.QueryForTest(t, `select module from settings where values ?| array['places', 'accesses', 'networks']`); len(held) != 0 { + t.Fatalf("a settings layer still holds a placement key: %v", held) + } + if after := inv.QueryForTest(t, `select coalesce(n.name, ''), s.set_at from settings s left join node n on n.id = s.node order by 1`); !reflect.DeepEqual(after, setAt) { + t.Fatalf("the layers or when they were set changed: %v, was %v", after, setAt) + } + for _, l := range placementLayers { + var want map[string]any + if err := json.Unmarshal([]byte(l.values), &want); err != nil { + t.Fatal(err) + } + got, has, err := inv.Layer(ctx, l.node, "library") + if err != nil || !has || !reflect.DeepEqual(roundTrip(t, got), want) { + t.Errorf("the layer on %q reads %v (%v, %v), was %v", l.node, got, has, err, want) + } + } + // Composition's view: the config values in Values, the placement in Placement, together the layer. + layers, err := inv.SettingsFor(ctx, "anchor", "library") + if err != nil || len(layers) != 2 { + t.Fatalf("anchor's layers: %v, %v", layers, err) + } + if layers[1].Values["workers"] == nil || layers[1].Values["places"] != nil || layers[1].Placement["places"] == nil || + layers[1].Placement["networks"] == nil || layers[0].Placement["accesses"] == nil || layers[0].Values["log"] != "warn" { + t.Fatalf("anchor's layers split wrong: %+v", layers) + } + for i, l := range layers { + var want map[string]any + _ = json.Unmarshal([]byte(placementLayers[i].values), &want) + if !reflect.DeepEqual(roundTrip(t, l.Whole()), want) { + t.Errorf("anchor's layer %s is %v whole, was %v", l.From, l.Whole(), want) + } + } + // Judged as composition judges them, as before: anchor is adopted, so its kept network composes. + if err := catalogue.JudgeSettings(placedLibrary(), layers, true); err != nil { + t.Fatalf("the moved layers no longer compose: %v", err) + } + + moved := inv.QueryForTest(t, `select coalesce(n.name, ''), h.replaced_by, h.why, h.set_by from settings_history h + left join node n on n.id = h.node order by 1`) + want := [][]string{ + {`""`, `"moved"`, `"moved to placement, ADR 0348 clause 11"`, `"the operator"`}, + {`"anchor"`, `"moved"`, `"moved to placement, ADR 0348 clause 11"`, `null`}, + {`"laptop"`, `"moved"`, `"moved to placement, ADR 0348 clause 11"`, `null`}, + } + if !reflect.DeepEqual(moved, want) { + t.Fatalf("the history of what moved: %v, want %v", moved, want) + } + past, err := inv.SettingsHistory(ctx, "anchor", "library") + if err != nil || len(past) != 1 || past[0].Why != "moved to placement, ADR 0348 clause 11" || past[0].Values["places"] == nil { + t.Fatalf("settings history on anchor: %+v, %v", past, err) + } +} + +// **The settings verb's path still takes a placement key, and routes it** (issue 651): a set keeps the +// placement beside the layer and the rest in it, reads back whole, is refused as before when the placement +// cannot compose (nothing written), and a set or a clear replaces the placement with the layer. +func TestASettingRoutesPlacementBesideTheLayer(t *testing.T) { + inv := ForTest(t) + ctx := t.Context() + if err := inv.RegisterModule(ctx, placedLibrary(), Source{}); err != nil { + t.Fatal(err) + } + if _, err := inv.AddNode(ctx, "anchor"); err != nil { + t.Fatal(err) + } + // Placement alone is a layer as well: the whole mesh's says where both accesses are. + meshWide := map[string]any{"accesses": map[string]any{ + "downloads": "/storage/downloads", "series": "/storage/series"}} + if err := inv.SetSettings(ctx, "", "library", meshWide); err != nil { + t.Fatal(err) + } + if got, has, _ := inv.Layer(ctx, "", "library"); !has || !reflect.DeepEqual(roundTrip(t, got), roundTrip(t, meshWide)) { + t.Fatalf("the mesh-wide layer of placement only reads %v (%v)", got, has) + } + set := map[string]any{"log": "warn", + "places": map[string]any{"config": map[string]any{"path": "/var/lib/library/config"}}, + "accesses": map[string]any{"series": "/storage/media/series"}} + if err := inv.SetSettings(ctx, "anchor", "library", set); err != nil { + t.Fatal(err) + } + stored := inv.QueryForTest(t, `select s.values, p.values from settings s join placement p using (module, node) + where s.node is not null`) + if len(stored) != 1 || stored[0][0] != `{"log":"warn"}` || + !strings.Contains(stored[0][1], `"places"`) || !strings.Contains(stored[0][1], `"accesses"`) { + t.Fatalf("the layer and its placement are kept as %v", stored) + } + if got, _, _ := inv.Layer(ctx, "anchor", "library"); !reflect.DeepEqual(roundTrip(t, got), roundTrip(t, set)) { + t.Fatalf("the layer reads %v, was set %v", got, set) + } + + // Refused as before, by the same judgement, and nothing changes. + for name, bad := range map[string]map[string]any{ + "an undeclared directory": {"places": map[string]any{"cache": "/var/lib/library/cache"}}, + "the machine's own": {"accesses": map[string]any{"series": "/etc"}}, + "a kept network on a converged machine": { + "networks": map[string]any{"server": []any{"predecessor_default"}}}, + } { + if err := inv.SetSettings(ctx, "anchor", "library", bad); err == nil { + t.Errorf("%s was kept", name) + } + } + if err := inv.SetSettings(ctx, "", "library", map[string]any{"accesses": meshWide["accesses"], + "networks": map[string]any{"server": []any{"x"}}}); err == nil || !strings.Contains(err.Error(), "per node") { + t.Errorf("a mesh-wide kept network: %v", err) + } + if got, _, _ := inv.Layer(ctx, "anchor", "library"); !reflect.DeepEqual(roundTrip(t, got), roundTrip(t, set)) { + t.Fatalf("a refused set changed the layer to %v", got) + } + + // A set without placement replaces it too, and the history keeps the layer whole. + if err := inv.SetSettings(ctx, "anchor", "library", map[string]any{"log": "debug"}); err != nil { + t.Fatal(err) + } + if rows := inv.QueryForTest(t, `select module from placement where node is not null`); len(rows) != 0 { + t.Fatalf("a set without placement left it: %v", rows) + } + past, err := inv.SettingsHistory(ctx, "anchor", "library") + if err != nil || len(past) != 1 || past[0].Values["places"] == nil || past[0].Values["log"] != "warn" { + t.Fatalf("the replaced layer is not kept whole: %+v, %v", past, err) + } + + // The merge gate's way in keeps a layer as the facts carry it, whole: split the same way. + if err := inv.KeepSettings(ctx, "anchor", "library", map[string]any{"log": "info", + "places": map[string]any{"config": "/var/lib/library/config"}}); err != nil { + t.Fatal(err) + } + if rows := inv.QueryForTest(t, `select values from settings where values ? 'places'`); len(rows) != 0 { + t.Fatalf("a kept layer put placement among the settings: %v", rows) + } + for _, node := range []string{"", "anchor"} { + if err := inv.ClearSettings(ctx, node, "library"); err != nil { + t.Fatal(err) + } + } + if rows := inv.QueryForTest(t, `select module from placement`); len(rows) != 0 { + t.Fatalf("a clear left the placement: %v", rows) + } +} + +func roundTrip(t *testing.T, v map[string]any) map[string]any { + t.Helper() + b, err := json.Marshal(v) + if err != nil { + t.Fatal(err) + } + out := map[string]any{} + if err := json.Unmarshal(b, &out); err != nil { + t.Fatal(err) + } + return out +} -- 2.54.0