diff --git a/cmd/mesh-control/modules.go b/cmd/mesh-control/modules.go index c5e4f46..96e2c3b 100644 --- a/cmd/mesh-control/modules.go +++ b/cmd/mesh-control/modules.go @@ -188,13 +188,37 @@ func moduleCommand(ctx context.Context, args []string) error { return nil case "forget": - if len(args) != 2 { - return errors.New("module forget ") - } - if err := inv.ForgetModule(ctx, args[1]); err != nil { + // **What goes with it is said before it goes** (novox/hq 04-ISSUES/017). The settings, the + // module's own secrets and the ports the mesh chose all cascade off the module row, so + // `forget` used to destroy them and report "forgotten" — an action succeeding into a state + // its own verify would reject, and a sealed secret is not recoverable afterwards. + set := flag.NewFlagSet("module forget", flag.ContinueOnError) + andHeld := set.Bool("and-what-it-holds", false, + "discard its settings, its own secrets and its ports along with it") + positionals, err := parseAround(set, args[1:]) + if err != nil { return err } - fmt.Printf("%s forgotten\n", args[1]) + if len(positionals) != 1 { + return errors.New("module forget [--and-what-it-holds]") + } + if !*andHeld { + if err := inv.ForgetModule(ctx, positionals[0]); err != nil { + return err + } + fmt.Printf("%s forgotten\n", positionals[0]) + return nil + } + held, err := inv.DiscardModule(ctx, positionals[0]) + if err != nil { + return err + } + fmt.Printf("%s forgotten\n", positionals[0]) + for _, line := range held.Lines() { + // Said after the fact as well as before it: this is the only record that these + // existed, and the next person to ask why the module came back empty reads it here. + fmt.Printf(" discarded%s\n", strings.TrimPrefix(line, " ")) + } return nil case "issue": diff --git a/internal/inventory/catalogue.go b/internal/inventory/catalogue.go index 39a849a..04e6e2f 100644 --- a/internal/inventory/catalogue.go +++ b/internal/inventory/catalogue.go @@ -204,10 +204,165 @@ func (i *Inventory) Provided(ctx context.Context, name string) (bool, error) { return source != nil && *source == "the control plane", nil } -// ForgetModule removes a module, unless a machine is running it, and never one the control plane -// provides. +// ErrStillHolds is why a module cannot be forgotten without saying so first. +// +// Its own error because it is not a fault either: the operator settings, the module's own secrets +// and the ports the mesh chose for it are all keyed on the module by name and all cascade when the +// row goes. Removing the module removes them, silently, and none of them can be recovered — a +// sealed secret least of all, because the mesh discarded the plaintext when it made it. +var ErrStillHolds = errors.New("the mesh still holds things for that module") + +// Holdings is everything keyed on a module that would go with it. +// +// **Named one at a time rather than counted.** "3 settings" tells somebody there is something to +// lose and not whether they can afford to lose it; "the mesh-wide layer, and anchor's" tells them +// what to write down before they type the command again. +type Holdings struct { + // Mesh is true when a mesh-wide settings layer exists for the module. + Mesh bool + // Nodes are the machines with a settings layer of their own for it, sorted. + Nodes []string + // Secrets are the module's own secrets, as " on ", sorted. These are the ones the + // mesh cannot make again: what is stored is sealed to a machine and the plaintext is gone. + Secrets []string + // Ports are the ports the mesh chose for it, as " on ", sorted. Made once and + // kept (novox/hq ADR 0038) — removing the module gives that promise up. + Ports []string +} + +// Any reports whether removing the module would discard anything. +func (h Holdings) Any() bool { + return h.Mesh || len(h.Nodes) > 0 || len(h.Secrets) > 0 || len(h.Ports) > 0 +} + +// Lines is what would be lost, one thing per line, for a person about to decide. +func (h Holdings) Lines() []string { + var out []string + if h.Mesh { + out = append(out, " settings, for the whole mesh") + } + for _, n := range h.Nodes { + out = append(out, " settings, on "+n) + } + for _, s := range h.Secrets { + out = append(out, " its own secret "+s+" — sealed, so the mesh cannot make it again") + } + for _, p := range h.Ports { + out = append(out, " the port "+p) + } + return out +} + +// HeldFor is everything the mesh keeps that is keyed on one module. +// +// Read rather than counted at the moment of removal, because the answer is the whole of what a +// person needs in order to say yes. +func (i *Inventory) HeldFor(ctx context.Context, name string) (Holdings, error) { + var held Holdings + rows, err := i.store.Pool().Query(ctx, + `select coalesce(n.name, '') from settings s left join node n on n.id = s.node + where s.module = $1 order by n.name nulls first`, name) + if err != nil { + return Holdings{}, err + } + for rows.Next() { + var node string + if err := rows.Scan(&node); err != nil { + rows.Close() + return Holdings{}, err + } + if node == "" { + held.Mesh = true + continue + } + held.Nodes = append(held.Nodes, node) + } + rows.Close() + if err := rows.Err(); err != nil { + return Holdings{}, err + } + + for _, read := range []struct { + query string + into *[]string + }{ + {`select s.name || ' on ' || n.name from module_secret s join node n on n.id = s.node + where s.module = $1 order by n.name, s.name`, &held.Secrets}, + {`select p.wanted::text || ' on ' || n.name from port_assignment p + join node n on n.id = p.node where p.module = $1 order by n.name, p.wanted`, &held.Ports}, + } { + rows, err := i.store.Pool().Query(ctx, read.query, name) + if err != nil { + return Holdings{}, err + } + for rows.Next() { + var one string + if err := rows.Scan(&one); err != nil { + rows.Close() + return Holdings{}, err + } + *read.into = append(*read.into, one) + } + rows.Close() + if err := rows.Err(); err != nil { + return Holdings{}, err + } + } + return held, nil +} + +// ForgetModule removes a module, unless a machine is running it, unless the mesh still holds +// things for it, and never one the control plane provides. +// +// **Refused rather than cascaded.** The settings, own-secrets and port assignments all name the +// module by a foreign key that cascades, so the row going takes them with it and says nothing. +// That is an action succeeding into a state its own verify would reject (novox/hq 04-ISSUES/017): +// the command reports "forgotten", the operator re-registers the module a moment later, and what +// comes back is a module with none of its configuration and none of its secrets — with nothing +// anywhere naming the moment they were lost. func (i *Inventory) ForgetModule(ctx context.Context, name string) error { + if err := i.mayForget(ctx, name); err != nil { + return err + } + held, err := i.HeldFor(ctx, name) + if err != nil { + return err + } + if held.Any() { + return fmt.Errorf("%w:\n%s\n\nAll of it goes when the module does. Run "+ + "`module forget %s --and-what-it-holds` if that is what you mean", + ErrStillHolds, strings.Join(held.Lines(), "\n"), name) + } + return i.discard(ctx, name) +} + +// DiscardModule removes a module and everything the mesh holds for it, having been told to. +// +// The same checks as ForgetModule except the one about what is held: a machine running it still +// refuses, and a module the control plane provides still refuses, because neither of those is +// something an operator can consent to on the module's behalf. +func (i *Inventory) DiscardModule(ctx context.Context, name string) (Holdings, error) { + if err := i.mayForget(ctx, name); err != nil { + return Holdings{}, err + } + held, err := i.HeldFor(ctx, name) + if err != nil { + return Holdings{}, err + } + if err := i.discard(ctx, name); err != nil { + return Holdings{}, err + } + // Returned so the caller can say what went, rather than "forgotten". A person who has just + // destroyed a sealed secret should be able to read which one from the output. + return held, nil +} + +// mayForget is the part of forgetting that is not about what is held. +func (i *Inventory) mayForget(ctx context.Context, name string) error { provided, err := i.Provided(ctx, name) + if errors.Is(err, pgx.ErrNoRows) { + return fmt.Errorf("%w: %s", ErrNoSuchModule, name) + } if err != nil { return err } @@ -235,10 +390,17 @@ func (i *Inventory) ForgetModule(ctx context.Context, name string) error { on = append(on, node) } rows.Close() + if err := rows.Err(); err != nil { + return err + } if len(on) > 0 { return fmt.Errorf("%w: %s. Unassign it first", ErrStillAssigned, strings.Join(on, ", ")) } + return nil +} +// discard is the removal itself, once it has been decided. +func (i *Inventory) discard(ctx context.Context, name string) error { tag, err := i.store.Pool().Exec(ctx, `delete from module where name = $1`, name) if err != nil { return err diff --git a/internal/inventory/forget_test.go b/internal/inventory/forget_test.go new file mode 100644 index 0000000..31e3b02 --- /dev/null +++ b/internal/inventory/forget_test.go @@ -0,0 +1,226 @@ +package inventory + +import ( + "errors" + "strings" + "testing" + + "github.com/novox/mesh-control/internal/catalogue" +) + +// What removing a module takes with it, and what re-registering one does not. +// +// Both were reported as one fault — "operator settings do not persist, because re-registering a +// module cascade-deletes them". Only half of it was true, and it was the other half. + +// **Registering a module again does not touch what the mesh holds for it.** The upsert is on the +// name, so a manifest changing — a new version, a new requirement, a new resource — leaves the +// settings, the secrets and the ports exactly where they were. +// +// This is here because it was believed not to be. A wrong belief about which command destroys data +// is expensive in both directions: it sends people looking for a fault that is not there, and it +// leaves the command that really does destroy it unexamined. +func TestRegisteringAModuleAgainKeepsWhatTheMeshHoldsForIt(t *testing.T) { + inv := fresh(t) + ctx := t.Context() + node, err := inv.AddNode(ctx, "anchor") + if err != nil { + t.Fatal(err) + } + key, _ := aSealingKey(t) + if err := inv.RecordSealingKey(ctx, node.ID, key); err != nil { + t.Fatal(err) + } + m := catalogue.Manifest{Module: "step-ca", Version: "1", + Provides: catalogue.Offers("acme-ca")} + if err := inv.RegisterModule(ctx, m, Source{}); err != nil { + t.Fatal(err) + } + if err := inv.SetSettings(ctx, "", "step-ca", map[string]any{"issuer": "the mesh"}); err != nil { + t.Fatal(err) + } + if err := inv.SetSettings(ctx, "anchor", "step-ca", map[string]any{"port": 9000}); err != nil { + t.Fatal(err) + } + if err := inv.AcceptSecretForModule(ctx, "anchor", "step-ca", "password", "sealed"); err != nil { + t.Fatal(err) + } + if _, err := inv.PortFor(ctx, "anchor", "step-ca", 9000, false); err != nil { + t.Fatal(err) + } + + // Re-registered at a new version, which is exactly what somebody bumping one does. + m.Version = "2" + if err := inv.RegisterModule(ctx, m, Source{}); err != nil { + t.Fatal(err) + } + + layers, err := inv.SettingsFor(ctx, "anchor", "step-ca") + if err != nil { + t.Fatal(err) + } + if len(layers) != 2 { + t.Fatalf("re-registering lost a settings layer: %+v", layers) + } + held, err := inv.HeldFor(ctx, "step-ca") + if err != nil { + t.Fatal(err) + } + if !held.Mesh || len(held.Nodes) != 1 || len(held.Secrets) != 1 || len(held.Ports) != 1 { + t.Fatalf("re-registering lost something the mesh held: %+v", held) + } + + // And the new manifest is what resolution sees, which is the point of re-registering at all. + shelf, err := inv.Catalogue(ctx) + if err != nil { + t.Fatal(err) + } + if shelf["step-ca"].Version != "2" { + t.Fatalf("the new manifest did not replace the old: %+v", shelf["step-ca"]) + } + if len(shelf["step-ca"].Provides) != 1 || shelf["step-ca"].Provides[0].Name != "acme-ca" { + // A version bump was reported as un-providing a provision. It does not. + t.Fatalf("a version bump changed what the module provides: %+v", shelf["step-ca"].Provides) + } +} + +// **Forgetting a module refuses while the mesh still holds things for it, and says what they are.** +// +// The settings, the module's own secrets and the ports the mesh chose all name the module by a +// foreign key that cascades. So the removal took them, silently, and reported "forgotten" — an +// action succeeding into a state its own verify would reject (novox/hq 04-ISSUES/017). A sealed +// secret is not recoverable afterwards: the mesh discarded the plaintext when it made it. +func TestForgettingAModuleRefusesRatherThanDiscardingWhatTheMeshHolds(t *testing.T) { + inv := fresh(t) + ctx := t.Context() + node, err := inv.AddNode(ctx, "anchor") + if err != nil { + t.Fatal(err) + } + key, _ := aSealingKey(t) + if err := inv.RecordSealingKey(ctx, node.ID, key); err != nil { + t.Fatal(err) + } + if err := inv.RegisterModule(ctx, manifest("step-ca", nil, nil), Source{}); err != nil { + t.Fatal(err) + } + if err := inv.SetSettings(ctx, "anchor", "step-ca", map[string]any{"port": 9000}); err != nil { + t.Fatal(err) + } + if err := inv.AcceptSecretForModule(ctx, "anchor", "step-ca", "password", "sealed"); err != nil { + t.Fatal(err) + } + if _, err := inv.PortFor(ctx, "anchor", "step-ca", 9000, false); err != nil { + t.Fatal(err) + } + + err = inv.ForgetModule(ctx, "step-ca") + if !errors.Is(err, ErrStillHolds) { + t.Fatalf("forgetting discarded what the mesh held, or refused for another reason: %v", err) + } + // It names them one at a time. "3 things" says there is something to lose and not whether it + // can be afforded; the sealed secret is the one that cannot be made again, and it is named. + for _, want := range []string{"settings, on anchor", "password on anchor", + "the mesh cannot make it again", "the port 9000 on anchor", "--and-what-it-holds"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the refusal does not say %q:\n%s", want, err) + } + } + // And nothing went. A refusal that half-happened would be worse than the cascade. + shelf, err := inv.Catalogue(ctx) + if err != nil { + t.Fatal(err) + } + if _, still := shelf["step-ca"]; !still { + t.Fatal("the module was removed by a command that refused") + } + layers, err := inv.SettingsFor(ctx, "anchor", "step-ca") + if err != nil { + t.Fatal(err) + } + if len(layers) != 1 { + t.Fatalf("a refusal took the settings anyway: %+v", layers) + } +} + +// Having been told, it goes — and what went is reported, because this is the only record that any +// of it existed. +func TestDiscardingAModuleSaysWhatWentWithIt(t *testing.T) { + inv := fresh(t) + ctx := t.Context() + node, err := inv.AddNode(ctx, "anchor") + if err != nil { + t.Fatal(err) + } + key, _ := aSealingKey(t) + if err := inv.RecordSealingKey(ctx, node.ID, key); err != nil { + t.Fatal(err) + } + if err := inv.RegisterModule(ctx, manifest("step-ca", nil, nil), Source{}); err != nil { + t.Fatal(err) + } + if err := inv.SetSettings(ctx, "", "step-ca", map[string]any{"issuer": "the mesh"}); err != nil { + t.Fatal(err) + } + if err := inv.AcceptSecretForModule(ctx, "anchor", "step-ca", "password", "sealed"); err != nil { + t.Fatal(err) + } + + held, err := inv.DiscardModule(ctx, "step-ca") + if err != nil { + t.Fatal(err) + } + if !held.Mesh || len(held.Secrets) != 1 { + t.Fatalf("what went was not reported: %+v", held) + } + shelf, err := inv.Catalogue(ctx) + if err != nil { + t.Fatal(err) + } + if _, still := shelf["step-ca"]; still { + t.Fatal("the module is still there") + } +} + +// A module the mesh holds nothing for is forgotten without ceremony. The refusal is about loss, +// not about the command. +func TestForgettingAModuleTheMeshHoldsNothingForJustWorks(t *testing.T) { + inv := fresh(t) + ctx := t.Context() + if _, err := inv.AddNode(ctx, "anchor"); err != nil { + t.Fatal(err) + } + if err := inv.RegisterModule(ctx, manifest("plain", nil, nil), Source{}); err != nil { + t.Fatal(err) + } + if err := inv.ForgetModule(ctx, "plain"); err != nil { + t.Fatalf("a module holding nothing was refused: %v", err) + } +} + +// A module still on a machine refuses first, whatever it holds: that is not a loss an operator can +// consent to on the machine's behalf, and unassign is what "I do not want this" means. +func TestAModuleStillAssignedRefusesBeforeAnythingAboutWhatItHolds(t *testing.T) { + inv := fresh(t) + ctx := t.Context() + if _, err := inv.AddNode(ctx, "anchor"); err != nil { + t.Fatal(err) + } + if err := inv.RegisterModule(ctx, manifest("step-ca", nil, nil), Source{}); err != nil { + t.Fatal(err) + } + if err := inv.SetSettings(ctx, "anchor", "step-ca", map[string]any{"a": 1}); err != nil { + t.Fatal(err) + } + if err := inv.Assign(ctx, "anchor", "step-ca"); err != nil { + t.Fatal(err) + } + for _, forget := range []func() error{ + func() error { return inv.ForgetModule(ctx, "step-ca") }, + func() error { _, err := inv.DiscardModule(ctx, "step-ca"); return err }, + } { + if err := forget(); !errors.Is(err, ErrStillAssigned) { + t.Fatalf("an assigned module was not refused for being assigned: %v", err) + } + } +}