diff --git a/go/compose/frombus/answer.go b/go/compose/frombus/answer.go index bb665eb..fc57937 100644 --- a/go/compose/frombus/answer.go +++ b/go/compose/frombus/answer.go @@ -3,6 +3,7 @@ package frombus import ( "context" "fmt" + "maps" "regexp" "slices" "sort" @@ -70,7 +71,7 @@ func Answer(ctx context.Context, r Reader, mesh Mesh, s Scope, fps map[string]st return refuse(fmt.Errorf("frombus: the credentials of %s: %w", s.Node, err)) } } - composed, skipped, stood, err := composeEach(in, s.Node, fps, false, s.Running, failed) + composed, skipped, stood, err := composeEach(in, s.Node, fps, false, s.Running, failed, ranOf(ctx, r, mesh, s.Running, fps)) if err != nil { // Not one module's: the node's own (a value served elsewhere, the mesh's network), said once as the node's. if len(failed) == 0 { @@ -207,59 +208,24 @@ func Answer(ctx context.Context, r Reader, mesh Mesh, s Scope, fps map[string]st return a } -// heldBehind is why a module is held with a held one it reads (composeEach's dependents). -func heldBehind(m string) string { - return "it reads what " + m + " contributes or listens on, and " + m + " is held" -} - -// behind is the held module a refusal's words say it is held behind, or "". -func behind(why string) string { - rest, ok := strings.CutPrefix(why, "it reads what ") - if !ok { - return "" - } - m, _, ok := strings.Cut(rest, " contributes or listens on, and ") - if !ok || why != heldBehind(m) { - return "" - } - return m -} - // heldAsToday moves into Held each refusal of a module the node keeps today as it runs (Scope.HeldToday, with how: -// held, failed, running a build of the old form), and of each module held only behind one of those: the candidate keeps -// them as the node does, which is no change (issue 1126: a seed reproduces today, and today keeps them so). Each says -// how it stands today (ModuleRefused.Today). +// held, failed, running a build of the old form): the candidate keeps it as the node does, which is no change (issue +// 1126: a seed reproduces today, and today keeps them so). Each says how it stands today (ModuleRefused.Today). No +// module is held behind another (issue 1960): a module that reads from a held one composes with what it last offered. func heldAsToday(a *delivery.CandidateAnswer, today map[string]string) { if len(today) == 0 { return } - how := map[string]string{} - for m, w := range today { - how[m] = w - } - for moved := true; moved; { - moved = false - kept := a.Refusals[:0] - for _, r := range a.Refusals { - w, keep := how[r.Module] - if !keep { - if m := behind(r.Why); m != "" { - if _, behindKept := how[m]; behindKept { - w, keep = "held behind "+m, true - } - } - } - if keep { - r.Today = w - how[r.Module] = w - a.Held = append(a.Held, r) - moved = true - } else { - kept = append(kept, r) - } + kept := a.Refusals[:0] + for _, r := range a.Refusals { + if w, keep := today[r.Module]; keep { + r.Today = w + a.Held = append(a.Held, r) + } else { + kept = append(kept, r) } - a.Refusals = kept } + a.Refusals = kept if len(a.Refusals) == 0 { a.Refusals = nil } @@ -310,15 +276,18 @@ func standsInFor(x, node string) (string, string) { // **A held module is what runs of it** (review of the apply, round 1): composed from what running holds of it // (compose.Carried of its own composition there; the node's own carries it wherever it runs there), so what the others // read of it (the machine ports its containers publish, the routes reaching it) is what runs, and none of its own -// composition is in out: the node keeps what it last applied. **A held module is no dependence of its neighbours** -// (issue 1126, the spreading hold): a consumer of what it provides, or a collector of its items, composes against what -// runs of it like any other module, and is never held for how the held one would compose as its stub (that comparison -// held every postgres consumer behind de-spiegel's hold on novox, three rounds in a row). What it composes is compared -// with what runs as any module's is: a real difference is still named, and a refusal of its own still refuses it. Only -// what a held module's new manifest says to others, which may not be the build's that runs, is held with it -// (dependents): the providers of what it contributes to in their own files (the proxy its routes), and, when it -// listens, the filter; its items reach their collectors through its own plan, which it keeps as it runs. -func composeEach(in compose.Input, node string, fps map[string]string, applied bool, running []delivery.Composed, failed map[string]string) (out map[string]delivery.Composed, skipped, stood []string, err error) { +// composition is in out: the node keeps what it last applied. +// +// **A hold never spreads** (issue 1126, issue 1960: de-spiegel's hold for an unset value held mailu, which held +// route-proxy, nftables and postgres on novox). No module is held because a module it reads from is held: a consumer, +// a collector, the provider of what the held one contributes to and the filter where it listens each compose with what +// the held module last offered. That offer (what it contributes, contributes many, listens on) is its manifest at the +// build it runs where one is known (ran: a composition of its own running at another build than it is composed at), +// else the manifest it is composed from. Only the part of the offer that cannot compose is left out, by provision +// (culprit): de-spiegel's routes, its names unset, leave route-proxy without them while mailu keeps its smtp account, +// and the hold says so. +func composeEach(in compose.Input, node string, fps map[string]string, applied bool, running []delivery.Composed, failed map[string]string, + ran func(module string) (catalogue.Manifest, bool)) (out map[string]delivery.Composed, skipped, stood []string, err error) { mode := compose.Foreseeing if applied { mode = compose.Reading @@ -329,43 +298,46 @@ func composeEach(in compose.Input, node string, fps map[string]string, applied b runs[m] = &running[i] } } - src := map[string]catalogue.Manifest{} // each held module's manifest before it was held - bareOf := map[string]bool{} - var hold func(m, why string, bare bool) - hold = func(m, why string, bare bool) { + src := map[string]catalogue.Manifest{} // each held module's manifest before it was held, offering what it last offered + left := map[string]map[string]bool{} // by held module, what of its offer is left out (leaveOut) + hold := func(m, why, leave string) { if _, done := failed[m]; !done { - failed[m] = why // a held module tried bare keeps the words it was first held for + failed[m] = why // a held module tried again keeps the words it was first held for } - first := false if _, ok := src[m]; !ok { man, ok := manifestOn(in, node, m) if !ok { return } - src[m], first = man, true + if ran != nil { + if was, ok := ran(m); ok { + man.Contributes, man.ContributesMany, man.Listens = was.Contributes, was.ContributesMany, was.Listens + } + } + src[m] = man } - bareOf[m] = bareOf[m] || bare - in = heldAs(in, node, m, src[m], bareOf[m], runs[m]) - if !first { - return - } - // What the others read of its manifest (what it contributes, what it listens on) is its new manifest's, not - // necessarily the build's that runs: what reads it is held with it (review of the apply, round 2). - for _, r := range dependents(in, node, src[m], failed) { - hold(r, heldBehind(m), false) + if leave != "" { + if left[m] == nil { + left[m] = map[string]bool{} + } + left[m][leave] = true + if leave != leaveAll { + failed[m] += "; composed without what it contributes to " + leave + ", which does not compose" + } } + in = heldAs(in, node, m, src[m], left[m], runs[m]) } for _, m := range sortedKeys(failed) { - hold(m, failed[m], false) + hold(m, failed[m], "") } for { out, skipped, stood, err = compose.CompositionsIn(in, node, fps, mode) if err != nil { - module, bare := culprit(in, node, fps, mode, failed, err) + module, leave := culprit(in, node, fps, mode, failed, err) if module == "" { return nil, nil, nil, err } - hold(module, err.Error(), bare) + hold(module, err.Error(), leave) continue } again := false @@ -385,7 +357,7 @@ func composeEach(in compose.Input, node string, fps map[string]string, applied b } for _, m := range sortedKeys(of) { if _, done := failed[m]; !done { - hold(m, "stands in for "+strings.Join(of[m], ", "), false) + hold(m, "stands in for "+strings.Join(of[m], ", "), "") again = true } } @@ -401,43 +373,9 @@ func composeEach(in compose.Input, node string, fps map[string]string, applied b return out, skipped, stood, nil } -// dependents is, of the modules placed on node and not held, those that read what man says to them: each that provides -// what man contributes to (the proxy its routes), and, when man listens on a port, each that holds the filter (the -// openings). -func dependents(in compose.Input, node string, man catalogue.Manifest, failed map[string]string) []string { - to := map[string]bool{} - for p := range man.Contributes { - to[p] = true - } - for p := range man.ContributesMany { - to[p] = true - } - listens := len(man.Listens) > 0 - if len(to) == 0 && !listens { - return nil - } - var out []string - for _, m := range assignedOn(in, node) { - if _, held := failed[m]; held || m == man.Module { - continue - } - pm, ok := manifestOn(in, node, m) - if !ok { - continue - } - if listens && pm.Filtering != nil { - out = append(out, m) - continue - } - for _, o := range pm.Provides { - if to[o.Name] { - out = append(out, m) - break - } - } - } - return out -} +// 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 = "*" // assignedOn is the modules placed on node, in placement order. func assignedOn(in compose.Input, node string) []string { @@ -462,18 +400,14 @@ func manifestOn(in compose.Input, node, module string) (catalogue.Manifest, bool return man, ok } -// culprit is the module placed on node whose composition the error is of: the first, in placement order, whose stub -// (compose.Stub) makes the composition in mode succeed or fail otherwise; empty when none does (the error is not one -// module's). Modules already refused are passed over. A module whose stub still fails the same is tried bare (its stub -// contributing nothing, bare true): a refusal of what it contributes is its own, which the stub keeps (issue 1126, -// seed-8: de-spiegel's route names on novox, an unset required value, refused the whole node). -func culprit(in compose.Input, node string, fps map[string]string, mode compose.Mode, failed map[string]string, err error) (string, bool) { - var assigned []string - for _, n := range in.Nodes { - if n.Name == node { - assigned = n.Assigned - } - } +// culprit is the module placed on node whose composition the error is of, and what of its offer to leave out: the +// first, in placement order, whose stub (compose.Stub) makes the composition in mode succeed or fail otherwise (leave +// ""); else the first whose stub without what it contributes to one provision does (leave that provision: de-spiegel's +// routes, its names unset, and not its smtp account, issue 1960); else the first whose bare stub does (leaveAll: its +// stub contributing nothing, its own secret never issued, issue 1126 seed-8). Empty when none does (the error is not +// one module's). Modules already refused are passed over for their stub. +func culprit(in compose.Input, node string, fps map[string]string, mode compose.Mode, failed map[string]string, err error) (string, string) { + assigned := assignedOn(in, node) // Only the modules the error names: the words of a module's refusal name it. Each try is a whole composition, so a // refusal naming none is no module's, answered at once rather than after a composition per module (issue 1126: ace's // answer took 7m46s). @@ -483,65 +417,97 @@ func culprit(in compose.Input, node string, fps map[string]string, mode compose. order = append(order, m) } } - changes := func(m string, bare bool) bool { - _, _, _, again := compose.CompositionsIn(stubbed(in, node, m, bare), node, fps, mode) + changes := func(m, leave string) bool { + _, _, _, again := compose.CompositionsIn(stubbed(in, node, m, leave), node, fps, mode) return again == nil || again.Error() != err.Error() } for _, m := range order { if _, done := failed[m]; done { continue } - if changes(m, false) { - return m, false - } - } - var kept map[string]catalogue.Manifest - for _, n := range in.Nodes { - if n.Name == node { - kept = n.Kept + if changes(m, "") { + return m, "" } } for _, m := range order { - // One already held is tried bare too, once: held for its build, its stub may still refuse (its own secret - // never issued, issue 1126). - if _, done := failed[m]; done && isBare(kept[m]) { + man, ok := manifestOn(in, node, m) + if !ok { continue } - if changes(m, true) { - return m, true + // One already held is tried again too: held for its build, its stub may still refuse (its own secret never + // issued, issue 1126). A provision named in the error is tried first, each at one more composition. + var to []string + for p := range man.Contributes { + to = append(to, p) + } + for p := range man.ContributesMany { + if _, both := man.Contributes[p]; !both { + to = append(to, p) + } + } + sort.SliceStable(to, func(i, j int) bool { + return strings.Contains(err.Error(), " "+to[i]) && !strings.Contains(err.Error(), " "+to[j]) + }) + for _, p := range to { + if changes(m, p) { + return m, p + } + } + if isBare(man) { + continue + } + if changes(m, leaveAll) { + return m, leaveAll } } - return "", false + return "", "" } -// isBare says a stub contributes nothing and declares no own secret (stubbed, bare). +// isBare says a stub contributes nothing and declares no own secret (stubbed, leaveAll). func isBare(m catalogue.Manifest) bool { return len(m.Contributes) == 0 && len(m.ContributesMany) == 0 && len(m.OwnSecrets) == 0 } // stubbed is in with module held on node as its stub alone (heldAs, nothing of it running), from the manifest node -// composes it from: a culprit's try. -func stubbed(in compose.Input, node, module string, bare bool) compose.Input { +// composes it from, leave left out of its offer: a culprit's try. +func stubbed(in compose.Input, node, module, leave string) compose.Input { man, ok := manifestOn(in, node, module) if !ok { return in } - return heldAs(in, node, module, man, bare, nil) + var left map[string]bool + if leave != "" { + left = map[string]bool{leave: true} + } + return heldAs(in, node, module, man, left, nil) } // 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; bare, contributing nothing to -// what it requires and declaring no own secret: none of a held module's own composition is 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, bare bool, runs *delivery.Composed) compose.Input { +// 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. +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...) for i := range out.Nodes { if out.Nodes[i].Name != node { continue } - if bare { + if left[leaveAll] { man.Contributes, man.ContributesMany, man.OwnSecrets = nil, nil, nil + } else if len(left) > 0 { + contributes := map[string]map[string]any{} + for p, v := range man.Contributes { + if !left[p] { + contributes[p] = v + } + } + many := maps.Clone(man.ContributesMany) + for p := range left { + delete(many, p) + } + man.Contributes, man.ContributesMany = contributes, many } kept := make(map[string]catalogue.Manifest, len(out.Nodes[i].Kept)+1) for k, v := range out.Nodes[i].Kept { diff --git a/go/compose/frombus/answer_rows_test.go b/go/compose/frombus/answer_rows_test.go index 87fe2c3..6eb01a2 100644 --- a/go/compose/frombus/answer_rows_test.go +++ b/go/compose/frombus/answer_rows_test.go @@ -104,12 +104,12 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { if part.Verdict != delivery.CandidateRefused || len(part.Refusals) != 2 || len(part.Held) != 1 || part.Held[0].Module != "api" { t.Errorf("held as today in part: %+v", part) } - // A module held only behind one kept today is kept as today too, saying behind which (novox's mailu behind de-spiegel). + // No module is held behind one kept today (issue 1960): mailu's own refusal stays a refusal beside de-spiegel's hold. if a := (delivery.CandidateAnswer{Refusals: []delivery.ModuleRefused{{Module: "de-spiegel", Why: "a value is unset"}, - {Module: "mailu", Why: heldBehind("de-spiegel")}, {Module: "proxy", Why: heldBehind("mailu")}, {Module: "web", Why: "no build"}}}); true { + {Module: "mailu", Why: "no build"}}}); true { heldAsToday(&a, map[string]string{"de-spiegel": "runs a build of the old form"}) - if len(a.Refusals) != 1 || a.Refusals[0].Module != "web" || len(a.Held) != 3 || a.Held[2].Today != "held behind mailu" { - t.Errorf("held behind a module kept today: %+v", a) + if len(a.Refusals) != 1 || a.Refusals[0].Module != "mailu" || len(a.Held) != 1 { + t.Errorf("held as today spreads: %+v", a) } } @@ -330,23 +330,26 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { // de-spiegel folded, and its manifest at the indexer's commit (e25e1beb, captured) asks four required config values // for its route names that nothing sets. novox answered the whole candidate refused ("de-spiegel contributing // praktijk to route says ${setting:praktijk-name} ...": its stub still contributed the route). Only de-spiegel is - // refused, in the config form's words, and proxy, which receives its routes (what it would compose lacks them: review - // of the apply, round 1); the rest is judged. + // refused, in the config form's words; the rest is judged. Issue 1960, novox's chain on 2026-10-29: the hold spread + // to mailu (its smtp provider) and proxy (its route provider), and from them on. Neither is held now: proxy composes + // without de-spiegel's routes, which do not compose, and mailu with its smtp account, which does. spiegel, err := os.ReadFile("testdata/de-spiegel-e25e1beb.module.json") if err != nil { t.Fatal(err) } - proxy := []byte(`{"module":"proxy","version":"1","capabilities":["systemd"],"provides":[{"name":"route","scope":"mesh"},{"name":"smtp","scope":"mesh"}],` + + proxy := []byte(`{"module":"proxy","version":"1","capabilities":["systemd"],"provides":[{"name":"route","scope":"mesh"}],` + `"resources":[{"id":"service","type":"service","unit":"proxy.service","state":"running","boot":"enabled"}]}`) - b = mk("lemurs", overlay.Name, "proxy", "de-spiegel") - for m, raw := range map[string][]byte{"de-spiegel": spiegel, "proxy": proxy} { + mailu := []byte(`{"module":"mailu","version":"1","capabilities":["systemd"],"provides":[{"name":"smtp","scope":"mesh"}],` + + `"resources":[{"id":"service","type":"service","unit":"mailu.service","state":"running","boot":"enabled"}]}`) + b = mk("lemurs", overlay.Name, "mailu", "proxy", "de-spiegel") + for m, raw := range map[string][]byte{"de-spiegel": spiegel, "proxy": proxy, "mailu": mailu} { b.manifests[delivery.ManifestDigest(raw)] = raw b.modules[m] = delivery.Module{Manifest: delivery.ManifestDigest(raw)} } p = b.placements["n1"] p.Decided.Endpoint = "n1.example:51820" b.placements["n1"] = p - fps = map[string]string{"lemurs": fp("e"), "proxy": fp("f")} + fps = map[string]string{"lemurs": fp("e"), "proxy": fp("f"), "mailu": fp("d")} a = Answer(ctx, b, Mesh{}, Scope{Node: "n1", Placements: given(b), InOwn: []string{"de-spiegel"}}, fps, nil, "p-h1", 3) refused = nil for _, r := range a.Refusals { @@ -355,11 +358,13 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { t.Errorf("de-spiegel is refused in other words than its config form's: %s", r.Why) } } - if a.Refused != "" || !reflect.DeepEqual(refused, []string{"de-spiegel", "proxy"}) { - t.Errorf("a module's own refusal: %+v, want de-spiegel refused with proxy, which receives its routes, the node not", a) + if a.Refused != "" || !reflect.DeepEqual(refused, []string{"de-spiegel"}) { + t.Errorf("a module's own refusal: %+v, want de-spiegel refused alone, the node, mailu and proxy not", a) } - if !slices.ContainsFunc(a.Added, func(m delivery.ModuleAdded) bool { return m.Module == "lemurs" }) { - t.Errorf("lemurs, which composes, is not judged beside de-spiegel's refusal: %+v", a) + for _, m := range []string{"lemurs", "mailu", "proxy"} { + if !slices.ContainsFunc(a.Added, func(x delivery.ModuleAdded) bool { return x.Module == m }) { + t.Errorf("%s, which composes, is not judged beside de-spiegel's refusal: %+v", m, a) + } } // (E) Issue 1126, the applying path (review of the seed): the composition novox applies follows the same rule. It @@ -393,16 +398,13 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { if err != nil { t.Fatalf("de-spiegel's own refusal refuses the node's whole composition: %v", err) } - if len(held) != 2 || !strings.Contains(held["de-spiegel"], "${config:praktijk-name}") || !strings.Contains(held["proxy"], "de-spiegel") { - t.Errorf("held %v, want de-spiegel in its config form's words, and proxy for its routes", held) + if len(held) != 1 || !strings.Contains(held["de-spiegel"], "${config:praktijk-name}") || !strings.Contains(held["de-spiegel"], "without what it contributes to route") { + t.Errorf("held %v, want de-spiegel alone, in its config form's words, composed without its routes", held) } - lemursKey, _ := delivery.ComposedKey("n1", "lemurs", fps["lemurs"]) - proxyKey, _ := delivery.ComposedKey("n1", "proxy", fps["proxy"]) - if _, ok := got[lemursKey]; !ok { - t.Errorf("lemurs, which composes, is not composed beside de-spiegel's refusal: %v", sortedKeys(got)) - } - if _, ok := got[proxyKey]; ok { - t.Errorf("proxy is composed without the routes of held de-spiegel: %v", sortedKeys(got)) + for _, m := range []string{"lemurs", "mailu", "proxy"} { + if key, _ := delivery.ComposedKey("n1", m, fps[m]); got[key].Module != m { + t.Errorf("%s is not composed beside de-spiegel's hold: %v", m, sortedKeys(got)) + } } carries := func(c delivery.Composed, what string) { t.Helper() @@ -434,7 +436,7 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { // (G) A held module that runs a composition of its own (review of the apply, round 1, finding 2): what the others // compose of it is what runs, never its stub's. site listens on 8080, placed at the machine's 20005, and its own // secret was never issued here: held, the filter must not open 8080 and drop the 20005 that runs. The filter, which - // reads what site's manifest listens on, is held with it (dependents). + // reads what site listens on, composes with it (issue 1960: a hold never spreads). site := []byte(`{"module":"site","version":"1","capabilities":["container-runtime"],` + `"listens":[{"port":8080,"from":"mesh","why":"the site"}],"own-secrets":{"key":"${dir:state}/key"},` + `"resources":[{"id":"state","type":"directory","path":"/var/lib/site","mode":"0700"},` + @@ -460,8 +462,12 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { json.RawMessage(`{"id":"site.state","module":"site","type":"directory","path":"/var/lib/site","mode":"0700"}`), json.RawMessage(`{"id":"site.server","module":"site","type":"container","name":"site","image":"registry.example/site@sha256:` + strings.Repeat("c", 64) + `","ports":["10.77.0.1:20005:8080"]}`)}} got, _, held, err = Compose(ctx, b, Mesh{}, Scope{Node: "n1", Running: []delivery.Composed{siteRuns}}, fps) - if err != nil || held["site"] == "" || held["nft"] == "" { - t.Fatalf("site, its own secret never issued: %v, held %v; want site held, and the filter, which depends on it", err, held) + nftKey, _ := delivery.ComposedKey("n1", "nft", fps["nft"]) + if err != nil || held["site"] == "" || held["nft"] != "" || got[nftKey].Module != "nft" { + t.Fatalf("site, its own secret never issued: %v, held %v; want site held alone, and the filter composed", err, held) + } + if rules, _ := json.Marshal(got[nftKey].Resources); !strings.Contains(string(rules), "dport 20005") { + t.Errorf("the filter drops held site's opening where it runs (20005): %s", rules) } for key, c := range got { for _, raw := range c.Resources { @@ -501,26 +507,25 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { t.Errorf("app's binding does not reach held site where it runs (20005): %s", bound) } delete(fps, "app") - // A held feeder of a collector holds no collector: its items reach their collection through its own plan, which it - // keeps as it runs. Only a provider it contributes to in that provider's own files is held behind it (the proxy its - // routes), as is the filter where it listens. - withCollector := compose.Input{Catalogue: map[string]catalogue.Manifest{ - "feeder": {Module: "feeder", Items: []catalogue.Item{{Collection: "routes", Content: "x"}}}, - "col": {Module: "col", Collections: []catalogue.Collection{{Name: "routes"}}}, - "proxy": {Module: "proxy", Provides: []catalogue.Offer{{Name: "route"}}}}, - Nodes: []compose.Node{{Name: "n1", Assigned: []string{"feeder", "col", "proxy"}}}} - feeder := withCollector.Catalogue["feeder"] - if d := dependents(withCollector, "n1", feeder, map[string]string{"feeder": "held"}); len(d) != 0 { - t.Errorf("a held feeder of a collector holds %v, want none", d) - } - feeder.Contributes = map[string]map[string]any{"route": {"name": "x"}} - if d := dependents(withCollector, "n1", feeder, map[string]string{"feeder": "held"}); !reflect.DeepEqual(d, []string{"proxy"}) { - t.Errorf("a held module contributing routes holds %v, want the proxy", d) + // A held module's offer is left out by provision, never whole when one part composes (issue 1960): de-spiegel's + // routes, its names unset, leave the proxy without them, and its smtp account stays with mailu. + withProxy := compose.Input{Catalogue: map[string]catalogue.Manifest{}, + Nodes: []compose.Node{{Name: "n1", Assigned: []string{"feeder"}}}} + feeder := catalogue.Manifest{Module: "feeder", Contributes: map[string]map[string]any{"route": {"name": "x"}, "smtp": {"account": "feeder"}}, + OwnSecrets: catalogue.OwnSecrets{"key": {Path: "/k"}}} + 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}} { + 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) + } } // (H) Review of the apply, round 2: a held module is composed from its new manifest, so what the others read of its // manifest (its listens, what it contributes) may not be what runs. site's new manifest moves it to 9090 while the - // build that runs listens on 8080: the filter, the holder of what site listens on, is held with it, and nothing - // applied opens 9090. + // build that runs (7) listens on 8080. The filter composes with what site last offered, its manifest at the build + // that runs (issue 1960: a hold never spreads), so it opens 8080 and nothing applied opens 9090. moved := []byte(strings.Replace(string(site), `"port":8080`, `"port":9090`, 1)) b.manifests[delivery.ManifestDigest(moved)] = moved b.modules["site"] = delivery.Module{Manifest: delivery.ManifestDigest(moved)} @@ -529,9 +534,15 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { } b.placements["n1"] = p siteRuns.Resources[1] = json.RawMessage(`{"id":"site.server","module":"site","type":"container","name":"site","image":"registry.example/site@sha256:` + strings.Repeat("c", 64) + `","ports":["8080"]}`) + siteRuns.FP = fp("7") + b.manifests[delivery.ManifestDigest(site)] = site + b.builds["site "+fp("7")] = &delivery.BuilderBuild{State: delivery.BuildDone, Manifest: delivery.ManifestDigest(site)} got, _, held, err = Compose(ctx, b, Mesh{}, Scope{Node: "n1", Running: []delivery.Composed{siteRuns}}, fps) - if err != nil || held["site"] == "" || held["nft"] == "" { - t.Errorf("site moved to 9090 in its new manifest: %v, held %v; want site held, and the filter with it", err, held) + if err != nil || held["site"] == "" || held["nft"] != "" { + t.Errorf("site moved to 9090 in its new manifest: %v, held %v; want site held alone", err, held) + } + if rules, _ := json.Marshal(got[nftKey].Resources); !strings.Contains(string(rules), "dport 8080") { + t.Errorf("the filter drops what held site last offered (8080): %s", rules) } for key, c := range got { for _, raw := range c.Resources { diff --git a/go/compose/frombus/frombus.go b/go/compose/frombus/frombus.go index d034a48..632d117 100644 --- a/go/compose/frombus/frombus.go +++ b/go/compose/frombus/frombus.go @@ -240,12 +240,7 @@ func input(ctx context.Context, r Reader, mesh Mesh, s Scope, refuse func(module in.Authority = delivery.AuthorityBundle(in.Authority, vault) } } - parse := func(raw []byte) (catalogue.Manifest, error) { - if mesh.Predefined != nil { - return catalogue.ParseManifestWith(raw, mesh.Predefined) - } - return catalogue.ParseManifestForInputs(raw) - } + parse := parser(mesh) read := func(module, digest string) (catalogue.Manifest, error) { raw, err := r.Manifest(ctx, digest) if err != nil { @@ -425,10 +420,47 @@ func Compose(ctx context.Context, r Reader, mesh Mesh, s Scope, fps map[string]s return nil, nil, held, fmt.Errorf("frombus: the credentials of %s: %w", s.Node, err) } } - out, skipped, _, err = composeEach(in, s.Node, fps, true, s.Running, held) + out, skipped, _, err = composeEach(in, s.Node, fps, true, s.Running, held, ranOf(ctx, r, mesh, s.Running, fps)) return out, skipped, held, err } +// parser reads a manifest as the indexer does, against the mesh's predefined keys when given. +func parser(mesh Mesh) func([]byte) (catalogue.Manifest, error) { + return func(raw []byte) (catalogue.Manifest, error) { + if mesh.Predefined != nil { + return catalogue.ParseManifestWith(raw, mesh.Predefined) + } + return catalogue.ParseManifestForInputs(raw) + } +} + +// ranOf is what a module last offered the others, read when it is held (composeEach, issue 1960): its manifest at the +// build its own composition runs at (running), where that is another build than the one it is composed at (fps) and +// the builder holds it done. False where nothing is known of it (it runs no composition of its own, it runs the build +// it is composed at, or its build is unread): the manifest it is composed from is its offer then. Only what it says to +// others is taken of it, which needs no take-in of its artifacts. +func ranOf(ctx context.Context, r Reader, mesh Mesh, running []delivery.Composed, fps map[string]string) func(string) (catalogue.Manifest, bool) { + parse := parser(mesh) + return func(module string) (catalogue.Manifest, bool) { + for _, c := range running { + if c.Module != module || c.FP == "" || c.FP == fps[module] { + continue + } + b, err := r.Build(ctx, module, c.FP) + if err != nil || b == nil || b.State != delivery.BuildDone || b.Manifest == "" { + return catalogue.Manifest{}, false + } + raw, err := r.Manifest(ctx, b.Manifest) + if err != nil { + return catalogue.Manifest{}, false + } + man, err := parse(raw) + return man, err == nil + } + return catalogue.Manifest{}, false + } +} + // fromBuilds puts in the built manifest of each module placed on the composing node, at the trunk's FP2, and of each // module it keeps at another build (Scope.KeptAt, into its node's Kept), read from the builder's done key and resolved // by the one take-in rule (takein.Manifest), a templates archive read in the artifact store as the node reaches it