Merge pull request 'A pin names the module as well as the node; a node that answers twice is refused' (#195) from feat/pin-names-the-provider into main

cmd/mesh-controller/main.go

@@ -203,7 +203,8 @@ func usage() {
   licence refresh <name>         mint a new access token and seal it to every holder
   rotate <provision> [--consumer <n>]  a new credential for every holder, both ends at once
   ask <module> <tool> [json]           call one of a module's tools over the broker, and print its answer
-  pin <node> <provision> <from>  which node this one gets a provision from
+  pin <node> <provision> <from-node> <module>
+                                 which provider this one gets a provision from: the module, and its node
   unpin <node> <provision>       put that question back
   plan <node> [--files|--json]     what that node would run, and why
   push [<node>] [--behind]       send a node everything it should be, or only those that need it

cmd/mesh-controller/modules.go

@@ -411,8 +411,8 @@ func describeOffers(offers []catalogue.Offer) string {
 // database should not change where an existing machine gets its data the day a second one
 // arrives.
 func pinCommand(ctx context.Context, args []string, setting bool) error {
-	if setting && len(args) != 3 {
-		return errors.New("pin <node> <provision> <from-node>")
+	if setting && len(args) != 4 {
+		return errors.New("pin <node> <provision> <from-node> <module>")
 	}
 	if !setting && len(args) != 2 {
 		return errors.New("unpin <node> <provision>")
@@ -431,17 +431,12 @@ func pinCommand(ctx context.Context, args []string, setting bool) error {
 		fmt.Printf("%s is no longer told where to get %s from\n", args[0], args[1])
 		return nil
 	}
-	if args[0] == args[2] {
-		// Allowed by nothing here, and worth saying rather than resolving into a confusing
-		// refusal later: a node providing something to itself is a node-scoped provision, and
-		// this field is for the other kind.
-		return fmt.Errorf("%s cannot get %s from itself; that would be a provision this machine "+
-			"provides, which does not need saying", args[0], args[1])
-	}
-	if err := inv.PinProvision(ctx, args[0], args[1], args[2]); err != nil {
+	// The provider's node may be this same machine: two modules beside the consumer can both
+	// answer a provision, and then the module is the whole question (novox/hq #258).
+	if err := inv.PinProvision(ctx, args[0], args[1], args[2], args[3]); err != nil {
 		return err
 	}
-	fmt.Printf("%s gets %s from %s\n", args[0], args[1], args[2])
+	fmt.Printf("%s gets %s from %s/%s\n", args[0], args[1], args[2], args[3])
 	fmt.Printf("  run `push %s` to send it\n", args[0])
 	return nil
 }

cmd/mesh-controller/seatverbs.go

@@ -79,6 +79,16 @@ func argvFor(verb string, args map[string]any) ([]string, error) {
 			return nil, err
 		}
 		return []string{verb, str("node"), str("module")}, nil
+	case "pin":
+		if err := need("node", "provision", "from", "module"); err != nil {
+			return nil, err
+		}
+		return []string{"pin", str("node"), str("provision"), str("from"), str("module")}, nil
+	case "unpin":
+		if err := need("node", "provision"); err != nil {
+			return nil, err
+		}
+		return []string{"unpin", str("node"), str("provision")}, nil
 	case "push":
 		// Sent and not waited for: the asker reads `status` for what the machine did, which is
 		// what a person at a shell does too. A tool call that blocked for a push's whole apply would
@@ -176,7 +186,7 @@ func seatToolHandlers() (map[string]link.ToolHandler, error) {
 			}
 			continue
 		}
-		if _, err := argvFor(verb, map[string]any{"node": "x", "module": "x", "repository": "x"}); err != nil {
+		if _, err := argvFor(verb, sampleArguments(v)); err != nil {
 			return nil, fmt.Errorf("the %s seat's row declares %q, which this control plane cannot run: %w",
 				catalogue.ControllerSeatName, verb, err)
 		}
@@ -215,3 +225,22 @@ func seatTools() map[string]any {
 	}
 	return map[string]any{"seats": seats}
 }
+
+// sampleArguments is one of every argument a verb's schema requires, so the check at start proves the
+// verb runnable rather than that it happens to want the arguments the check guessed.
+func sampleArguments(v catalogue.Verb) map[string]any {
+	sample := map[string]any{"node": "x", "module": "x", "repository": "x"}
+	switch required := v.Input["required"].(type) {
+	case []string:
+		for _, k := range required {
+			sample[k] = "x"
+		}
+	case []any:
+		for _, k := range required {
+			if name, ok := k.(string); ok {
+				sample[name] = "x"
+			}
+		}
+	}
+	return sample
+}

internal/catalogue/brokered_test.go

@@ -25,7 +25,7 @@ func reachable() Node {
 func onNetwork(nodes ...string) map[string][]Provider {
 	out := make([]Provider, 0, len(nodes))
 	for _, n := range nodes {
-		out = append(out, Provider{Node: n, At: n + ".internal"})
+		out = append(out, Provider{Node: n, At: n + ".internal", Module: "postgres"})
 	}
 	return map[string][]Provider{"postgres-database": out}
 }
@@ -99,7 +99,7 @@ func TestSayingWhichOneSettlesIt(t *testing.T) {
 	got, err := Resolve(brokeredShelf(), []string{"meshboard"}, reachable(),
 		World{
 			Offered: onNetwork("anchor", "archive"),
-			Pinned:  map[string]string{"postgres-database": "archive"},
+			Pinned:  map[string]Chosen{"postgres-database": {Node: "archive", Module: "postgres"}},
 		})
 	if err != nil {
 		t.Fatal(err)
@@ -115,7 +115,7 @@ func TestBeingPointedAtAMachineThatDoesNotProvideItIsRefused(t *testing.T) {
 	_, err := Resolve(brokeredShelf(), []string{"meshboard"}, reachable(),
 		World{
 			Offered: onNetwork("anchor", "archive"),
-			Pinned:  map[string]string{"postgres-database": "somewhere-else"},
+			Pinned:  map[string]Chosen{"postgres-database": {Node: "somewhere-else", Module: "postgres"}},
 		})
 	if err == nil {
 		t.Fatal("a machine was silently given a different database from the one chosen")
@@ -131,12 +131,12 @@ func TestOneProviderDoesNotOverruleAChoice(t *testing.T) {
 	_, err := Resolve(brokeredShelf(), []string{"meshboard"}, reachable(),
 		World{
 			Offered: onNetwork("anchor"),
-			Pinned:  map[string]string{"postgres-database": "archive"},
+			Pinned:  map[string]Chosen{"postgres-database": {Node: "archive", Module: "postgres"}},
 		})
 	if err == nil {
 		t.Fatal("the only database was used although another was chosen")
 	}
-	if !strings.Contains(err.Error(), "only anchor provides it") {
+	if !strings.Contains(err.Error(), "only anchor/postgres provides it") {
 		t.Fatalf("the refusal does not say what is available: %v", err)
 	}
 }

internal/catalogue/chosen.go

@@ -0,0 +1,100 @@
+package catalogue
+
+import (
+	"sort"
+)
+
+// Chosen is the provider somebody named for a provision: the module, and the node it runs on. Both,
+// always (novox/hq #258) — a provision comes from a module, and the same module on two machines is
+// two answers, so neither half alone says which. Module is empty only on a record made before this
+// was asked, and such a record is honoured exactly as long as it is unambiguous.
+type Chosen struct {
+	Node   string
+	Module string
+}
+
+func (c Chosen) String() string {
+	if c.Module == "" {
+		return c.Node
+	}
+	return c.Node + "/" + c.Module
+}
+
+// matches is whether this provider is the one chosen.
+func (c Chosen) matches(p Provider) bool {
+	return p.Node == c.Node && (c.Module == "" || p.Module == c.Module)
+}
+
+// among is every offered provider the choice names — one, when the choice is whole.
+func (c Chosen) among(where []Provider) []Provider {
+	var out []Provider
+	for _, p := range where {
+		if c.matches(p) {
+			out = append(out, p)
+		}
+	}
+	return out
+}
+
+// nameOf is how a refusal names a provider: the node and the module on it.
+func nameOf(p Provider) string {
+	return Chosen{Node: p.Node, Module: p.Module}.String()
+}
+
+// providerNames is every provider named, sorted, for a refusal to list.
+func providerNames(where []Provider) []string {
+	out := make([]string, 0, len(where))
+	for _, p := range where {
+		out = append(out, nameOf(p))
+	}
+	sort.Strings(out)
+	return out
+}
+
+// providersHere is which modules in this node's own set offer a provision, sorted.
+func providersHere(catalogue map[string]Manifest, here func(string) bool, want string) []string {
+	var out []string
+	for name, m := range catalogue {
+		if !here(name) {
+			continue
+		}
+		for _, o := range m.Offers() {
+			if o == want {
+				out = append(out, name)
+				break
+			}
+		}
+	}
+	sort.Strings(out)
+	return out
+}
+
+// servedByOne is what one provider beside the consumer says a consumer needs to know, or nothing.
+//
+// Serving is *whether* a need is created at all when the provider is on this same machine (novox/hq
+// 04-ISSUES/038's sibling): a need never created is a binding the consumer never gets. The manifest
+// alone answers that; the values are settled later, with the node's settings.
+func servedByOne(m Manifest, want string) map[string]any {
+	if _, ok := m.Serves[want]; ok {
+		return ServedOn(m, want, nil)
+	}
+	return nil
+}
+
+// sharedByOne is the own secret that provider names as its credential (ADR 0158), or "" when it
+// gives each consumer its own.
+func sharedByOne(m Manifest, want string) string {
+	if own, shared := m.SharedCredentialOf(want); shared {
+		return own
+	}
+	return ""
+}
+
+func oneOf(list []string, s string) bool {
+	for _, x := range list {
+		if x == s {
+			return true
+		}
+	}
+	return false
+}

internal/catalogue/pin_names_module_test.go

@@ -0,0 +1,105 @@
+package catalogue
+
+import (
+	"strings"
+	"testing"
+)
+
+// Two modules on one node both provide acme-ca — public-acme (Let's Encrypt) and step-ca (the
+// mesh's own authority) on novox — and a route-proxy elsewhere must get the public one (novox/hq
+// #258). A pin names the module as well as the node, so that it can say which.
+
+func issuerShelf() map[string]Manifest {
+	return shelf(
+		Manifest{Module: "public-acme", Version: "1", Provides: FromAnywhere("acme-ca"),
+			Serves: map[string]map[string]any{"acme-ca": {"at": "acme-v02.api.letsencrypt.org"}}},
+		Manifest{Module: "step-ca", Version: "1", Provides: FromAnywhere("acme-ca"),
+			Serves: map[string]map[string]any{"acme-ca": {"at": "novox.internal"}}},
+		Manifest{Module: "route-proxy", Version: "1", Requires: []string{"acme-ca"}},
+	)
+}
+
+func twoIssuersOnOneNode() map[string][]Provider {
+	return map[string][]Provider{"acme-ca": {
+		{Node: "novox", At: "novox.internal", Module: "public-acme", Serves: map[string]any{"at": "acme-v02.api.letsencrypt.org"}},
+		{Node: "novox", At: "novox.internal", Module: "step-ca", Serves: map[string]any{"at": "novox.internal"}},
+	}}
+}
+
+func TestTwoProvidersOnOneNodeAreRefusedWithBothNamed(t *testing.T) {
+	// The refusal must name the pair, because a node alone cannot tell them apart.
+	_, err := Resolve(issuerShelf(), []string{"route-proxy"}, reachable(),
+		World{Offered: twoIssuersOnOneNode()})
+	if err == nil {
+		t.Fatal("two providers on one node were resolved by picking")
+	}
+	for _, want := range []string{"novox/public-acme", "novox/step-ca", "<node> <module>"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Fatalf("the refusal does not say %q: %v", want, err)
+		}
+	}
+}
+
+func TestAPinNamesTheModule(t *testing.T) {
+	got, err := Resolve(issuerShelf(), []string{"route-proxy"}, reachable(),
+		World{Offered: twoIssuersOnOneNode(),
+			Pinned: map[string]Chosen{"acme-ca": {Node: "novox", Module: "public-acme"}}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(got.Needs) != 1 || got.Needs[0].From != "novox" || got.Needs[0].Serves["at"] != "acme-v02.api.letsencrypt.org" {
+		t.Fatalf("the named module was not the one taken: %+v", got.Needs)
+	}
+}
+
+func TestARecordNamingOnlyTheNodeIsRefusedWhenThatNodeAnswersTwice(t *testing.T) {
+	// A pin from before the module was asked for. It once took the last one listed — a coin flip.
+	_, err := Resolve(issuerShelf(), []string{"route-proxy"}, reachable(),
+		World{Offered: twoIssuersOnOneNode(), Pinned: map[string]Chosen{"acme-ca": {Node: "novox"}}})
+	if err == nil {
+		t.Fatal("a node that answers twice was resolved by picking")
+	}
+	if !strings.Contains(err.Error(), "provides it 2 times") || !strings.Contains(err.Error(), "pin workstation acme-ca novox <module>") {
+		t.Fatalf("the refusal does not ask for the module: %v", err)
+	}
+}
+
+func TestAPinNamingAModuleThatDoesNotProvideItIsRefused(t *testing.T) {
+	_, err := Resolve(issuerShelf(), []string{"route-proxy"}, reachable(),
+		World{Offered: twoIssuersOnOneNode(), Pinned: map[string]Chosen{"acme-ca": {Node: "novox", Module: "gitea"}}})
+	if err == nil || !strings.Contains(err.Error(), "novox/gitea does not provide it") {
+		t.Fatalf("a module that does not provide it was not refused by name: %v", err)
+	}
+}
+
+func TestTwoProvidersBesideTheConsumerAreRefusedUntilOneIsNamed(t *testing.T) {
+	// The same ambiguity on the consumer's own machine. This was settled by a map walk — random,
+	// per plan — which is how novox's own proxy got its issuer.
+	_, err := Resolve(issuerShelf(), []string{"route-proxy", "public-acme", "step-ca"}, reachable(), World{})
+	if err == nil {
+		t.Fatal("two providers beside the consumer were resolved by picking")
+	}
+	if !strings.Contains(err.Error(), "workstation provides \"acme-ca\" 2 times") || !strings.Contains(err.Error(), "public-acme, step-ca") {
+		t.Fatalf("the refusal does not list them: %v", err)
+	}
+}
+
+func TestAPinSettlesTwoProvidersBesideTheConsumer(t *testing.T) {
+	got, err := Resolve(issuerShelf(), []string{"route-proxy", "public-acme", "step-ca"}, reachable(),
+		World{Pinned: map[string]Chosen{"acme-ca": {Node: "workstation", Module: "public-acme"}}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	var found bool
+	for _, n := range got.Needs {
+		if n.Name == "acme-ca" {
+			found = true
+			if n.Serves["at"] != "acme-v02.api.letsencrypt.org" {
+				t.Fatalf("the named module was not the one taken: %+v", n)
+			}
+		}
+	}
+	if !found {
+		t.Fatalf("no need for acme-ca was created: %+v", got.Needs)
+	}
+}

internal/catalogue/resolve.go

@@ -48,11 +48,12 @@ type World struct {
 	Holdings []Held
 	// Offered is what other nodes provide at mesh scope, and everything needed to use it.
 	Offered map[string][]Provider
-	// Pinned is which node this machine was told to get a provision from, by name. Only consulted
-	// when more than one node could answer -- a choice recorded before it was needed should not
-	// start meaning something the day a second provider appears, and one recorded and then made
-	// unnecessary should not quietly stop applying either.
-	Pinned map[string]string
+	// Pinned is which provider this machine was told to get a provision from, by name: a module and
+	// the node it runs on, both (novox/hq #258). Only consulted when more than one could answer -- a
+	// choice recorded before it was needed should not start meaning something the day a second
+	// provider appears, and one recorded and then made unnecessary should not quietly stop applying
+	// either.
+	Pinned map[string]Chosen
 	// Licences is every provision answered by a **record rather than a node**, by provision name.
 	//
 	// novox/hq ADR 0024: a hosted model is on nobody's machine and is reached over the public
@@ -260,6 +261,10 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world
 	// still "choose one" after somebody has chosen one. That makes the remedy useless, and it is
 	// how this read when first used.
 	satisfied := map[string]bool{}
+	// Which modules a person assigned here, hostable. The walk marks a module chosen only when it
+	// reaches it, and a consumer may be reached before the provider beside it — so the provider of
+	// something already satisfied is looked for among these as well as among the chosen.
+	assignedHere := map[string]bool{}
 
 	// Everything a person assigned goes in first, except what this machine cannot run. Those are
 	// choices already made, and a requirement one of them answers is not a choice to put back to
@@ -284,6 +289,7 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world
 			for _, o := range m.Offers() {
 				satisfied[o] = true
 			}
+			assignedHere[a] = true
 		}
 		because[a] = "assigned"
 		queue = append(queue, a)
@@ -311,6 +317,42 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world
 		// commonest arrangement of all — a service and its database on one node — the weakest
 		// handling, silently.
 		if satisfied[want] && !isModule(catalogue, want) {
+			here := func(name string) bool { return chosen[name] || assignedHere[name] }
+			local := providersHere(catalogue, here, want)
+			// Which of them it matters to choose between. A plain capability — a shell, a display
+			// server — asks nothing of whoever answers it, and three shells beside an editor are
+			// not a choice to put to anybody. One that grants a credential, or serves a fact the
+			// consumer cannot guess, becomes a binding, and a binding is to one provider.
+			matter := local
+			if !brokered[want] {
+				matter = nil
+				for _, name := range local {
+					if _, ok := catalogue[name].Serves[want]; ok {
+						matter = append(matter, name)
+					}
+				}
+			}
+			var by Manifest
+			switch len(matter) {
+			case 0:
+				// Nothing to bind to; satisfied by its presence, as it was.
+			case 1:
+				by = catalogue[matter[0]]
+			default:
+				// Two modules on this machine answer it. Taking whichever a map walk met first
+				// was the rule until novox/hq #258 — random, per plan — and the same stance as
+				// across machines applies: ambiguity is refused, never resolved by picking.
+				c, pinned := world.Pinned[want]
+				if !pinned || c.Node != node.Name || !oneOf(matter, c.Module) {
+					reported[want] = true
+					problems = append(problems, fmt.Sprintf(
+						"%s provides %q %d times, wanted by %s — say which with `pin %s %s %s <module>`: %s",
+						node.Name, want, len(matter), because[want], node.Name, want, node.Name,
+						strings.Join(matter, ", ")))
+					continue
+				}
+				by = catalogue[c.Module]
+			}
 			if brokered[want] {
 				// Answered here, and still a need: the provider is this node.
 				//
@@ -326,9 +368,9 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world
 				}
 				needs = append(needs, Needed{
 					Name: want, From: node.Name, At: at,
-					Serves: servedHere(catalogue, chosen, want), For: because[want],
-					SharedOwn: sharedHere(catalogue, chosen, want)})
-			} else if served := servedHere(catalogue, chosen, want); len(served) > 0 {
+					Serves: servedByOne(by, want), For: because[want],
+					SharedOwn: sharedByOne(by, want)})
+			} else if served := servedByOne(by, want); len(served) > 0 {
 				// Answered here with no credential to mint, but the provider serves facts the
 				// consumer cannot guess — a port, a model name — and so still needs a binding.
 				// **The reachability rule does not apply**: both ends are on this same machine, so
@@ -358,11 +400,7 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world
 		if brokered[want] {
 			reported[want] = true
 			where := world.Offered[want]
-			names := make([]string, 0, len(where))
-			for _, p := range where {
-				names = append(names, p.Node)
-			}
-			sort.Strings(names)
+			names := providerNames(where)
 			take := func(p Provider) {
 				if node.At != "" && p.At == "" || node.At == "" && p.At != "" || node.At == "" && p.At == "" {
 					// One of them is not on the private network, so there is no path between
@@ -396,17 +434,17 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world
 					"nothing in this mesh provides %q, wanted by %s %s",
 					want, because[want], remedy))
 			case len(where) == 1:
-				if chosenNode, pinned := world.Pinned[want]; pinned && chosenNode != where[0].Node {
+				if c, pinned := world.Pinned[want]; pinned && !c.matches(where[0]) {
 					// One provider, and it is not the one this machine was told to use. Silently
 					// using the other would be the mesh overruling a choice somebody made.
 					problems = append(problems, fmt.Sprintf(
 						"%s was told to get %q from %s, and only %s provides it",
-						node.Name, want, chosenNode, where[0].Node))
+						node.Name, want, c, nameOf(where[0])))
 					break
 				}
 				take(where[0])
 			default:
-				chosenNode, pinned := world.Pinned[want]
+				c, pinned := world.Pinned[want]
 				if !pinned {
 					// **The seat's holder answers, when a seat delivers this** (novox/hq ADR 0110).
 					// Not a guess, which ADR 0009 refuses: the choice was made once, mesh-wide, by
@@ -418,27 +456,32 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world
 						break
 					}
 					problems = append(problems, fmt.Sprintf(
-						"%d nodes provide %q, wanted by %s — say which with `pin %s %s <node>`: %s",
+						"%d providers of %q, wanted by %s — say which with `pin %s %s <node> <module>`: %s",
 						len(where), want, because[want], node.Name, want,
 						strings.Join(names, ", ")))
 					break
 				}
-				var chosen *Provider
-				for i, w := range where {
-					if w.Node == chosenNode {
-						chosen = &where[i]
-					}
-				}
-				if chosen == nil {
-					// Pointed at a machine that does not answer this. Refused rather than
+				matching := c.among(where)
+				switch len(matching) {
+				case 0:
+					// Pointed at a provider that does not answer this. Refused rather than
 					// falling back to another: a fallback would quietly move somebody's data to
 					// a machine they did not choose, which is the whole reason this is asked.
 					problems = append(problems, fmt.Sprintf(
 						"%s was told to get %q from %s, and %s does not provide it — these do: %s",
-						node.Name, want, chosenNode, chosenNode, strings.Join(names, ", ")))
-					break
+						node.Name, want, c, c, strings.Join(names, ", ")))
+				case 1:
+					take(matching[0])
+				default:
+					// A record naming only the node, from before a pin named the module, and that
+					// node answers twice. This once took the last one listed (novox/hq #258): a
+					// coin flip, handed to whoever reads the certificate it chose.
+					problems = append(problems, fmt.Sprintf(
+						"%s was told to get %q from %s, and %s provides it %d times — say which with "+
+							"`pin %s %s %s <module>`: %s",
+						node.Name, want, c, c.Node, len(matching), node.Name, want, c.Node,
+						strings.Join(providerNames(matching), ", ")))
 				}
-				take(*chosen)
 			}
 			continue
 		}
@@ -597,31 +640,6 @@ func Resolve(catalogue map[string]Manifest, assigned []string, node Node, world
 // need that is never created is a binding the consumer never gets. It is right about that from the
 // manifest alone, which is why walking the catalogue mid-resolution is enough here and is not
 // enough for the values.
-// sharedHere is the own secret the provider of a provision on this same machine names as its
-// credential (ADR 0158), or "" when the provider gives each consumer its own.
-func sharedHere(catalogue map[string]Manifest, chosen map[string]bool, want string) string {
-	for name, m := range catalogue {
-		if !chosen[name] {
-			continue
-		}
-		if own, shared := m.SharedCredentialOf(want); shared {
-			return own
-		}
-	}
-	return ""
-}
-
-func servedHere(catalogue map[string]Manifest, chosen map[string]bool, want string) map[string]any {
-	for name, m := range catalogue {
-		if !chosen[name] {
-			continue
-		}
-		if _, ok := m.Serves[want]; ok {
-			return ServedOn(m, want, nil)
-		}
-	}
-	return nil
-}
 
 // isModule reports whether a name is a module in its own right rather than only something
 // modules provide.

internal/catalogue/seats_test.go

@@ -249,7 +249,7 @@ func TestAPinStillWinsOverTheSeat(t *testing.T) {
 	// A consumer coupled to one provider's contents has said so, and the seat does not overrule it.
 	got, err := Resolve(registryShelf(), []string{"builder"}, reachable(),
 		World{Offered: twoRegistries(), Held: giteaHoldsTheSeat(),
-			Pinned: map[string]string{"npm-package-registry": "archive"}})
+			Pinned: map[string]Chosen{"npm-package-registry": {Node: "archive", Module: "verdaccio"}}})
 	if err != nil {
 		t.Fatal(err)
 	}

internal/catalogue/verbs.go

@@ -97,6 +97,16 @@ var ControllerVerbs = []Verb{
 		Input: schema(map[string]string{"node": "the machine's name", "module": "the module's name"}, []string{"node", "module"})},
 	{Name: "unassign", Description: "Take a module off a machine.",
 		Input: schema(map[string]string{"node": "the machine's name", "module": "the module's name"}, []string{"node", "module"})},
+	{Name: "pin", Description: "Tell a machine which provider answers a provision for it — the module, and the node " +
+		"it runs on, both. Asked for when more than one could answer; the refusal lists them.",
+		Input: schema(map[string]string{
+			"node":      "the machine's name",
+			"provision": "the provision, as the consumer requires it",
+			"from":      "the node the chosen provider runs on",
+			"module":    "the module providing it there",
+		}, []string{"node", "provision", "from", "module"})},
+	{Name: "unpin", Description: "Take that choice back, putting the question to the mesh again.",
+		Input: schema(map[string]string{"node": "the machine's name", "provision": "the provision"}, []string{"node", "provision"})},
 	{Name: "push", Description: "Send a machine everything it should be — or every machine that is behind, when no machine is named.",
 		Input: schema(map[string]string{"node": "the machine's name; every machine behind when absent"}, nil)},
 	{Name: "rotate", Description: "Replace a credential. A pair credential, by provision (and a consuming machine, " +

internal/inventory/catalogue.go

@@ -841,24 +841,28 @@ func (i *Inventory) SettingsFor(ctx context.Context, nodeName, module string) ([
 	return layers, rows.Err()
 }
 
-// PinProvision records which node a machine gets a provision from.
+// PinProvision records which provider a machine gets a provision from: a module, and the node it
+// runs on — both, always (novox/hq #258). A provision comes from a module, and the same module on
+// two machines is two answers, so neither half alone says which.
 //
-// Only needed when more than one node could answer. Recordable before that, because a mesh with
-// one database should not change where an existing machine gets its data the day a second
-// arrives.
-func (i *Inventory) PinProvision(ctx context.Context, nodeName, provision, provider string) error {
+// Only needed when more than one could answer. Recordable before that, because a mesh with one
+// database should not change where an existing machine gets its data the day a second arrives.
+func (i *Inventory) PinProvision(ctx context.Context, nodeName, provision, providerNode, module string) error {
+	if strings.TrimSpace(module) == "" {
+		return fmt.Errorf("a pin names the module providing %q as well as the node it runs on", provision)
+	}
 	node, err := i.NodeByName(ctx, nodeName)
 	if err != nil {
 		return err
 	}
-	from, err := i.NodeByName(ctx, provider)
+	from, err := i.NodeByName(ctx, providerNode)
 	if err != nil {
 		return err
 	}
 	_, err = i.store.Pool().Exec(ctx,
-		`insert into provision_pin (node, name, provider) values ($1, $2, $3)
-		 on conflict (node, name) do update set provider = excluded.provider, pinned_at = now()`,
-		node.ID, provision, from.ID)
+		`insert into provision_pin (node, name, provider, module) values ($1, $2, $3, $4)
+		 on conflict (node, name) do update set provider = excluded.provider, module = excluded.module, pinned_at = now()`,
+		node.ID, provision, from.ID, module)
 	return err
 }
 
@@ -879,27 +883,28 @@ func (i *Inventory) UnpinProvision(ctx context.Context, nodeName, provision stri
 	return nil
 }
 
-// PinsFor is what a node was told about where its provisions come from.
-func (i *Inventory) PinsFor(ctx context.Context, nodeName string) (map[string]string, error) {
+// PinsFor is what a node was told about where its provisions come from. A record from before a pin
+// named the module carries the node alone; the resolver honours it while it is unambiguous.
+func (i *Inventory) PinsFor(ctx context.Context, nodeName string) (map[string]catalogue.Chosen, error) {
 	node, err := i.NodeByName(ctx, nodeName)
 	if err != nil {
 		return nil, err
 	}
 	rows, err := i.store.Pool().Query(ctx,
-		`select p.name, n.name from provision_pin p join node n on n.id = p.provider
+		`select p.name, n.name, coalesce(p.module, '') from provision_pin p join node n on n.id = p.provider
 		 where p.node = $1`, node.ID)
 	if err != nil {
 		return nil, err
 	}
 	defer rows.Close()
 
-	out := map[string]string{}
+	out := map[string]catalogue.Chosen{}
 	for rows.Next() {
-		var name, provider string
-		if err := rows.Scan(&name, &provider); err != nil {
+		var name, provider, module string
+		if err := rows.Scan(&name, &provider, &module); err != nil {
 			return nil, err
 		}
-		out[name] = provider
+		out[name] = catalogue.Chosen{Node: provider, Module: module}
 	}
 	return out, rows.Err()
 }

internal/inventory/catalogue_test.go

@@ -385,19 +385,19 @@ func TestAPinSurvivesAndCanBeChanged(t *testing.T) {
 			t.Fatal(err)
 		}
 	}
-	if err := inv.PinProvision(ctx, "user", "postgres-database", "first"); err != nil {
+	if err := inv.PinProvision(ctx, "user", "postgres-database", "first", "postgres"); err != nil {
 		t.Fatal(err)
 	}
 	// Changing the answer replaces it rather than adding a second, or a machine would be told to
 	// use two databases and nothing would say which.
-	if err := inv.PinProvision(ctx, "user", "postgres-database", "second"); err != nil {
+	if err := inv.PinProvision(ctx, "user", "postgres-database", "second", "postgres"); err != nil {
 		t.Fatal(err)
 	}
 	pins, err := inv.PinsFor(ctx, "user")
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(pins) != 1 || pins["postgres-database"] != "second" {
+	if len(pins) != 1 || pins["postgres-database"].Node != "second" || pins["postgres-database"].Module != "postgres" {
 		t.Fatalf("got %v", pins)
 	}
 	if err := inv.UnpinProvision(ctx, "user", "postgres-database"); err != nil {
@@ -422,7 +422,7 @@ func TestAPinGoesWhenTheProviderLeavesTheMesh(t *testing.T) {
 			t.Fatal(err)
 		}
 	}
-	if err := inv.PinProvision(ctx, "consumer", "postgres-database", "provider"); err != nil {
+	if err := inv.PinProvision(ctx, "consumer", "postgres-database", "provider", "postgres"); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := inv.store.Pool().Exec(ctx, `delete from node where name = 'provider'`); err != nil {

internal/inventory/migrations/0050-a-pin-names-the-module.sql

@@ -0,0 +1,12 @@
+-- A pin names the module as well as the node (novox/hq #258).
+--
+-- 0008 said "not a module: the same module on two machines is two answers, and which machine is the
+-- whole question". Half right. Two modules on one machine can both answer a provision — public-acme
+-- and step-ca both offer acme-ca on novox — and then which *module* is the whole question, and a
+-- node alone cannot ask it. The resolver, given a node that answered twice, took the last one listed.
+--
+-- A provider is a (node, module) pair (design 23), and a pin names the pair. Nullable, so a record
+-- made before this was asked keeps meaning what it meant: honoured while that node answers once,
+-- refused with the module asked for when it answers twice.
+alter table provision_pin add column module text;
+

internal/inventory/migrations/0051-a-pin-made-before-is-completed.sql

@@ -0,0 +1,19 @@
+-- The records already made are completed where the mesh can tell: a pin naming a node on which
+-- exactly one assigned module offers the provision (or is the module itself, for a requirement that
+-- names a module) gets that module. A node that answers twice is left to say which — the resolver
+-- refuses it with the module asked for, rather than this guessing on its behalf.
+update provision_pin p
+   set module = sub.module
+  from (
+        select p2.node, p2.name, min(a.module) as module, count(distinct a.module) as answers
+          from provision_pin p2
+          join assignment a on a.node = p2.provider
+          join module m on m.name = a.module
+         where p2.module is null
+           and (a.module = p2.name
+                or exists (select 1
+                             from jsonb_array_elements(coalesce(m.manifest -> 'provides', '[]'::jsonb)) e
+                            where (case when jsonb_typeof(e) = 'string' then e #>> '{}' else e ->> 'name' end) = p2.name))
+         group by p2.node, p2.name
+       ) sub
+ where sub.node = p.node and sub.name = p.name and sub.answers = 1;

internal/inventory/pin_completed_test.go

@@ -0,0 +1,86 @@
+package inventory
+
+import (
+	"context"
+	"os"
+	"testing"
+)
+
+// A pin made before it named the module (migration 0051, novox/hq #258): completed where the node it
+// names answers once, left for a person where it answers twice.
+
+func legacyPin(t *testing.T, inv *Inventory, node, provision, provider string) {
+	t.Helper()
+	_, err := inv.store.Pool().Exec(context.Background(),
+		`insert into provision_pin (node, name, provider)
+		 select u.id, $2, p.id from node u, node p where u.name = $1 and p.name = $3`,
+		node, provision, provider)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func completeEarlierPins(t *testing.T, inv *Inventory) {
+	t.Helper()
+	sql, err := os.ReadFile("migrations/0051-a-pin-made-before-is-completed.sql")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := inv.store.Pool().Exec(context.Background(), string(sql)); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestAPinMadeBeforeIsCompletedWhenTheNodeAnswersOnce(t *testing.T) {
+	inv := fresh(t)
+	ctx := context.Background()
+	for _, n := range []string{"user", "provider"} {
+		if _, err := inv.AddNode(ctx, n); err != nil {
+			t.Fatal(err)
+		}
+	}
+	for _, m := range []string{"postgres", "redis"} {
+		if err := inv.RegisterModule(ctx, manifest(m, []string{m + "-database"}, nil), Source{}); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := inv.Assign(ctx, "provider", m); err != nil {
+			t.Fatal(err)
+		}
+	}
+	legacyPin(t, inv, "user", "postgres-database", "provider")
+	completeEarlierPins(t, inv)
+	pins, err := inv.PinsFor(ctx, "user")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := pins["postgres-database"]; got.Node != "provider" || got.Module != "postgres" {
+		t.Fatalf("the record was not completed with the one module that answers: %+v", got)
+	}
+}
+
+func TestAPinMadeBeforeIsLeftOpenWhenTheNodeAnswersTwice(t *testing.T) {
+	inv := fresh(t)
+	ctx := context.Background()
+	for _, n := range []string{"user", "provider"} {
+		if _, err := inv.AddNode(ctx, n); err != nil {
+			t.Fatal(err)
+		}
+	}
+	for _, m := range []string{"public-acme", "step-ca"} {
+		if err := inv.RegisterModule(ctx, manifest(m, []string{"acme-ca"}, nil), Source{}); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := inv.Assign(ctx, "provider", m); err != nil {
+			t.Fatal(err)
+		}
+	}
+	legacyPin(t, inv, "user", "acme-ca", "provider")
+	completeEarlierPins(t, inv)
+	pins, err := inv.PinsFor(ctx, "user")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := pins["acme-ca"]; got.Node != "provider" || got.Module != "" {
+		t.Fatalf("a node that answers twice was guessed for: %+v", got)
+	}
+}