Merge remote-tracking branch 'origin/main' into feat/900-node-files

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
+// says what it did in one line. Nothing for a module that declares no broker secret and runs no
-// whose user is already minted (a credential is rotated on purpose, never by re-assigning); and when
+// provisioner client; nothing for one whose user is already minted (a credential is rotated on
-// the bus cannot be reached from here, the line names the verb and the push that would refuse the
+// purpose, never by re-assigning); and when the bus cannot be reached from here, the line names the
-// module until it is run — never a silent placeholder (novox/hq issue 203).
+// 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,9 +956,18 @@ func issueOnTheNewBus(ctx context.Context, inv *inventory.Inventory, m catalogue
 	if 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)
+	}
+}

internal/catalogue/delivery_seat.go

@@ -9,7 +9,7 @@ package catalogue
 // DeliverySeat is the seat mesh-delivery holds.
 const DeliverySeat = "mesh-delivery"
 
-// deliveryVerbs are the delivery seat's tools: six that read, and the acts of a person and of healer H2.
+// deliveryVerbs are the delivery seat's tools: six that read, and the acts of a person, a reviewer and healer H2.
 func deliveryVerbs() []Verb {
 	return []Verb{
 		{Name: "deliveries", Description: "Every delivery not final, and those that ended in the last day, one line " +
@@ -84,6 +84,15 @@ func deliveryVerbs() []Verb {
 				"dry":        "\"true\": what it would retire, changing nothing"}, []string{"why"}, "dry"),
 			// Added after the seat's holder shipped (novox/hq issue 313): optional until mesh-delivery serves it.
 			Optional: true},
+		{Name: "mark-ready", Description: "A reviewer's word that a delivery's head is approved: the holder puts the " +
+			"label mesh/ready on its pull request and then writes \"ready at <head>\", the order the forge counts, " +
+			"as the mesh's own forge account, naming who reviewed it and why. Refused when the delivery's commit is " +
+			"no longer its pull request's head. mesh-delivery then merges it once its checks pass (novox/hq ADR 0364).",
+			Input: schema(map[string]string{"id": "the delivery's id (owner/repository@commit): the head reviewed",
+				"reviewed-by": "who reviewed it, as who@node", "why": "the review's verdict in a line"},
+				[]string{"id", "reviewed-by", "why"}),
+			// Added after the seat's holder shipped (novox/hq issue 913): optional until mesh-delivery serves it.
+			Optional: true},
 	}
 }
 

internal/catalogue/delivery_seat_test.go

@@ -133,3 +133,32 @@ func TestTheDeliverySeatPromisesTimesOptionally(t *testing.T) {
 		t.Fatalf("a holder serving times: %v", err)
 	}
 }
+
+// The seat marks a reviewed head ready (novox/hq issue 913): `mark-ready`, with the delivery's id, who reviewed it
+// and why, so a reviewer marks a pull request through one audited verb, label and comment in the order the forge
+// counts, instead of two raw forge calls. Added after the holder shipped, it is optional.
+func TestTheDeliverySeatPromisesMarkReadyOptionally(t *testing.T) {
+	seat, _ := SeatNamed(DeliverySeat)
+	var mark *Verb
+	for i := range seat.Serves {
+		if seat.Serves[i].Name == "mark-ready" {
+			mark = &seat.Serves[i]
+		}
+	}
+	if mark == nil || !mark.Optional {
+		t.Fatalf("the delivery seat promises %v, mark-ready optionally", VerbNames(seat.Serves))
+	}
+	if req, _ := mark.Input["required"].([]string); !reflect.DeepEqual(req, []string{"id", "reviewed-by", "why"}) {
+		t.Errorf("mark-ready requires %v, wanted id, reviewed-by and why", req)
+	}
+	var without []string
+	for _, v := range VerbNames(seat.Serves) {
+		if v != "mark-ready" {
+			without = append(without, v)
+		}
+	}
+	m := Manifest{Module: "mesh-delivery", Claims: []Claim{{Name: DeliverySeat, Scope: ScopeMesh, Serves: without}}}
+	if err := CanHold(m, seat); err != nil {
+		t.Fatalf("a holder without mark-ready yet: %v", err)
+	}
+}

testdata/store-tests

@@ -126,3 +126,4 @@
 ./cmd/mesh-controller TestTheRecheckVerbAsksMeshDeliveryOnABusOfItsOwn
 ./cmd/mesh-controller TestAGroupsCompositionIntoAReleaseBranchIsJudgedByItsOwnController
 ./cmd/mesh-controller TestAGroupsCompositionJudgedWithTheValidatorIntoAReleaseBranchIsJudgedByItsOwnController
+./cmd/mesh-controller TestAssigningTheClientIssuesItsAccount