Merge pull request 'Compose a bus user only for a module that can read an account (hq issue 195)' (#270) from fix/195-only-modules-that-read-an-account-are-bus-users into main

cmd/mesh-controller/push.go

@@ -945,13 +945,9 @@ func raiseTheBus(ctx context.Context, inv *inventory.Inventory, address string)
 		return err
 	}
 	hearing := 0
-	for _, p := range users {
-		consumer, needed := broker.ConsumerFor(p)
-		if !needed {
-			continue
-		}
-		if err := js.EnsureConsumer(consumer); err != nil {
-			return fmt.Errorf("how %s on %s hears what it consumes: %w", p.Module, p.Node, err)
+	for _, c := range broker.ConsumersOf(users) {
+		if err := js.EnsureConsumer(c.Consumer); err != nil {
+			return fmt.Errorf("how %s on %s hears what it consumes: %w", c.Module, c.Node, err)
 		}
 		hearing++
 	}

internal/broker/derived.go

@@ -144,6 +144,44 @@ func ConsumerFor(p Principal) (Consumer, bool) {
 	}, true
 }
 
+// ModuleConsumer is one module's durable consumer, with the module and node it is for.
+type ModuleConsumer struct {
+	Node, Module string
+	Consumer     Consumer
+}
+
+// ConsumersOf is every module's durable consumer the composed users imply: each module user's, and
+// each module a runtime carries — a carried module with no account of its own is no user (novox/hq
+// issue 195), and its consumer is still the controller's to make, since the runtime reads it on the
+// module's behalf (ADR 0198). One per module and node, whichever of the two named it first.
+func ConsumersOf(users []Principal) []ModuleConsumer {
+	var out []ModuleConsumer
+	seen := map[string]bool{}
+	add := func(p Principal) {
+		c, needed := ConsumerFor(p)
+		if !needed || seen[p.Node+"/"+p.Module] {
+			return
+		}
+		seen[p.Node+"/"+p.Module] = true
+		out = append(out, ModuleConsumer{Node: p.Node, Module: p.Module, Consumer: c})
+	}
+	for _, p := range users {
+		if p.Kind == KindModule {
+			add(p)
+		}
+	}
+	for _, p := range users {
+		if p.Kind != KindNodeTools {
+			continue
+		}
+		for _, d := range p.Carries {
+			add(Principal{Kind: KindModule, Node: p.Node, Module: d.Module, Emits: d.Emits,
+				Consumes: d.Consumes, Serves: d.Serves, Holds: d.Holds, Uses: d.Uses, Watches: d.Watches})
+		}
+	}
+	return out
+}
+
 // HolderConsumerFor is the worker a seat's holders share on that seat's work queue.
 //
 // **One worker for every holder, and each holder pulls one ask when it is idle** (novox/hq ADR

internal/broker/users.go

@@ -37,6 +37,10 @@ type Declared struct {
 	State []Bucket
 	// Reads are other modules' state it reads, each `<module>.<name>` (novox/hq ADR 0201).
 	Reads []string
+	// NoAccount says the module declares no own secret named broker, so no account could ever be
+	// delivered to it and nothing can connect as it (novox/hq issue 195). Said in the negative so a
+	// record that does not say is composed as it always was.
+	NoAccount bool
 }
 
 // Records is what composing a user list needs to know about the mesh, and nothing more.
@@ -81,6 +85,15 @@ func Users(r Records) ([]Principal, error) {
 			if runtimeHere && d.Module == RuntimeModule {
 				continue
 			}
+			// **A module with nowhere to read an account is no user** (novox/hq issue 195). `module
+			// issue` refuses it one (issue 078: an account nothing reads is an orphan), so its user
+			// could only ever be left out of the file for want of a password — and every such module
+			// was named, on every status and plan, as a credential the mesh had not minted. Where the
+			// runtime is, it speaks for the module; where it is not, the module cannot speak at all,
+			// and a user would not change that.
+			if d.NoAccount {
+				continue
+			}
 			out = append(out, Principal{
 				Kind: KindModule, Node: node, Module: d.Module,
 				Emits: d.Emits, Consumes: d.Consumes, Serves: d.Serves,

internal/broker/users_test.go

@@ -296,3 +296,57 @@ func TestTheRuntimeModuleBecomesTheMachinesRuntimePrincipal(t *testing.T) {
 		t.Error("telegram lost its own principal when the runtime arrived on its node")
 	}
 }
+
+// A module that declares nowhere to read an account is no user: it could never be issued one, so it
+// was only ever named as a credential the mesh had not minted (novox/hq issue 195). Where the runtime
+// is, the runtime still carries it — its grants are the runtime's.
+func TestAModuleThatCannotReadAnAccountIsNoUser(t *testing.T) {
+	r := someRecords()
+	r.Assigned["one"] = append(r.Assigned["one"],
+		Declared{Module: "packet-filter", NoAccount: true, Emits: []string{"rule.changed"}},
+		Declared{Module: RuntimeModule})
+	users, err := Users(r)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, u := range users {
+		if u.Username() == "one.packet-filter" {
+			t.Fatalf("packet-filter declares no broker secret and was composed as a user: %v", namesOf(t, r))
+		}
+		if u.Kind == KindNodeTools {
+			carried := false
+			for _, d := range u.Carries {
+				carried = carried || d.Module == "packet-filter"
+			}
+			if !carried {
+				t.Error("the runtime stopped carrying a module that has no account of its own")
+			}
+		}
+	}
+	if names := strings.Join(namesOf(t, r), ","); !strings.Contains(names, "one.telegram") {
+		t.Errorf("a module that does read an account lost its user: %s", names)
+	}
+}
+
+// A carried module with no account still has its consumer made: the runtime reads it on the module's
+// behalf (ADR 0198), and the consumer was derived from the module's own user until issue 195 took
+// that user away.
+func TestACarriedModuleWithNoAccountStillHasItsConsumer(t *testing.T) {
+	r := someRecords()
+	r.Assigned["one"] = append(r.Assigned["one"],
+		Declared{Module: "listener", NoAccount: true, Consumes: []string{"shop.order.placed"}},
+		Declared{Module: RuntimeModule})
+	r.Assigned["two"] = append(r.Assigned["two"],
+		Declared{Module: "auditor", Consumes: []string{"shop.order.placed"}})
+	users, err := Users(r)
+	if err != nil {
+		t.Fatal(err)
+	}
+	got := map[string]int{}
+	for _, c := range ConsumersOf(users) {
+		got[c.Node+"/"+c.Module]++
+	}
+	if got["one/listener"] != 1 || got["two/auditor"] != 1 || len(got) != 2 {
+		t.Fatalf("consumers: %v", got)
+	}
+}

internal/inventory/busrecords.go

@@ -136,6 +136,11 @@ func declaredFor(m catalogue.Manifest, seats map[string]catalogue.SeatDeclaratio
 		State: bucketsOf(m),
 		Reads: m.Reads,
 	}
+	// Whether it can be given an account at all: delivered as its own secret named broker, so one
+	// that declares none has nowhere to read it (novox/hq issue 195).
+	if _, reads := m.OwnSecrets["broker"]; !reads {
+		d.NoAccount = true
+	}
 	for _, c := range m.Claims {
 		// Every seat with a protocol, the mesh's own included. One that says only who does a job is
 		// not here and grants nothing, which is most of them.