diff --git a/cmd/mesh-controller/main.go b/cmd/mesh-controller/main.go index 1650403..4f6ce60 100644 --- a/cmd/mesh-controller/main.go +++ b/cmd/mesh-controller/main.go @@ -203,7 +203,8 @@ func usage() { licence refresh mint a new access token and seal it to every holder rotate [--consumer ] a new credential for every holder, both ends at once ask [json] call one of a module's tools over the broker, and print its answer - pin which node this one gets a provision from + pin + which provider this one gets a provision from: the module, and its node unpin put that question back plan [--files|--json] what that node would run, and why push [] [--behind] send a node everything it should be, or only those that need it diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index 111ce4d..9ad3ecb 100644 --- a/cmd/mesh-controller/modules.go +++ b/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 ") + if setting && len(args) != 4 { + return errors.New("pin ") } if !setting && len(args) != 2 { return errors.New("unpin ") @@ -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 } diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 3b3c608..5e7f0b6 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/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 +} diff --git a/internal/catalogue/brokered_test.go b/internal/catalogue/brokered_test.go index 09d4cf2..c379f71 100644 --- a/internal/catalogue/brokered_test.go +++ b/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) } } diff --git a/internal/catalogue/chosen.go b/internal/catalogue/chosen.go new file mode 100644 index 0000000..1fb8233 --- /dev/null +++ b/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 +} diff --git a/internal/catalogue/pin_names_module_test.go b/internal/catalogue/pin_names_module_test.go new file mode 100644 index 0000000..ad7c893 --- /dev/null +++ b/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", " "} { + 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 ") { + 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) + } +} diff --git a/internal/catalogue/resolve.go b/internal/catalogue/resolve.go index db82d16..74be21e 100644 --- a/internal/catalogue/resolve.go +++ b/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 `: %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 `: %s", + "%d providers of %q, wanted by %s — say which with `pin %s %s `: %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 `: %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. diff --git a/internal/catalogue/seats_test.go b/internal/catalogue/seats_test.go index f30a92e..128b01c 100644 --- a/internal/catalogue/seats_test.go +++ b/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) } diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index cb96ed0..4d573a5 100644 --- a/internal/catalogue/verbs.go +++ b/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, " + diff --git a/internal/inventory/catalogue.go b/internal/inventory/catalogue.go index f63597b..cbf8a90 100644 --- a/internal/inventory/catalogue.go +++ b/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() } diff --git a/internal/inventory/catalogue_test.go b/internal/inventory/catalogue_test.go index 1fd4e9e..9c0f974 100644 --- a/internal/inventory/catalogue_test.go +++ b/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 { diff --git a/internal/inventory/migrations/0050-a-pin-names-the-module.sql b/internal/inventory/migrations/0050-a-pin-names-the-module.sql new file mode 100644 index 0000000..141065f --- /dev/null +++ b/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; + diff --git a/internal/inventory/migrations/0051-a-pin-made-before-is-completed.sql b/internal/inventory/migrations/0051-a-pin-made-before-is-completed.sql new file mode 100644 index 0000000..c016dbe --- /dev/null +++ b/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; diff --git a/internal/inventory/pin_completed_test.go b/internal/inventory/pin_completed_test.go new file mode 100644 index 0000000..e090993 --- /dev/null +++ b/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) + } +}