diff --git a/cmd/mesh-controller/acts.go b/cmd/mesh-controller/acts.go index 54703d8e..4f02544c 100644 --- a/cmd/mesh-controller/acts.go +++ b/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 "" } diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index 80c58cab..754bc044 100644 --- a/cmd/mesh-controller/modules.go +++ b/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\": }", 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 diff --git a/cmd/mesh-controller/provisioner_account_test.go b/cmd/mesh-controller/provisioner_account_test.go index bf537bff..20dc7b15 100644 --- a/cmd/mesh-controller/provisioner_account_test.go +++ b/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) + } +}