diff --git a/go/compose/frombus/answer.go b/go/compose/frombus/answer.go index 078dbbc..0f0bfe9 100644 --- a/go/compose/frombus/answer.go +++ b/go/compose/frombus/answer.go @@ -72,7 +72,7 @@ func Answer(ctx context.Context, r Reader, mesh Mesh, s Scope, fps map[string]st } composed, skipped, stood, err := compose.CompositionsShadow(in, s.Node, fps) for err != nil { - module := culprit(in, s.Node, fps, failed, err) + module, bare := culprit(in, s.Node, fps, failed, err) if module == "" { // 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 { @@ -83,7 +83,7 @@ func Answer(ctx context.Context, r Reader, mesh Mesh, s Scope, fps map[string]st break } failed[module] = err.Error() - in = stubbed(in, s.Node, module) + in = stubbed(in, s.Node, module, bare) composed, skipped, stood, err = compose.CompositionsShadow(in, s.Node, fps) } // The node's own composition is judged only where the node runs one: one that runs none applies it only once a want @@ -246,8 +246,10 @@ func standsInFor(x, node string) (string, string) { // 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 succeed or fail otherwise; empty when none does (the error is not one module's). -// Modules already refused are passed over. -func culprit(in compose.Input, node string, fps map[string]string, failed map[string]string, err error) string { +// 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, failed map[string]string, err error) (string, bool) { var assigned []string for _, n := range in.Nodes { if n.Name == node { @@ -263,19 +265,32 @@ func culprit(in compose.Input, node string, fps map[string]string, failed map[st order = append(order, m) } } + changes := func(m string, bare bool) bool { + _, _, _, again := compose.CompositionsShadow(stubbed(in, node, m, bare), node, fps) + return again == nil || again.Error() != err.Error() + } for _, m := range order { if _, done := failed[m]; done { continue } - if _, _, _, again := compose.CompositionsShadow(stubbed(in, node, m), node, fps); again == nil || again.Error() != err.Error() { - return m + if changes(m, false) { + return m, false } } - return "" + for _, m := range order { + if _, done := failed[m]; done { + continue + } + if changes(m, true) { + return m, true + } + } + return "", false } -// stubbed is in with module composed as a stub on node alone (compose.Stub, as node's Kept). -func stubbed(in compose.Input, node, module string) compose.Input { +// stubbed is in with module composed as a stub on node alone (compose.Stub, as node's Kept); bare, a stub that also +// contributes nothing to what it requires. +func stubbed(in compose.Input, node, module string, bare bool) compose.Input { out := in out.Nodes = append([]compose.Node(nil), in.Nodes...) for i := range out.Nodes { @@ -292,7 +307,11 @@ func stubbed(in compose.Input, node, module string) compose.Input { for k, v := range out.Nodes[i].Kept { kept[k] = v } - kept[module] = compose.Stub(man) + stub := compose.Stub(man) + if bare { + stub.Contributes, stub.ContributesMany = nil, nil + } + kept[module] = stub out.Nodes[i].Kept = kept } return out diff --git a/go/compose/frombus/answer_rows_test.go b/go/compose/frombus/answer_rows_test.go index b1a463c..b90e2a1 100644 --- a/go/compose/frombus/answer_rows_test.go +++ b/go/compose/frombus/answer_rows_test.go @@ -111,7 +111,7 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { } // A refusal naming no module placed here is no module's: no compose is spent stubbing each in turn (issue 1126: ace's // answer took 7m46s, one compose per module), though here a stub would compose. - if m := culprit(in, "n1", fps, map[string]string{}, fmt.Errorf("nothing in this mesh provides \"artifact-store\"")); m != "" { + if m, _ := culprit(in, "n1", fps, map[string]string{}, fmt.Errorf("nothing in this mesh provides \"artifact-store\"")); m != "" { t.Errorf("a refusal naming no placed module was laid on %s", m) } ownKey, _ := delivery.ComposedNodeKey("n1") @@ -274,6 +274,40 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { t.Errorf("the operator export is composed as %s, want the file that runs %s", keptComposed, keptFile) } + // (D) Issue 1126, seed-8 (mesh-catalog #542): a module's own refusal refuses that module, never the node. novox runs + // 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 the rest is judged. + 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"}],` + + `"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} { + 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")} + a = Answer(ctx, b, Mesh{}, Scope{Node: "n1", Placements: given(b), InOwn: []string{"de-spiegel"}}, fps, nil, "h1", 3) + refused = nil + for _, r := range a.Refusals { + refused = append(refused, r.Module) + if r.Module == "de-spiegel" && (!strings.Contains(r.Why, "${config:praktijk-name}") || strings.Contains(r.Why, "${setting:")) { + 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"}) { + t.Errorf("a module's own refusal: %+v, want de-spiegel refused alone, the node not", a) + } + if !slices.ContainsFunc(a.Added, func(m delivery.ModuleAdded) bool { return m.Module == "proxy" }) { + t.Errorf("proxy, which composes, is not judged beside de-spiegel's refusal: %+v", a) + } } // A module another node cannot run in this composition offers nothing in it: the refusal names it and why diff --git a/go/compose/frombus/testdata/de-spiegel-e25e1beb.module.json b/go/compose/frombus/testdata/de-spiegel-e25e1beb.module.json new file mode 100644 index 0000000..b4c4bd2 --- /dev/null +++ b/go/compose/frombus/testdata/de-spiegel-e25e1beb.module.json @@ -0,0 +1,129 @@ +{ + "module": "de-spiegel", + "version": "1", + "slug": "spiegel", + "capabilities": [ + "container-runtime" + ], + "requires": [ + "route", + "smtp" + ], + "contributes": { + "route": { + "site": { + "label": "de-spiegel", + "port": 35621 + }, + "praktijk": { + "name": "${config:praktijk-name}", + "port": 35621 + }, + "praktijk-www": { + "name": "${config:praktijk-www-name}", + "redirect": "https://${config:praktijk-name}" + }, + "psychologie": { + "name": "${config:psychologie-name}", + "port": 35621 + }, + "psychologie-www": { + "name": "${config:psychologie-www-name}", + "redirect": "https://${config:psychologie-name}" + } + }, + "smtp": { + "account": "de-spiegel" + } + }, + "binds": { + "route": "/var/lib/de-spiegel/route.json", + "smtp": "/var/lib/de-spiegel/smtp.json" + }, + "config": { + "praktijk-name": { + "type": "string", + "description": "The public name the practice's site is served on; the operator's, with no default (novox/hq ADR 0112).", + "trusted": true, + "required": true + }, + "praktijk-www-name": { + "type": "string", + "description": "The public name that redirects to the practice's site (its www form); the operator's, with no default (novox/hq ADR 0112).", + "trusted": true, + "required": true + }, + "psychologie-name": { + "type": "string", + "description": "The public name the psychology practice's site is served on; the operator's, with no default (novox/hq ADR 0112).", + "trusted": true, + "required": true + }, + "psychologie-www-name": { + "type": "string", + "description": "The public name that redirects to the psychology practice's site (its www form); the operator's, with no default (novox/hq ADR 0112).", + "trusted": true, + "required": true + } + }, + "listens": [ + { + "port": 35621, + "protocol": "tcp", + "from": "mesh", + "why": "the de-spiegel site and its /contact endpoint over http; the practice's three public names are route grants, and the proxy reaches every one of them here" + } + ], + "resources": [ + { + "id": "state", + "type": "directory", + "path": "/var/lib/de-spiegel", + "mode": "0700" + }, + { + "id": "server-env", + "type": "file", + "path": "/var/lib/de-spiegel/server.env", + "mode": "0600", + "content": "SMTP_HOST=${bound:smtp:at}\nSMTP_PORT=${bound:smtp:port}\nSMTP_AUTH_USER=de-spiegel@${bound:smtp:domain}\nSMTP_AUTH_PASS=${secret:smtp}\n" + }, + { + "id": "net", + "type": "network", + "name": "de-spiegel" + }, + { + "id": "server", + "type": "container", + "name": "de-spiegel", + "artifact": "site", + "network": "de-spiegel", + "env-file": [ + "/var/lib/de-spiegel/server.env" + ], + "ports": [ + "35621" + ], + "secrets-in-environment": "the application's own code reads SMTP_AUTH_USER/PASS from the environment (server/index.js); converting is this repository's change" + } + ], + "build": { + "on": [ + { + "arg": "NODE_BASE", + "image": "node@sha256:48e4b67d85f87bd551df43704e24d252f56cc5f8e9718841aace50f19948f0f9" + } + ], + "artifacts": [ + { + "name": "site", + "kind": "image", + "from": "Dockerfile" + } + ] + }, + "secrets": { + "smtp": "/var/lib/de-spiegel/smtp.secret" + } +}