Merge pull request 'A held module composes only as what runs of it, so its hold no longer spreads to the modules that use it (issue 1126)' (#128) from fix/1126-held-as-runs into main

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])

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) {

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"`

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

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
+// composition is in out: the node keeps what it last applied. **A held module is no dependence of its neighbours**
-// whose composition differs when the held ones are composed as their stubs instead is held with them, and a node's own
+// (issue 1126, the spreading hold): a consumer of what it provides, or a collector of its items, composes against what
-// composition that differs (but for the held modules' own resources) refuses the node. And since a held module is
+// runs of it like any other module, and is never held for how the held one would compose as its stub (that comparison
-// composed from its new manifest, which may not be the build's that runs, what reads that manifest is held with it
+// held every postgres consumer behind de-spiegel's hold on novox, three rounds in a row). What it composes is compared
-// (dependents): the providers of what it contributes to (the proxy its routes), and, when it listens, the filter.
+// 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).

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
+	// secret was never issued here: held, the filter must not open 8080 and drop the 20005 that runs. The filter, which
-	// composition depends on how site is held (here the filter) is held with it.
+	// 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

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) {