cmd/mesh-controller/collect.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"time"
 
 	"github.com/novox/mesh-controller/internal/artifacts"
 	"github.com/novox/mesh-controller/internal/inventory"
@@ -49,28 +50,43 @@ func collect(ctx context.Context, inv *inventory.Inventory) {
 		return
 	}
 
+	// **Bounded, because this runs inside somebody's build.** The first sweep of a mesh that has
+	// never collected has the whole history to get through, and a person waiting on `build` should
+	// not pay for it. Two bounds, and what is left over is simply offered again next time —
+	// builds are frequent, and the point is that the store stops growing, not that it empties
+	// tonight.
+	within, stop := context.WithTimeout(ctx, sweepBudget)
+	defer stop()
 	store := artifacts.Store{Address: address}
+
 	var done []string
-	var refused int
-	for _, reference := range references {
-		switch err := store.LetGo(ctx, reference); {
-		case err == nil, errors.Is(err, artifacts.Gone):
+	var left int
+	for i, reference := range references {
+		if i >= mostPerSweep || within.Err() != nil {
+			left = len(references) - i
+			break
+		}
+		err := store.LetGo(within, reference)
+		if err == nil || errors.Is(err, artifacts.Gone) {
 			// Gone is the outcome wanted, already true. Recorded so the next sweep does not ask
 			// again for ever.
 			done = append(done, reference)
-		default:
-			refused++
-			if refused == 1 {
-				// Once per sweep. A store that refuses one refuses all of them, and a hundred
-				// identical lines would bury the reason.
-				fmt.Fprintf(os.Stderr, "the artifact store kept %s: %v\n", reference, err)
-			}
+			continue
 		}
+		// **Stopped at the first refusal, not pushed through.** A store that refuses one refuses
+		// all of them — deletion disabled, the store down, the network gone — so going on would
+		// be a hundred identical failures and a hundred identical log lines in front of whoever
+		// was building something.
+		fmt.Fprintf(os.Stderr, "the artifact store kept %s, so nothing more was asked of it: %v\n",
+			reference, err)
+		left = len(references) - i
+		break
 	}
+
 	if len(done) > 0 {
+		// Recorded outside `within`: the deletions happened, and losing the record of them because
+		// the sweep ran out of budget would mean asking about them again for ever.
 		if err := inv.MarkCollected(ctx, done); err != nil {
-			// Said, and that is all: the artifacts are gone either way, and the only cost of an
-			// unrecorded collection is that the next sweep asks about them again.
 			fmt.Fprintf(os.Stderr, "the store let go of %d artifact(s) and the record of it did not keep: %v\n",
 				len(done), err)
 			return
@@ -78,7 +94,15 @@ func collect(ctx context.Context, inv *inventory.Inventory) {
 		fmt.Fprintf(os.Stderr, "the artifact store let go of %d artifact(s) the mesh no longer keeps\n",
 			len(done))
 	}
-	if refused > 0 {
-		fmt.Fprintf(os.Stderr, "%d artifact(s) were not collected; the next build asks again\n", refused)
+	if left > 0 {
+		fmt.Fprintf(os.Stderr, "%d more to collect; the next build asks again\n", left)
 	}
 }
+
+// mostPerSweep is how many artifacts one sweep will ask about. Enough that a mesh building
+// several times a day converges within days of this landing; small enough that no single build
+// waits on the whole backlog.
+const mostPerSweep = 200
+
+// sweepBudget is the longest a sweep will keep a build waiting.
+const sweepBudget = 60 * time.Second

internal/catalogue/consumer_into_serves.go

@@ -177,7 +177,7 @@ func sortedAnyKeys(values map[string]any) []string {
 // choice servedOnThisMachine makes for the consumer's half. Nothing serving it on this machine is
 // not an error: a contribution can reach a machine whose provider is a record or an adapter, and
 // then there is nothing derived to tell.
-func (r Resolution) derivedFor(provision, as string, settings SettingsBy) (map[string]any, error) {
+func (r Resolution) derivedFor(provision, as, consumer, local string, settings SettingsBy) (map[string]any, error) {
 	for _, m := range r.Modules {
 		serves, said := m.Serves[provision]
 		if !said {
@@ -195,6 +195,27 @@ func (r Resolution) derivedFor(provision, as string, settings SettingsBy) (map[s
 		if names == nil {
 			return nil, nil
 		}
+		// **A consumer that keeps several holders of this provision is refused** — this is issue
+		// 124's own failure one case to the side, and it would be just as quiet.
+		//
+		// Each holder gets its own login, `…_<local>` (ADR 0094), and a provider derives from the
+		// login, so it would make one resource per holder. The consumer's side has no such
+		// dimension: one binding file per provision, one `${bound:<provision>:<key>}`, both
+		// derived from the un-suffixed identity. So the provider would create the holder's
+		// resource and the consumer would be configured against a name nothing made — it would
+		// authenticate successfully and be refused on every object, which reads like a credential
+		// fault and is not one.
+		//
+		// Lifting this means giving the consumer's side a local dimension. That is a decision,
+		// not an omission, and until it is taken the mesh says so rather than guessing.
+		if local != "" {
+			return nil, fmt.Errorf(
+				"%s keeps several holders of %s (this one is %q), and %s derives %s for each "+
+					"consumer from the login the mesh minted. Each holder has its own login, and a "+
+					"consumer is told one value per requirement — so the two ends would name "+
+					"different things and nothing would compare them (novox/hq ADR 0201)",
+				consumer, local, provision, m.Module, orNothing(sortedAnyKeys(names)))
+		}
 		settled, err := Settle(names, settings[m.Module])
 		if err != nil {
 			return nil, fmt.Errorf("%s serving %s: %w", m.Module, provision, err)

internal/catalogue/declaration.go

@@ -1307,7 +1307,7 @@ func (r Resolution) contributions(settings SettingsBy, grants []Grant,
 			continue
 		}
 		as := holderAs(ConsumerIdentity(g.Consumer, IdentitySource(g.Slug, g.From)), g.Local)
-		derived, err := r.derivedFor(g.Provision, as, settings)
+		derived, err := r.derivedFor(g.Provision, as, g.From, g.Local, settings)
 		if err != nil {
 			return nil, err
 		}

internal/catalogue/derived_for_consumer_test.go

@@ -295,3 +295,49 @@ func storeGrants(t *testing.T, out []map[string]any) []Contribution {
 	t.Fatalf("the provider was given no contributions file: %v", out)
 	return nil
 }
+
+// A consumer that keeps SEVERAL holders of one provision is refused, rather than told one thing
+// while its provider is told another.
+//
+// **This is issue 124's own failure, one case to the side.** The mesh gives each holder its own
+// login — `mesh_node_mod_<local>` (ADR 0094) — and the provider derives from the login, so it
+// would make one resource per holder. The consumer's side has no such dimension: there is one
+// binding file per provision and one `${bound:<provision>:<key>}`, both derived from the
+// un-suffixed identity. So the provider would create `…-mod-cold` and the consumer would be
+// configured against `…-mod`: it would authenticate successfully and be refused on every object,
+// which is exactly the fault this whole record exists to end.
+//
+// Refused, loudly, at the one place that can see both halves. Lifting it means giving the
+// consumer's side a local dimension, which is a decision and not an omission.
+func TestAConsumerWithSeveralHoldersOfADerivingProviderIsRefused(t *testing.T) {
+	m := files()
+	// Two holders of the one provision, the shape ADR 0094 gives a module that keeps several.
+	m.Secrets = nil
+	m.SecretsMany = map[string]map[string]string{"s3-bucket": {
+		"hot":  "/var/lib/files/hot.secret",
+		"cold": "/var/lib/files/cold.secret",
+	}}
+	m.Resources = []map[string]any{{
+		"id": "env", "type": "file", "path": "/var/lib/files/env", "mode": "0600",
+		"content": "BUCKET=${bound:s3-bucket:bucket}\n",
+	}}
+	r, err := Resolve(shelf(store(), m), []string{"store", "files"}, reachable(), World{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = r.Declaration(Rendering{Grants: []Grant{
+		{Provision: "s3-bucket", Consumer: "workstation", From: "files", Slug: "files",
+			Local: "hot", Values: map[string]any{}, Sealed: "c2VhbGVk"},
+		{Provision: "s3-bucket", Consumer: "workstation", From: "files", Slug: "files",
+			Local: "cold", Values: map[string]any{}, Sealed: "c2VhbGVk"},
+	}})
+	if err == nil {
+		t.Fatal("a consumer with several holders of a deriving provider was accepted; " +
+			"its two ends would have disagreed in silence")
+	}
+	for _, want := range []string{"files", "s3-bucket", "bucket"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("the refusal does not name %q: %v", want, err)
+		}
+	}
+}