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)
+}

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)
 	}

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)
+	}
+}

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

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
+}

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"])
+	}
+}