Merge pull request 'Issue the provisioner client its own account when its module declares no broker (issue 917)' (#359) from fix/917-provisioner-account into main

cmd/mesh-controller/acts.go

@@ -391,10 +391,10 @@ func blockedElsewhere(ctx context.Context, open *stores, except string) 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).
+// says what it did in one line. Nothing for a module that declares no broker secret and runs no
+// provisioner client; 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)
@@ -406,6 +406,10 @@ func issueOnAssign(ctx context.Context, open *stores, node, module string) strin
 		return ""
 	}
 	user := broker.Principal{Kind: broker.KindModule, Node: node, Module: module}.Username()
+	if _, reads := m.OwnSecrets["broker"]; !reads {
+		// A module running the provisioner client alone is minted the client's account (issue 917).
+		user = broker.Principal{Kind: broker.KindProvisioner, Node: node, Module: module}.Username()
+	}
 	if _, minted, err := inv.BusUserHash(ctx, user); err != nil || minted {
 		return ""
 	}

cmd/mesh-controller/modules.go

@@ -923,8 +923,10 @@ func brokerReachableAt(ctx context.Context, inv *inventory.Inventory, known brok
 // module's own secret named broker; a module that declares none has nothing to read it with, and
 // an account nothing reads is an orphan on the bus (novox/hq 04-ISSUES/078). Said before the
 // account is made, not after.
+// A module that runs its machine's provisioner client and declares no broker is issued the client's account
+// alone (issue 917): the client speaks on the bus only as itself.
 func mayIssue(m catalogue.Manifest) error {
-	if _, reads := m.OwnSecrets["broker"]; !reads {
+	if _, reads := m.OwnSecrets["broker"]; !reads && !runsTheProvisionerClient(m) {
 		return fmt.Errorf("%s declares no own secret named broker, so there is nowhere to deliver "+
 			"an account; a module that speaks on the bus declares \"own-secrets\": {\"broker\": <path>}", m.Module)
 	}
@@ -943,14 +945,6 @@ func mayIssue(m catalogue.Manifest) error {
 func issueOnTheNewBus(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest,
 	node, busAddress string) error {
 
-	user := broker.Principal{Kind: broker.KindModule, Node: node, Module: m.Module}.Username()
-	password, err := inv.MintBusPassword(ctx, inventory.BusUser{
-		Username: user, Kind: busKindOf(m.Module), Node: node, Module: m.Module,
-	})
-	if err != nil {
-		return err
-	}
-
 	// Where the module is told to find the bus, and what certificate it must present. The same pair
 	// a node is told, for the same reason: a mesh's bus presents its own certificate, in no public
 	// trust store, so an address alone fails at TLS.
@@ -962,8 +956,17 @@ func issueOnTheNewBus(ctx context.Context, inv *inventory.Inventory, m catalogue
 	if err != nil {
 		return err
 	}
-	if err := issueWith(ctx, inv, m, node, busAddress, known, reachable, user, password); err != nil {
-		return err
+	if _, reads := m.OwnSecrets["broker"]; reads {
+		user := broker.Principal{Kind: broker.KindModule, Node: node, Module: m.Module}.Username()
+		password, err := inv.MintBusPassword(ctx, inventory.BusUser{
+			Username: user, Kind: busKindOf(m.Module), Node: node, Module: m.Module,
+		})
+		if err != nil {
+			return err
+		}
+		if err := issueWith(ctx, inv, m, node, busAddress, known, reachable, user, password); err != nil {
+			return err
+		}
 	}
 	if !runsTheProvisionerClient(m) {
 		return nil

cmd/mesh-controller/provisioner_account_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"slices"
+	"strings"
 	"testing"
 
 	"github.com/novox/mesh-controller/internal/broker"
@@ -60,3 +61,32 @@ func TestTheProvisionerClientsAccountIsIssuedForItsServiceAndNeverInvented(t *te
 		t.Fatal("a module with nowhere to read the client's account is read as running it")
 	}
 }
+
+// A module that runs the client and declares no broker of its own, as the catalogue's mesh-provisioner-client does,
+// may be issued: the client's account alone (issue 917). Before, `module issue` refused it for having no broker,
+// while a push refused it for having no account, so no machine could run the client.
+func TestAClientWithoutABrokerOfItsOwnMayBeIssued(t *testing.T) {
+	if err := mayIssue(aClientModule()); err != nil {
+		t.Fatalf("the client alone is refused an account: %v", err)
+	}
+	neither := aBot("bot", false)
+	if err := mayIssue(neither); err == nil {
+		t.Fatal("a module with nowhere to read any account may be issued one")
+	}
+}
+
+// Assigning the client tries to issue the client's account, as assigning any module issues its broker (issue 917):
+// without a bus to issue on, the line says so and names the verb, never nothing.
+func TestAssigningTheClientIssuesItsAccount(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	client := aClientModule()
+	register(t, open, client)
+	out, err := assign(ctx, open, "laptop", client.Module)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(out, "bus credential") {
+		t.Fatalf("assigning the client said nothing of its account:\n%s", out)
+	}
+}