From c2999a4e13cc68bd65cd5f4be1ed02f9870c19c0 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 29 Oct 2026 00:49:00 +0100 Subject: [PATCH] A held module is no dependence of its neighbours: its hold stops spreading (issue 1126) novox's seed p-20261028t231141-ec84b4 refused 11 modules "what it composes of ..., held, is not what runs of them": de-spiegel's hold spread through mailu to postgres, nftables and route-proxy, and from postgres to every consumer, and gitea, held so, refused the node: "its credential of postgres-database is handed to the provisioner and the module is not: nothing would fill it". Three rounds each moved the error. The kind: a held module still took part in the composition as a dependence, so its hold spread. - The quick fix: let heldAsToday also follow dependsOnHeld's words, and skip the handed refusal for gitea. It moves the error once more and leaves every node's applying path holding consumers. - The thorough fix, chosen: a held module composes only as what runs of it. composeEach no longer compares the composition with each held module as its stub (dependsOnHeld removed): a consumer or collector composes against what runs of the held module like any other module, and a real difference is still a named difference in the answer, a module's own refusal still its refusal. A stub (compose.Stub, catalogue.Manifest.Stubbed) leaves the credentials and own secrets it would hand the provisioner out of its plan as they run, never refused for want of a plan. Only what a held module's new manifest says to others in their own files is still held behind it (dependents: the provider of what it contributes to, the filter where it listens), which heldAsToday already follows. Tests: rows added to existing tests (a held provider with a binding consumer, which the old code held in the seed's words; a held feeder of a collector; a held module with a handed credential and own secret). No new test function. --- go/catalogue/declaration.go | 6 ++ go/catalogue/handed_secrets_test.go | 16 ++++ go/catalogue/manifest.go | 5 ++ go/compose/composed.go | 2 + go/compose/frombus/answer.go | 100 ++----------------------- go/compose/frombus/answer_rows_test.go | 51 ++++++++++++- go/compose/frombus/frombus_test.go | 16 +++- 7 files changed, 100 insertions(+), 96 deletions(-) diff --git a/go/catalogue/declaration.go b/go/catalogue/declaration.go index 66c0660..6a96e6e 100644 --- a/go/catalogue/declaration.go +++ b/go/catalogue/declaration.go @@ -1103,6 +1103,9 @@ func (r Resolution) compose(with Rendering, owner map[string]string, } for _, unsettled := range resources { if pr, ok := unsettled[HandedProvisionField].(PlanProvision); ok { + if handing == nil && m.Stubbed { + continue // held: its credential file is what runs of it, left out (issue 1126) + } if handing == nil { return nil, fmt.Errorf("%s: its credential of %s is handed to the provisioner and the module is "+ "not: nothing would fill it", m.Module, pr.Provision) @@ -1127,6 +1130,9 @@ func (r Resolution) compose(with Rendering, owner map[string]string, continue } if unsettled[HandedSecretField] != nil { + if handing == nil && m.Stubbed { + continue // held: its own secret's file is what runs of it, left out (issue 1126) + } if handing == nil { return nil, fmt.Errorf("%s: its own secret %v is handed to the provisioner and the module is "+ "not: nothing would fill it", m.Module, unsettled[HandedSecretField]) diff --git a/go/catalogue/handed_secrets_test.go b/go/catalogue/handed_secrets_test.go index 5d06214..d7f988a 100644 --- a/go/catalogue/handed_secrets_test.go +++ b/go/catalogue/handed_secrets_test.go @@ -185,6 +185,22 @@ func TestAHandedModulesCredentialIssuedForItIsNamedInItsPlanAndNoValueIsSealed(t if !reflect.DeepEqual(c.Plans["bot"].Provisions, want) { t.Fatalf("the plan names %+v, want %+v", c.Plans["bot"].Provisions, want) } + // Issue 1126 (novox's seed: "gitea: its credential of postgres-database is handed to the provisioner and the module + // is not"): held, the module is composed as its stub, with no template and no item, so nothing is handed. Its + // credential and its own secret are what runs of it, left out, and the composition is not refused. + r := forgeReader(t, true, true) + for i := range r.Modules { + if r.Modules[i].Module == "bot" { + h := *r.Modules[i].Handed + h.Templates = nil + r.Modules[i].Handed, r.Modules[i].Stubbed, r.Modules[i].Build = &h, true, nil + r.Modules[i].Resources = slices.DeleteFunc(r.Modules[i].Resources, func(x map[string]any) bool { return x["type"] != "directory" }) + } + } + c, got = composeWith(t, r, handingTo("store:5000")) + if strings.Contains(got, SecretID("forge-read")) || strings.Contains(got, NeedID("token")) || c.Plans["bot"].Provisions != nil { + t.Fatalf("a held module's handed credential or own secret is composed: %s", got) + } } func TestACredentialIssuedForAHolderIsSealedAsBeforeWhereItsModuleIsNotHanded(t *testing.T) { diff --git a/go/catalogue/manifest.go b/go/catalogue/manifest.go index d91df7f..b40dbad 100644 --- a/go/catalogue/manifest.go +++ b/go/catalogue/manifest.go @@ -870,6 +870,11 @@ type Manifest struct { // path its manifest leaves to the mesh, a binding). Nil for any other module. Never in a manifest's JSON. OwnsPaths []string `json:"-"` + // Stubbed marks a module composed as its stub (compose.Stub: held on its node, issue 1126): none of its own + // composition is applied, so what it hands the provisioner (a credential the vault keeps for it, its own secrets) + // is left out of its plan as it runs, never refused for want of a plan. Never in a manifest's JSON. + Stubbed bool `json:"-"` + // Jails are the fail2ban jails this module declares for its own service (novox/hq to-be 31). // Written into whichever node runs the module, the same way `listens` become that node's rules. Jails []Jail `json:"jails,omitempty"` diff --git a/go/compose/composed.go b/go/compose/composed.go index 599e890..b5a7d7a 100644 --- a/go/compose/composed.go +++ b/go/compose/composed.go @@ -318,6 +318,8 @@ func Stub(m catalogue.Manifest) catalogue.Manifest { m.OwnsPaths = catalogue.DeclaredPaths(m) } m.Build = nil + // What it hands the provisioner is left out as it runs, not refused for want of a plan (catalogue.Manifest.Stubbed). + m.Stubbed = true // Nor is a template its build kept for its own files handed: a stub carries none of them (issue 1126: novox's node // refused "postgres: its build kept a template for consumers, which it does not declare" with postgres held). Its // items stay, and the templates they render through: a held module's contributions to other modules' collections diff --git a/go/compose/frombus/answer.go b/go/compose/frombus/answer.go index fee3a66..bb665eb 100644 --- a/go/compose/frombus/answer.go +++ b/go/compose/frombus/answer.go @@ -2,7 +2,6 @@ package frombus import ( "context" - "encoding/json" "fmt" "regexp" "slices" @@ -11,7 +10,6 @@ import ( "git.novox.be/novox/mesh-sdk/go/catalogue" "git.novox.be/novox/mesh-sdk/go/compose" - "git.novox.be/novox/mesh-sdk/go/declaration" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -312,11 +310,14 @@ 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. **What depends on how it is held is held too:** a module -// whose composition differs when the held ones are composed as their stubs instead is held with them, and a node's own -// composition that differs (but for the held modules' own resources) refuses the node. And since a held module is -// composed from its new manifest, which may not be the build's that runs, what reads that manifest is held with it -// (dependents): the providers of what it contributes to (the proxy its routes), and, when it listens, the filter. +// 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) { mode := compose.Foreseeing if applied { @@ -392,17 +393,6 @@ func composeEach(in compose.Input, node string, fps map[string]string, applied b return nil, nil, nil, fmt.Errorf("%w on %s: %s", compose.ErrStoodIn, node, strings.Join(nodes, ", ")) } } - if !again && len(failed) > 0 { - dependent, own := dependsOnHeld(in, node, fps, mode, out, src, bareOf, failed) - if own { - return nil, nil, nil, fmt.Errorf("frombus: %s's own composition depends on how %s, held, is composed, and is not applied", - node, strings.Join(sortedKeys(failed), ", ")) - } - for _, m := range dependent { - hold(m, "what it composes of "+strings.Join(sortedKeys(failed), ", ")+", held, is not what runs of them", false) - again = true - } - } if !again { break } @@ -411,80 +401,6 @@ func composeEach(in compose.Input, node string, fps map[string]string, applied b return out, skipped, stood, nil } -// dependsOnHeld composes in again with each held module as its stub rather than as what runs of it, and answers the -// modules, not held, whose composition differs between the two (sorted), and own when the node's own composition does -// but for the held modules' own resources. A composition of stubs that fails says nothing more: what runs composed. -func dependsOnHeld(in compose.Input, node string, fps map[string]string, mode compose.Mode, asRuns map[string]delivery.Composed, - src map[string]catalogue.Manifest, bareOf map[string]bool, failed map[string]string) (dependent []string, own bool) { - stubs := in - for _, m := range sortedKeys(failed) { - if man, ok := src[m]; ok { - stubs = heldAs(stubs, node, m, man, bareOf[m], nil) - } - } - asStubs, _, _, err := compose.CompositionsIn(stubs, node, fps, mode) - if err != nil { - return nil, false - } - isHeld := func(m string) bool { _, ok := failed[m]; return ok } - keys := map[string]bool{} - for k := range asRuns { - keys[k] = true - } - for k := range asStubs { - keys[k] = true - } - for _, k := range sortedKeys(keys) { - a, inA := asRuns[k] - b, inB := asStubs[k] - m := a.Module - if !inA { - m = b.Module - } - if m != "" && isHeld(m) { - continue - } - if m == "" { - a, b = withoutHeld(a, isHeld), withoutHeld(b, isHeld) - } - da, errA := delivery.ComposedDigestOf(a) - db, errB := delivery.ComposedDigestOf(b) - if inA == inB && errA == nil && errB == nil && da == db { - continue - } - if m == "" { - own = true - continue - } - dependent = append(dependent, m) - } - return dependent, own -} - -// withoutHeld is the node's own composition c without the resources of a held module (its id naming it) and without -// its filled paths, for comparing the rest. -func withoutHeld(c delivery.Composed, isHeld func(string) bool) delivery.Composed { - out := c - out.Filled = nil - out.Resources = nil - for _, raw := range c.Resources { - var head struct { - ID string `json:"id"` - Module string `json:"module"` - } - _ = json.Unmarshal(raw, &head) - owner := head.Module - if owner == "" { - owner, _ = declaration.ModuleGuess(head.ID) - } - if owner != "" && isHeld(owner) { - continue - } - out.Resources = append(out.Resources, raw) - } - return out -} - // 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). diff --git a/go/compose/frombus/answer_rows_test.go b/go/compose/frombus/answer_rows_test.go index a870cd9..87fe2c3 100644 --- a/go/compose/frombus/answer_rows_test.go +++ b/go/compose/frombus/answer_rows_test.go @@ -433,8 +433,8 @@ 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. A module whose - // composition depends on how site is held (here the filter) is held with it. + // 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). 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"},` + @@ -470,6 +470,53 @@ func answerNamesEveryRefusalAndLeavesAnOldBuildToTheNode(t *testing.T) { } } } + // (G2) Issue 1126, the spreading hold: a held module is no dependence of its neighbours. site, held, provides page, and + // app requires and binds it: app composes against what runs of site and is not held (novox's seed held every postgres + // consumer behind de-spiegel's hold, "what it composes of …, held, is not what runs of them"). + provider := []byte(strings.Replace(string(site), `"listens":`, `"provides":[{"name":"page","scope":"node"}],"serves":{"page":{"port":8080}},"listens":`, 1)) + app := []byte(`{"module":"app","version":"1","capabilities":["systemd"],"requires":["page"],"binds":{"page":"/etc/app/page.json"},` + + `"resources":[{"id":"service","type":"service","unit":"app.service","state":"running","boot":"enabled"}]}`) + b2 := mk("lemurs", overlay.Name, "site", "nft", "app") + for m, raw := range map[string][]byte{"site": provider, "nft": nft, "app": app} { + b2.manifests[delivery.ManifestDigest(raw)] = raw + b2.modules[m] = delivery.Module{Manifest: delivery.ManifestDigest(raw)} + } + p2 := b2.placements["n1"] + p2.Decided.Endpoint = "n1.example:51820" + for i := range p2.Modules { + if p2.Modules[i].Module == "site" { + p2.Modules[i].Ports = map[string]int{"8080": 20005} + } + } + b2.placements["n1"] = p2 + b2.records["n1"] = rec + b2.held = true + fps["app"] = fp("9") + got, _, held, err = Compose(ctx, b2, Mesh{}, Scope{Node: "n1", Running: []delivery.Composed{siteRuns}}, fps) + appKey, _ := delivery.ComposedKey("n1", "app", fps["app"]) + if _, composed := got[appKey]; err != nil || held["site"] == "" || held["app"] != "" || !composed { + t.Errorf("a consumer of held site: %v, held %v; want site held, and app composed against what runs of it", err, held) + } + if bound, _ := json.Marshal(got[appKey].Resources); !strings.Contains(string(bound), "20005") { + 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) + } // (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 diff --git a/go/compose/frombus/frombus_test.go b/go/compose/frombus/frombus_test.go index 122349e..cd3d74d 100644 --- a/go/compose/frombus/frombus_test.go +++ b/go/compose/frombus/frombus_test.go @@ -45,6 +45,8 @@ type bus struct { served map[string]map[string]any servedRead []string buildsRead []string + // held answers every credential asked as held, sealed to the consumer. + held bool } func (b *bus) Build(_ context.Context, module, fp string) (*delivery.BuilderBuild, error) { @@ -89,7 +91,7 @@ func (b *bus) OwnSecrets(context.Context, string) ([]compose.OwnSecretRow, error return b.own, b.ownErr } -func (b *bus) Credentials(_ context.Context, node string, asConsumer, _ []compose.CredentialKey) ([]compose.Credential, []compose.Issued, error) { +func (b *bus) Credentials(_ context.Context, node string, asConsumer, asProvider []compose.CredentialKey) ([]compose.Credential, []compose.Issued, error) { b.asked = append(b.asked, asConsumer...) for _, k := range asConsumer { placed := false @@ -100,7 +102,17 @@ func (b *bus) Credentials(_ context.Context, node string, asConsumer, _ []compos return nil, nil, fmt.Errorf("%s is not on %s: neither assigned there nor required by a module there", k.ConsumerModule, node) } } - return nil, nil, nil + var out []compose.Credential + var issued []compose.Issued + for _, k := range append(append([]compose.CredentialKey{}, asConsumer...), asProvider...) { + if b.held { + out = append(out, compose.Credential{Provision: k.Provision, Consumer: k.Consumer, ConsumerModule: k.ConsumerModule, + Provider: k.Provider, Local: k.Local, State: compose.CredentialHeld, Sealed: `"box"`}) + issued = append(issued, compose.Issued{Provision: k.Provision, Consumer: k.Consumer, ConsumerModule: k.ConsumerModule, + Provider: k.Provider, Local: k.Local, ForProvider: `"box"`}) + } + } + return out, issued, nil } func (b *bus) Values(_ context.Context, node, module string) (map[string]any, error) { -- 2.54.0