From a3e8a4185ba0b28176864d4abd726bfa20e1e238 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 3 Oct 2026 04:01:17 +0200 Subject: [PATCH] An assignment issues its bus credential, a push refuses one nobody issued, and what reads a secret restarts on it (hq issue 203) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `assign` recorded a module and `push` sealed a random own secret where its bus credential belongs; the process crash-looped until a person ran `module issue` and pushed again, and the only warning was one line in a list printed on every push. Now assigning a module that declares a broker secret issues the credential in the same act — kept when one exists, so re-assigning rotates nothing — and when the bus cannot be reached from here the assignment says which verb to run. A push never seals a placeholder in a credential's place: a module whose bus user is unminted is refused by name, with the verb. The control plane's own user is the installer's, seeded at genesis, which the test now says. And what reads one of a module's own secrets is restarted when it changes — composed for a container or daemon that names the secret's path in its volumes, environment or env-files, so a manifest need not say it: the build machine ran on an hour-old credential because its manifest restarted it on its environment file alone (issue 206). A scheduled or run-once process is left alone; it reads afresh. --- cmd/mesh-controller/acts.go | 38 ++++++++ cmd/mesh-controller/addresses_test.go | 6 ++ .../credential_on_assign_test.go | 90 ++++++++++++++++++ cmd/mesh-controller/plan.go | 17 ++++ internal/catalogue/declaration.go | 93 +++++++++++++++++++ internal/catalogue/secret_restart_test.go | 52 +++++++++++ 6 files changed, 296 insertions(+) create mode 100644 cmd/mesh-controller/credential_on_assign_test.go create mode 100644 internal/catalogue/secret_restart_test.go diff --git a/cmd/mesh-controller/acts.go b/cmd/mesh-controller/acts.go index 9644a76..e51e99c 100644 --- a/cmd/mesh-controller/acts.go +++ b/cmd/mesh-controller/acts.go @@ -3,6 +3,7 @@ package main import ( "context" "fmt" + "github.com/novox/mesh-controller/internal/broker" "sort" "strings" @@ -67,6 +68,13 @@ func assign(ctx context.Context, open *stores, node, module string) (string, err for _, line := range settled { said += "\n " + line } + // Its bus credential, in the same act (novox/hq issue 203): an assignment pushed before its + // credential exists delivers a process that cannot authenticate and crash-loops until somebody + // runs a second verb and a second push. Issued here when the module speaks on the bus and has + // no credential yet; kept when it has one, so re-assigning rotates nothing. + if line := issueOnAssign(ctx, open, node, module); line != "" { + said += "\n " + line + } plan, _, err := planFor(ctx, open, node) if err != nil { // Kept, and still refused. Both halves are the answer, and the rest of the mesh is still @@ -152,3 +160,33 @@ func blockedElsewhere(ctx context.Context, open *stores, except string) string { out.WriteString("\nThis may or may not be what just changed — it is what is true now.") return out.String() } + +// issueOnAssign gives a newly assigned module its bus credential, the way `module issue` does, and +// says what it did in one line. Nothing for a module that declares no broker secret; nothing for one +// whose user is already minted (a credential is rotated on purpose, never by re-assigning); and when +// the bus cannot be reached from here, the line names the verb and the push that would refuse the +// module until it is run — never a silent placeholder (novox/hq issue 203). +func issueOnAssign(ctx context.Context, open *stores, node, module string) string { + inv := open.inventory + shelf, err := inv.Catalogue(ctx) + if err != nil { + return "" + } + m, known := shelf[module] + if !known || mayIssue(m) != nil { + return "" + } + user := broker.Principal{Kind: broker.KindModule, Node: node, Module: module}.Username() + if _, minted, err := inv.BusUserHash(ctx, user); err != nil || minted { + return "" + } + busAddress, err := broker.BusAddress() + if err == nil { + err = issueOnTheNewBus(ctx, inv, m, node, busAddress) + } + if err != nil { + return fmt.Sprintf("its bus credential is not issued (%v): `module issue %s --node %s` first — "+ + "`push %s` refuses to send %s until it is", err, module, node, node, module) + } + return fmt.Sprintf("its bus credential is issued and sealed to %s, and arrives with the push", node) +} diff --git a/cmd/mesh-controller/addresses_test.go b/cmd/mesh-controller/addresses_test.go index 39ec4ec..5a23d2e 100644 --- a/cmd/mesh-controller/addresses_test.go +++ b/cmd/mesh-controller/addresses_test.go @@ -175,6 +175,12 @@ func TestTheControlPlaneIsToldWhereTheNodePutTheStoreAndTheBroker(t *testing.T) Guards: []int{15672}, Resources: []map[string]any{{"id": "server", "type": "container", "name": "mesh-broker", "ports": []any{"5671:5671", "5672:5672", "127.0.0.1:15672:15672"}, "image": "mq@" + aDigest}}}) + // The control plane's own bus user is the installer's, seeded at genesis before the controller + // runs (SeedBusUser); without it a push now refuses the credential nobody issued (issue 203). + if err := open.inventory.SeedBusUser(ctx, inventory.BusUser{Username: "anchor.mesh-controller", + Kind: inventory.BusController, Node: "anchor", Module: "mesh-controller"}, "bootstrap"); err != nil { + t.Fatal(err) + } if _, err := assign(ctx, open, "anchor", "mesh-controller"); err != nil { t.Fatal(err) } diff --git a/cmd/mesh-controller/credential_on_assign_test.go b/cmd/mesh-controller/credential_on_assign_test.go new file mode 100644 index 0000000..959ef07 --- /dev/null +++ b/cmd/mesh-controller/credential_on_assign_test.go @@ -0,0 +1,90 @@ +package main + +import ( + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" +) + +// A fresh assignment is pushed before its credential exists (novox/hq issue 203): `assign` recorded +// the module, `push` sealed a random own secret where the bus credential belongs, and the process +// crash-looped until a person ran `module issue` and pushed again. Now assigning a module that speaks +// on the bus issues its credential in the same act — or, when the bus cannot be reached from here, +// says which verb to run — and a push never seals a placeholder in a credential's place. + +func aTalker() catalogue.Manifest { + return catalogue.Manifest{Module: "talker", Version: "1", + OwnSecrets: catalogue.OwnSecrets{"broker": {Path: "/var/lib/mesh/talker/broker"}}, + Resources: []map[string]any{ + {"id": "state", "type": "directory", "path": "/var/lib/mesh/talker", "mode": "0700"}, + }} +} + +func TestAssigningAModuleThatSpeaksOnTheBusNamesItsCredential(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + register(t, open, aTalker()) + + // No bus is known to this process, so the credential cannot be issued here: the assignment + // stands and says exactly what must happen before a push — never silently. + said, err := assign(ctx, open, "laptop", "talker") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(said, "module issue talker --node laptop") { + t.Fatalf("an assignment whose credential could not be issued does not name the verb:\n%s", said) + } + + // And the push refuses to send it, naming the same verb, rather than sealing a placeholder. + plan, settings, err := planFor(ctx, open, "laptop") + if err != nil { + t.Fatal(err) + } + _, err = declarationFor(ctx, open, "laptop", plan, settings) + if err == nil { + t.Fatal("a push sealed a placeholder where talker's bus credential belongs") + } + if !strings.Contains(err.Error(), "module issue talker --node laptop") || !strings.Contains(err.Error(), "issue 203") { + t.Fatalf("the refusal does not say what to run: %v", err) + } + + // Once the user is minted, the push goes on to the credential the mesh sealed, and re-assigning + // does not mint again: a credential rotates on purpose, never by habit. + if _, err := open.inventory.MintBusPassword(ctx, inventory.BusUser{ + Username: "laptop.talker", Kind: inventory.BusModule, Node: "laptop", Module: "talker"}); err != nil { + t.Fatal(err) + } + hash, _, err := open.inventory.BusUserHash(ctx, "laptop.talker") + if err != nil { + t.Fatal(err) + } + said, err = assign(ctx, open, "laptop", "talker") + if err != nil { + t.Fatal(err) + } + if strings.Contains(said, "module issue") { + t.Fatalf("a module with a minted credential was told to issue one:\n%s", said) + } + again, _, err := open.inventory.BusUserHash(ctx, "laptop.talker") + if err != nil { + t.Fatal(err) + } + if again != hash { + t.Fatal("re-assigning rotated the credential") + } +} + +// A module that declares no broker secret is left alone: nothing to issue, nothing said. +func TestAssigningAModuleThatDoesNotSpeakSaysNothingOfCredentials(t *testing.T) { + open := aMesh(t) + register(t, open, helloWeb()) + said, err := assign(t.Context(), open, "laptop", "hello-web") + if err != nil { + t.Fatal(err) + } + if strings.Contains(said, "credential") { + t.Fatalf("a module without a broker secret was told about credentials:\n%s", said) + } +} diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index 4300808..ef8712c 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -533,6 +533,23 @@ func renderingFor(ctx context.Context, open *stores, node string, var sealed string var err error if choosing == Allocating { + // **The broker credential is never invented here** (novox/hq issue 203). Every other + // own secret is the mesh's to make — a password nobody else knows — but this one + // is an account on the bus, minted by `module issue` and sealed by it; a push that + // made a random one would deliver a file the process cannot read and report the + // machine applied. Refused by name, with the verb. + if name == "broker" { + user := broker.Principal{Kind: broker.KindModule, Node: node, Module: m.Module}.Username() + if _, minted, err := inv.BusUserHash(ctx, user); err != nil { + return catalogue.Rendering{}, inventory.Node{}, err + } else if !minted { + return catalogue.Rendering{}, inventory.Node{}, fmt.Errorf( + "%s on %s has no bus credential: nothing was issued for %s, and a push "+ + "would seal a placeholder its process cannot read (novox/hq issue 203). "+ + "`module issue %s --node %s`, then push again", + m.Module, node, user, m.Module, node) + } + } sealed, err = inv.SecretForModule(ctx, node, m.Module, name) } else { var held bool diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 03846bd..bc66f65 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -872,6 +872,15 @@ func (r Resolution) compose(with Rendering, owner map[string]string, if renamed := reflectsRenamed(m.Module, resource["reload-on"]); renamed != nil { copied["reload-on"] = renamed } + // **What reads one of this module's own secrets is restarted when it changes** (novox/hq + // issue 203, issue 206). A credential is re-issued by the mesh, and a container that + // mounted the old file keeps the old one open: the build machine ran for an hour on a + // credential the mesh had replaced, because its manifest restarted it on its + // environment file and nobody had thought to name the credential too. Composed here so + // no manifest has to say it, for a container or a daemon that names the secret's path. + if reads := secretsReadBy(copied, m); len(reads) > 0 { + copied["restart-on"] = withRestartOn(copied["restart-on"], reads) + } // **A version prepares its state before it runs** (novox/hq ADR 0135). Derived from the // module's own resource rather than declared beside it: what prepares the state is the // module's own code, so what it is given has to be what that code is given — and a @@ -2079,3 +2088,87 @@ func portOfEndpoint(values map[string]any, ports map[string]int) { values["port"] = port } } + +// secretsReadBy is the file resources of this module's own secrets that a container or a daemon reads +// — named in its volumes, its environment or its env-files by the secret's placed path — as +// restart-on ids. Nothing for other shapes, and nothing for a scheduled or run-once process, which +// the host refuses a restart-on for (it runs again anyway, and reads the file afresh). +func secretsReadBy(resource map[string]any, m Manifest) []string { + kind := fmt.Sprint(resource["type"]) + if kind != "container" && kind != "process" { + return nil + } + if resource["schedule"] != nil || resource["run-once"] == true { + return nil + } + var mentioned []string + for _, key := range []string{"volumes", "env", "env-file"} { + mentioned = append(mentioned, stringsIn(resource[key])...) + } + var out []string + for _, name := range sortedKeys(m.OwnSecrets) { + path := m.OwnSecrets[name].Path + if path == "" { + continue + } + for _, s := range mentioned { + // A volume is `source:destination[:mode]`; an env value or an env-file is the path itself. + if s == path || strings.HasPrefix(s, path+":") { + out = append(out, m.Module+"."+NeedID(name)) + break + } + } + } + return out +} + +// stringsIn is every string in a list or a map's values; nothing for anything else. +func stringsIn(v any) []string { + switch x := v.(type) { + case []any: + var out []string + for _, item := range x { + if s, ok := item.(string); ok { + out = append(out, s) + } + } + return out + case []string: + return x + case map[string]any: + var out []string + for _, k := range sortedKeys(x) { + if s, ok := x[k].(string); ok { + out = append(out, s) + } + } + return out + case map[string]string: + var out []string + for _, k := range sortedKeys(x) { + out = append(out, x[k]) + } + return out + } + return nil +} + +// withRestartOn is a resource's restart-on list with these ids added once each. +func withRestartOn(have any, add []string) []any { + var out []any + seen := map[string]bool{} + for _, id := range reflectsRenamed("", have) { + s := fmt.Sprint(id) + if !seen[s] { + seen[s] = true + out = append(out, s) + } + } + for _, id := range add { + if !seen[id] { + seen[id] = true + out = append(out, id) + } + } + return out +} diff --git a/internal/catalogue/secret_restart_test.go b/internal/catalogue/secret_restart_test.go new file mode 100644 index 0000000..5460c09 --- /dev/null +++ b/internal/catalogue/secret_restart_test.go @@ -0,0 +1,52 @@ +package catalogue + +import ( + "reflect" + "strings" + "testing" +) + +// What reads one of a module's own secrets is restarted when the secret changes (novox/hq issue 203, +// issue 206): the build machine kept an hour-old credential open because its manifest restarted it +// on its environment file alone. Composed, so a manifest need not say it; a scheduled process is +// left alone, because the host refuses a restart-on for one and it reads the file afresh each run. +func TestAContainerReadingAnOwnSecretIsRestartedWhenItChanges(t *testing.T) { + m := Manifest{Module: "agent", Version: "1", + OwnSecrets: OwnSecrets{"broker": {Path: "/var/lib/mesh/agent/broker"}}, + Resources: []map[string]any{ + {"id": "mesh-state", "type": "directory", "path": "/var/lib/mesh/agent", "mode": "0700"}, + {"id": "settings", "type": "file", "path": "/var/lib/mesh/agent/agent.env", "mode": "0600", "content": "A=1\n"}, + {"id": "server", "type": "container", "name": "agent", "network": "host", + "image": "registry.example/agent@sha256:" + strings.Repeat("a", 64), + "volumes": []any{"/var/lib/mesh/agent:/run/mesh:ro", "/var/lib/mesh/agent/broker:/run/mesh/broker:ro"}, + "env-file": []any{"/var/lib/mesh/agent/agent.env"}, + "restart-on": []any{"settings"}}, + {"id": "nightly", "type": "container", "name": "agent-nightly", "schedule": "0 3 * * *", + "image": "registry.example/agent@sha256:" + strings.Repeat("a", 64), + "volumes": []any{"/var/lib/mesh/agent/broker:/run/mesh/broker:ro"}}, + {"id": "other", "type": "container", "name": "agent-other", + "image": "registry.example/agent@sha256:" + strings.Repeat("a", 64)}, + }} + got, err := Resolve(shelf(m), []string{m.Module}, + Node{Name: "anchor", At: "10.0.0.1", Capabilities: map[string]bool{"container-runtime": true}}, World{}) + if err != nil { + t.Fatal(err) + } + out, err := got.Declaration(Rendering{Needed: map[string]map[string]string{"agent": {"broker": "SEALED"}}}) + if err != nil { + t.Fatal(err) + } + by := map[string]map[string]any{} + for _, r := range out { + by[r["id"].(string)] = r + } + if want := []any{"agent.settings", "agent.needs-broker"}; !reflect.DeepEqual(by["agent.server"]["restart-on"], want) { + t.Fatalf("the server reads the credential and is not restarted on it: %v", by["agent.server"]["restart-on"]) + } + if _, has := by["agent.nightly"]["restart-on"]; has { + t.Fatalf("a scheduled container was given a restart-on, which the host refuses: %v", by["agent.nightly"]["restart-on"]) + } + if _, has := by["agent.other"]["restart-on"]; has { + t.Fatalf("a container that reads no secret was given one to restart on: %v", by["agent.other"]["restart-on"]) + } +}