From 409c56685d4796bf6be27ce276d27611abf5f895 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 31 Oct 2026 00:20:01 +0100 Subject: [PATCH] Keep a held module's contributions when only its own secret is missing, and say a provider's move as consumers joining or leaving (issue 774) A module held for an own secret never made (nextcloud's admin on novox, 2026-10-30) was composed as a bare stub, so its contributions left postgres's grants and keycloak's clients, and their helpers would have retired its login. The culprit now tries leaving out its own secrets alone before its whole offer. JudgeMove said a changed received-, bound- or secret- file as "where its data is kept"; it now names the consumers that join or leave. The judgement stays a move: its yes is what kept nextcloud's login. Quick fix: leave out own secrets alone before the whole offer. Thorough fix (not here): carry a held module's last contributions from what runs into its providers' received files whatever part of it fails to compose. --- go/compose/frombus/answer.go | 27 ++++++-- go/compose/frombus/answer_rows_test.go | 2 +- go/compose/frombus/held_rows_test.go | 19 +++++- go/delivery/judge.go | 92 +++++++++++++++++++++++++- go/delivery/judge_test.go | 38 +++++++---- 5 files changed, 153 insertions(+), 25 deletions(-) diff --git a/go/compose/frombus/answer.go b/go/compose/frombus/answer.go index f7d6e93..71e6f22 100644 --- a/go/compose/frombus/answer.go +++ b/go/compose/frombus/answer.go @@ -322,7 +322,7 @@ func composeEach(in compose.Input, node string, fps map[string]string, applied b left[m] = map[string]bool{} } left[m][leave] = true - if leave != leaveAll { + if leave != leaveAll && leave != leaveOwn { failed[m] += "; composed without what it contributes to " + leave + ", which does not compose" } } @@ -404,9 +404,16 @@ func blames(m string, again, err error) bool { return again == nil || again.Error() != err.Error() } -// leaveAll leaves out the whole of a held module's offer and its own secrets (a bare stub); any other word leaves out -// what it contributes to that provision alone. -const leaveAll = "*" +// leaveAll leaves out the whole of a held module's offer and its own secrets (a bare stub); leaveOwn its own secrets +// alone, its offer kept; any other word leaves out what it contributes to that provision alone. +// +// leaveOwn is tried before leaveAll: a module held for an own secret never made (nextcloud's admin, issue 774, +// 2026-10-30) keeps what it contributes, so its providers' received files keep it and their helpers never retire its +// login while it is held. +const ( + leaveAll = "*" + leaveOwn = "own-secrets" +) // assignedOn is the modules placed on node, in placement order. func assignedOn(in compose.Input, node string) []string { @@ -487,6 +494,9 @@ func culprit(in compose.Input, node string, fps map[string]string, mode compose. if isBare(man) { continue } + if len(man.OwnSecrets) > 0 && changes(m, leaveOwn) { + return m, leaveOwn + } if changes(m, leaveAll) { return m, leaveAll } @@ -515,9 +525,9 @@ func stubbed(in compose.Input, node, module, leave string) compose.Input { // heldAs is in with module held on node (compose.Node.Held): composed from man as what runs of it (compose.Carried of // runs; its stub, compose.Stub, when runs is nil or holds nothing of it), as node's Kept; with what left names out of -// its offer: a provision's contributions, or with leaveAll everything it contributes and its own secrets, none of a -// held module's own composition being applied, so what it has not been issued (restic's repository on a node it never -// ran on) is no refusal of the rest. +// its offer: a provision's contributions, with leaveOwn its own secrets alone, or with leaveAll everything it contributes +// and its own secrets, none of a held module's own composition being applied, so what it has not been issued (restic's +// repository on a node it never ran on) is no refusal of the rest. func heldAs(in compose.Input, node, module string, man catalogue.Manifest, left map[string]bool, runs *delivery.Composed) compose.Input { out := in out.Nodes = append([]compose.Node(nil), in.Nodes...) @@ -528,6 +538,9 @@ func heldAs(in compose.Input, node, module string, man catalogue.Manifest, left if left[leaveAll] { man.Contributes, man.ContributesMany, man.OwnSecrets = nil, nil, nil } else if len(left) > 0 { + if left[leaveOwn] { + man.OwnSecrets = nil + } contributes := map[string]map[string]any{} for p, v := range man.Contributes { if !left[p] { diff --git a/go/compose/frombus/answer_rows_test.go b/go/compose/frombus/answer_rows_test.go index 6eb01a2..45fbe64 100644 --- a/go/compose/frombus/answer_rows_test.go +++ b/go/compose/frombus/answer_rows_test.go @@ -516,7 +516,7 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { for _, row := range []struct { left map[string]bool route, smtp, own bool - }{{nil, true, true, true}, {map[string]bool{"route": true}, false, true, true}, {map[string]bool{leaveAll: true}, false, false, false}} { + }{{nil, true, true, true}, {map[string]bool{"route": true}, false, true, true}, {map[string]bool{leaveOwn: true}, true, true, false}, {map[string]bool{leaveAll: true}, false, false, false}} { k := heldAs(withProxy, "n1", "feeder", feeder, row.left, nil).Nodes[0].Kept["feeder"] if _, r := k.Contributes["route"]; r != row.route || (k.Contributes["smtp"] != nil) != row.smtp || (len(k.OwnSecrets) > 0) != row.own { t.Errorf("held leaving out %v offers %+v, own secrets %v", row.left, k.Contributes, k.OwnSecrets) diff --git a/go/compose/frombus/held_rows_test.go b/go/compose/frombus/held_rows_test.go index f38dac6..5566683 100644 --- a/go/compose/frombus/held_rows_test.go +++ b/go/compose/frombus/held_rows_test.go @@ -76,7 +76,8 @@ func aRunningModuleWaitsForItsBuild(t *testing.T) { tools := `{"module":"node-tools","version":"1","own-secrets":{"broker":"/var/lib/mesh/node-tools/broker"},` + `"build":{"artifacts":[{"name":"runtime","kind":"bundle","language":"go","system":"arch","from":"cmd/node-tools"}]}}` lamp := `{"module":"lamp","version":"1","resources":[{"id":"service","type":"service","unit":"lamp.service","state":"running","boot":"enabled"}]}` - store := `{"module":"artifacts","version":"1","provides":[{"name":"artifact-store","scope":"mesh"}],"serves":{"artifact-store":{"port":5100}}}` + store := `{"module":"artifacts","version":"1","provides":[{"name":"artifact-store","scope":"mesh"}],"serves":{"artifact-store":{"port":5100}},` + + `"receives":{"artifact-store":"/var/lib/artifacts/received.json"}}` old, next := "src2:"+strings.Repeat("a", 64), "src2:"+strings.Repeat("b", 64) running := []delivery.Composed{{Node: "n1", Module: catalogue.RuntimeModule, FP: old, Resources: []json.RawMessage{ json.RawMessage(`{"id":"node-tools.runtime","module":"node-tools","type":"process","name":"node-tools","env":{"MESH_TOOL_MODULES":""}}`), @@ -94,6 +95,7 @@ func aRunningModuleWaitsForItsBuild(t *testing.T) { {name: "the trunk's build done", want: next, done: true, digest: dg("2")}, } { b, fps := aNode(t, store, tools, lamp) + b.held = true // every grant held, so a contribution's credential stands in for nothing b.own = []compose.OwnSecretRow{{Node: "n1", Module: "node-tools", Name: "broker", Origin: "made", SealedTo: "sk", Sealed: `"box"`}} mod := b.modules["node-tools"] mod.FP2 = next @@ -165,12 +167,13 @@ func aHeldModuleOfItsOwnAccountNeverRefusesTheNode(t *testing.T) { `"build":{"artifacts":[{"name":"runtime","kind":"bundle","language":"go","system":"arch","from":"cmd/node-tools"}]}}` own := func(name string) string { return `{"module":"` + name + `","version":"1","runs-as":"` + name + `","secrets-owner":"` + name + `","tools":["status"],` + - `"own-secrets":{"broker":"/var/lib/` + name + `/broker"},` + + `"own-secrets":{"broker":"/var/lib/` + name + `/broker"},"contributes":{"artifact-store":{"name":"` + name + `"}},` + `"resources":[{"id":"account","type":"user","name":"` + name + `"}],` + `"build":{"artifacts":[{"name":"tools","kind":"bundle","language":"typescript","entrypoints":["tools/index.js"]}]}}` } lamp := `{"module":"lamp","version":"1","resources":[{"id":"service","type":"service","unit":"lamp.service","state":"running","boot":"enabled"}]}` - store := `{"module":"artifacts","version":"1","provides":[{"name":"artifact-store","scope":"mesh"}],"serves":{"artifact-store":{"port":5100}}}` + store := `{"module":"artifacts","version":"1","provides":[{"name":"artifact-store","scope":"mesh"}],"serves":{"artifact-store":{"port":5100}},` + + `"receives":{"artifact-store":"/var/lib/artifacts/received.json"}}` for _, row := range []struct { name string issued []string // the own-account modules whose broker is issued @@ -185,6 +188,7 @@ func aHeldModuleOfItsOwnAccountNeverRefusesTheNode(t *testing.T) { held: map[string]string{"node-tools": "its build produced 0 bundle(s)", "check": "not here to run it from", "heal": "not here to run it from"}}, } { b, fps := aNode(t, store, tools, own("check"), lamp, own("heal")) + b.held = true // every grant held, so a contribution's credential stands in for nothing b.own = []compose.OwnSecretRow{{Node: "n1", Module: "node-tools", Name: "broker", Origin: "made", SealedTo: "sk", Sealed: `"box"`}} for _, m := range row.issued { b.own = append(b.own, compose.OwnSecretRow{Node: "n1", Module: m, Name: "broker", Origin: "made", SealedTo: "sk", Sealed: `"box"`}) @@ -208,6 +212,15 @@ func aHeldModuleOfItsOwnAccountNeverRefusesTheNode(t *testing.T) { t.Errorf("%s: the node is refused: %v", row.name, err) continue } + // A held module keeps what it contributes (issue 774, 2026-10-31: nextcloud held for its admin secret dropped out + // of postgres's received file, and postgres's helper would have locked its login): only its own secrets are left + // out of its stand-in, never its contributions, so the provider's received file names every consumer. + storeKey, _ := delivery.ComposedKey("n1", "artifacts", fps["artifacts"]) + for _, m := range []string{"check", "heal"} { + if got, _ := json.Marshal(out[storeKey].Resources); !strings.Contains(string(got), `\"from\": \"`+m+`\"`) { + t.Errorf("%s: the provider's received file drops %s's contribution: %s", row.name, m, got) + } + } for _, m := range []string{"node-tools", "check", "lamp", "heal"} { key, _ := delivery.ComposedKey("n1", m, fps[m]) _, composed := out[key] diff --git a/go/delivery/judge.go b/go/delivery/judge.go index 1aef5da..7da4d6c 100644 --- a/go/delivery/judge.go +++ b/go/delivery/judge.go @@ -107,7 +107,26 @@ func judged(from, to Composed) (Judgement, string) { // moving its data). It is said loudly in the words, and never hides an item that moved. if len(moved) > 0 { sort.Strings(moved) - words := "it changes where its data is kept: " + strings.Join(moved, ", ") + // A file the mesh makes for the module's provisions (what its consumers contribute, what it is bound to, the + // secret it is given) is said as the consumers that join or leave, not as its data moving (issue 774, + // 2026-10-31: postgres's yes said "where its data is kept" when nextcloud left its grants). Still a move: the + // yes it asks is what keeps a consumer's login while that consumer is held. + var kept, made []string + for _, id := range moved { + if who := consumersOf(id, before[id], after[id]); who != nil { + made = append(made, who...) + } else { + kept = append(kept, id) + } + } + var parts []string + if len(kept) > 0 { + parts = append(parts, "it changes where its data is kept: "+strings.Join(kept, ", ")) + } + if len(made) > 0 { + parts = append(parts, "consumers join or leave: "+strings.Join(made, ", ")) + } + words := strings.Join(parts, "; ") if from.Data != to.Data { words += fmt.Sprintf("; its data flag differs too (running %v, composed %v)", from.Data, to.Data) } @@ -120,6 +139,77 @@ func judged(from, to Composed) (Judgement, string) { return JudgedNeither, "it recreates no container and moves no data" } +// consumersOf is, for a changed file the mesh makes for a provision (received-, bound- or secret-, catalogue.ReceivedID, +// BoundID, SecretID), what changes of it in words: each consumer that joins or leaves a received file, by the given +// list's from and node; else that what it is given for the provision changes. Nil for any other resource. +func consumersOf(id string, before, after *judgedResource) []string { + _, local, _ := strings.Cut(id, ".") + r := after + if r == nil { + r = before + } + if r == nil || r.kind != string(declaration.TypeFile) { + return nil + } + if provision, ok := strings.CutPrefix(local, "received-"); ok { + was, now := givenIn(before), givenIn(after) + var out []string + for _, c := range sortedGiven(was) { + if !now[c] { + out = append(out, c+" leaves "+provision) + } + } + for _, c := range sortedGiven(now) { + if !was[c] { + out = append(out, c+" joins "+provision) + } + } + if len(out) == 0 { + out = append(out, "what "+provision+"'s consumers are given changes") + } + return out + } + for _, prefix := range []string{"bound-", "secret-"} { + if provision, ok := strings.CutPrefix(local, prefix); ok { + return []string{"what it is given for " + provision + " changes"} + } + } + return nil +} + +// givenIn is the consumers a received file's content names, each as " on ". +func givenIn(r *judgedResource) map[string]bool { + out := map[string]bool{} + if r == nil { + return out + } + var file struct { + Content string `json:"content"` + } + var received struct { + Given []struct { + From string `json:"from"` + Node string `json:"node"` + } `json:"given"` + } + if json.Unmarshal(r.raw, &file) != nil || json.Unmarshal([]byte(file.Content), &received) != nil { + return out + } + for _, g := range received.Given { + out[g.From+" on "+g.Node] = true + } + return out +} + +func sortedGiven(m map[string]bool) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + sort.Strings(out) + return out +} + // judgedResource is a resource as the judgement reads it: its kind, the path it acts on, what its containers mount, // and its canonical bytes. type judgedResource struct { diff --git a/go/delivery/judge_test.go b/go/delivery/judge_test.go index 2ffa7c1..39545ec 100644 --- a/go/delivery/judge_test.go +++ b/go/delivery/judge_test.go @@ -28,6 +28,8 @@ func TestJudgeMove(t *testing.T) { config2 = `{"id":"mesh-mail.conf","type":"file","module":"mesh-mail","path":"/var/lib/mesh-mail/spool/conf","content":"b"}` process = `{"id":"mesh-mail.daemon","type":"process","module":"mesh-mail","exec":"/usr/bin/a"}` process2 = `{"id":"mesh-mail.daemon","type":"process","module":"mesh-mail","exec":"/usr/bin/b"}` + received = `{"id":"mesh-mail.received-smtp","type":"file","module":"mesh-mail","path":"/var/lib/mesh-mail/spool/received.json","content":"{\"given\":[{\"from\":\"cloud\",\"node\":\"novox\"},{\"from\":\"photos\",\"node\":\"novox\"}]}"}` + received2 = `{"id":"mesh-mail.received-smtp","type":"file","module":"mesh-mail","path":"/var/lib/mesh-mail/spool/received.json","content":"{\"given\":[{\"from\":\"photos\",\"node\":\"novox\"},{\"from\":\"wiki\",\"node\":\"ace\"}]}"}` users = `{"id":"nats.bus-users","type":"file","module":"nats","path":"/etc/nats/users","content":"a"}` users2 = `{"id":"nats.bus-users","type":"file","module":"nats","path":"/etc/nats/users","content":"b"}` natsConf = `{"id":"nats.conf","type":"file","module":"nats","path":"/etc/nats/conf","content":"a"}` @@ -43,25 +45,35 @@ func TestJudgeMove(t *testing.T) { from, to *Composed want Judgement userList bool + words string // what the words must say, when given }{ - {"a new image", mail('a', true, container, spool, tools), mail('b', true, container2, spool, tools), JudgedRecreates, false}, - {"a container gone", mail('a', true, container, spool), mail('b', true, spool), JudgedRecreates, false}, - {"its spool moved", mail('a', true, container, spool), mail('b', true, container, spoolMoved), JudgedMovesData, false}, - {"a file in its spool", mail('a', true, container, spool, config), mail('b', true, container, spool, config2), JudgedMovesData, false}, - {"its data flag alone", mail('a', false, container, spool), mail('b', true, container, spool), JudgedNeither, false}, - {"its data flag and its spool", mail('a', false, container, spool), mail('b', true, container, spoolMoved), JudgedMovesData, false}, - {"a process of a module with data", mail('a', true, spool, process), mail('b', true, spool, process2), JudgedMovesData, false}, - {"a process of a module without", mail('a', false, process), mail('b', false, process2), JudgedNeither, false}, - {"its tools only", mail('a', true, container, spool, tools), mail('b', true, container, spool, tools2), JudgedNeither, false}, - {"never ran", nil, mail('b', true, container), JudgedNeither, false}, - {"equal, written otherwise", mail('a', true, `{"type":"directory","id":"mesh-mail.spool","module":"mesh-mail","path":"/var/lib/mesh-mail/spool"}`), mail('b', true, spool), JudgedUnchanged, false}, - {"only the user list", bus(natsConf, users), bus(natsConf, users2), JudgedNeither, true}, - {"the user list and more", bus(natsConf, users), bus(natsConf2, users2), JudgedNeither, false}, + {"a new image", mail('a', true, container, spool, tools), mail('b', true, container2, spool, tools), JudgedRecreates, false, ""}, + {"a container gone", mail('a', true, container, spool), mail('b', true, spool), JudgedRecreates, false, ""}, + {"its spool moved", mail('a', true, container, spool), mail('b', true, container, spoolMoved), JudgedMovesData, false, ""}, + {"a file in its spool", mail('a', true, container, spool, config), mail('b', true, container, spool, config2), JudgedMovesData, false, "it changes where its data is kept: mesh-mail.conf"}, + // A received file the mesh makes, in its data's place, names its consumers joining or leaving, not its data moving + // (issue 774, 2026-10-31: postgres asked a yes for "where its data is kept" when nextcloud left its grants). + {"a consumer leaves", mail('a', true, container, spool, received), mail('b', true, container, spool, received2), JudgedMovesData, false, + "consumers join or leave: cloud on novox leaves smtp, wiki on ace joins smtp"}, + {"a consumer leaves and a file moves", mail('a', true, container, spool, received, config), mail('b', true, container, spool, received2, config2), JudgedMovesData, false, + "it changes where its data is kept: mesh-mail.conf; consumers join or leave: cloud on novox leaves smtp, wiki on ace joins smtp"}, + {"its data flag alone", mail('a', false, container, spool), mail('b', true, container, spool), JudgedNeither, false, ""}, + {"its data flag and its spool", mail('a', false, container, spool), mail('b', true, container, spoolMoved), JudgedMovesData, false, ""}, + {"a process of a module with data", mail('a', true, spool, process), mail('b', true, spool, process2), JudgedMovesData, false, ""}, + {"a process of a module without", mail('a', false, process), mail('b', false, process2), JudgedNeither, false, ""}, + {"its tools only", mail('a', true, container, spool, tools), mail('b', true, container, spool, tools2), JudgedNeither, false, ""}, + {"never ran", nil, mail('b', true, container), JudgedNeither, false, ""}, + {"equal, written otherwise", mail('a', true, `{"type":"directory","id":"mesh-mail.spool","module":"mesh-mail","path":"/var/lib/mesh-mail/spool"}`), mail('b', true, spool), JudgedUnchanged, false, ""}, + {"only the user list", bus(natsConf, users), bus(natsConf, users2), JudgedNeither, true, ""}, + {"the user list and more", bus(natsConf, users), bus(natsConf2, users2), JudgedNeither, false, ""}, } { got := JudgeMove(c.from, *c.to) if got.Judgement != c.want || got.UserListOnly != c.userList || got.Words == "" { t.Errorf("%s: %+v, want %s (user list only %v)", c.name, got, c.want, c.userList) } + if c.words != "" && got.Words != c.words { + t.Errorf("%s: says %q, want %q", c.name, got.Words, c.words) + } if strings.Contains(c.name, "data flag alone") && !strings.Contains(got.Words, "WARNING: its data flag differs") { t.Errorf("%s: the flag's difference is not said: %q", c.name, got.Words) }