Place the bus's users again on a re-run until the bus is adopted (hq issue 146, review 102)

A re-run's apply writes the bundle's one-entry user list over the list placed at
enrolment, and an enrolled machine skipped placing it: enrolled was taken for adopted,
against ADR 0331 §5. Until the controller knows the nats module, the installer places
the composed list again. Also drop the previous broker's AMQP and management ports,
which nothing raises any more, and say why the bus always carries a ports setting.

cmd/mesh-bootstrap/main.go

@@ -130,7 +130,7 @@ const usage = `mesh-bootstrap — make a bare machine into a mesh
 
   The foundation's ports are this machine's, each checked free before anything is
   raised and kept as the node's setting for the module that binds it:
-  --store-port 5432  --bus-port 5671  --amqp-port 5672  --management-port 15672
+  --store-port 5432  --bus-port 5671
   --registry-port 5000 (follows --registry, and must agree with it)
   --packages-port 3000  --hub-port 51820/udp
   --overlay-range    the private network's range (default 10.42.0.0/16); refused
@@ -325,9 +325,7 @@ func newFlagSet(opts *bootstrap.Options, jsonOut *bool) *flag.FlagSet {
 		into       *int
 	}{
 		{"store-port", "the store", &opts.Ports.Store},
-		{"bus-port", "the bus (amqps)", &opts.Ports.Bus},
-		{"amqp-port", "the broker's AMQP", &opts.Ports.AMQP},
-		{"management-port", "the broker's management, on loopback", &opts.Ports.Management},
+		{"bus-port", "the bus", &opts.Ports.Bus},
 		{"registry-port", "the registry", &opts.Ports.Registry},
 		{"packages-port", "the package registry", &opts.Ports.Packages},
 		{"hub-port", "the private network's hub (udp)", &opts.Ports.Hub},

internal/bootstrap/adopted.go

@@ -106,11 +106,11 @@ type AdoptedRewrite struct {
 }
 
 // RewriteAdopted makes the produced bundle one for an adopted machine: the foundation's own filter
-// taken out, and the mesh's guard put in its place, guarding on this node the store's port, the
-// broker's management port and the broker's plaintext port. The last is published on every
-// interface and the foundation's filter admits it from the private network only, so a found
-// firewall that filters only incoming traffic would leave it reachable from anywhere (novox/hq ADR
-// 0103). Every one is a port of a module genesis takes. The nftables package stays: the guard is loaded with it, and
+// taken out, and the mesh's guard put in its place, guarding on this node the store's port. It is
+// published on every interface and the foundation's filter admits it from the private network only,
+// so a found firewall that filters only incoming traffic would leave it reachable from anywhere
+// (novox/hq ADR 0103). The bus is reached from anywhere on purpose — a node enrols over it — and its
+// monitoring port is published on loopback only, so neither is guarded. The nftables package stays: the guard is loaded with it, and
 // installing a package loads no table. Openings are not the bundle's — the first push declares
 // them, once there is a controller to derive them.
 func RewriteAdopted(r *Rewritten, p FoundationPorts) (AdoptedRewrite, error) {
@@ -128,11 +128,11 @@ func RewriteAdopted(r *Rewritten, p FoundationPorts) (AdoptedRewrite, error) {
 		out.Removed = append(out.Removed, id)
 	}
 
-	out.Guarded = []int{p.Store, p.Management, p.AMQP}
+	out.Guarded = []int{p.Store}
 	var text bytes.Buffer
 	text.WriteString(",\n    // The mesh's guard (novox/hq ADR 0100): this machine is adopted, so its own firewall\n" +
 		"    // stays in force and the foundation's filter is not loaded. The guard only refuses: the\n" +
-		"    // store's port and the broker's management and plaintext ports, except from the machine and\n" +
+		"    // store's port, except from the machine and\n" +
 		"    // the private network.")
 	for _, res := range guardResources(out.Guarded) {
 		var one bytes.Buffer

internal/bootstrap/adopted_test.go

@@ -70,7 +70,7 @@ func TestTheGuardRefusesOnlyWhatIsAddressedToThisMachine(t *testing.T) {
 
 func TestAnAdoptedBundleLoadsNoDroppingTableAndExactlyTheGuard(t *testing.T) {
 	r := producedBundle(t)
-	p := FoundationPorts{Store: 5433, Management: 15673, AMQP: 5773}
+	p := FoundationPorts{Store: 5433}
 	if _, err := RewritePorts(&r, p, ""); err != nil {
 		t.Fatal(err)
 	}
@@ -101,9 +101,9 @@ func TestAnAdoptedBundleLoadsNoDroppingTableAndExactlyTheGuard(t *testing.T) {
 	if len(guards) != 1 {
 		t.Fatalf("%d guard table(s)", len(guards))
 	}
-	// The store's, the broker's plaintext and its management port: each one the filter admits
-	// from the private network only (novox/hq ADR 0103).
-	if !strings.Contains(guards[0].Content, "tcp dport { 5433, 5773, 15673 } drop") {
+	// The store's: the one foundation port the filter admits from the private network only
+	// (novox/hq ADR 0103). The bus is reached from anywhere, its monitoring only on loopback.
+	if !strings.Contains(guards[0].Content, "tcp dport 5433 drop") && !strings.Contains(guards[0].Content, "tcp dport { 5433 } drop") {
 		t.Errorf("the guard does not refuse this node's ports: %s", guards[0].Content)
 	}
 	if strings.Count(guards[0].Content, "accept") != 1 || !strings.Contains(guards[0].Content, "policy accept") {

internal/bootstrap/enrol.go

@@ -94,6 +94,13 @@ func Enrol(ctx context.Context, o Options, sys system.System, control controlPla
 	switch mine, err := identity.Load(where); {
 	case err == nil && mine.Node == o.Node:
 		say("  already enrolled  " + o.Node + " — " + where)
+		// **Enrolled is not adopted** (novox/hq ADR 0331 §5). This run's apply wrote the bundle's
+		// user list again — the controller's entry and nobody else's — over the list placed when
+		// the machine enrolled. Until the bus is the nats module, that list is still the
+		// installer's to place, so it is placed again, before anything dials the bus as a node.
+		if err := placeUntilAdopted(ctx, control, say); err != nil {
+			return out, err
+		}
 	case err == nil:
 		return out, fmt.Errorf(
 			"this machine is already node %q and was asked to become %q.\n"+
@@ -192,6 +199,27 @@ func placeTheBusUsers(ctx context.Context, control controlPlane, say func(string
 	return nil
 }
 
+// placeUntilAdopted places the bus's users unless the nats module has them now (novox/hq ADR 0331
+// §5): from the bus's adoption on, the module receives the list in its declaration, and the
+// installer's bridge is not run again.
+//
+// **Placed again rather than left alone.** The other way out — writing the bundle's list only
+// where no list exists yet — needs a file the applier creates and then never reconciles, which no
+// resource of a declaration is; and it would keep whatever list was there, while the controller's
+// composition is the one that is right. So a re-run converges the file on what the controller says.
+// The module is asked of the controller by name (`module list`), which is where adoption registers it.
+func placeUntilAdopted(ctx context.Context, control controlPlane, say func(string)) error {
+	known, err := control.tell(ctx, "module", "list")
+	if err != nil {
+		return err
+	}
+	if mentions(known, BusModule) {
+		say("  bus users         the " + BusModule + " module's — not placed by the installer")
+		return nil
+	}
+	return placeTheBusUsers(ctx, control, say)
+}
+
 // runTheHost makes sure something on this machine is listening to the mesh, and proves it.
 //
 // **The installer does not install the service, and says so.** A unit file is a packaging decision

internal/bootstrap/enrol_test.go

@@ -59,6 +59,9 @@ func TestAMachineThatHasAlreadyEnrolledIsNotEnrolledAgain(t *testing.T) {
 		switch {
 		case strings.Contains(joined, "node list"):
 			return "anchor               here           01J0\n", nil
+		// The bus is the nats module already, so its users are the module's.
+		case strings.Contains(joined, "module list"):
+			return "nats                 1\npostgres             1\n", nil
 		// An agent is running here too. Both halves are needed: enrolling makes the mesh hear from
 		// a machine once, so the first answer alone also describes a machine with no agent at all.
 		case name == "pgrep":
@@ -82,8 +85,8 @@ func TestAMachineThatHasAlreadyEnrolledIsNotEnrolledAgain(t *testing.T) {
 		t.Errorf("a token was issued for a machine that already holds an identity: %v",
 			runtime.commands)
 	}
-	// Nor are the bus's users placed again: from the adoption on they are the nats module's, and
-	// the installer's bridge is not run a second time (novox/hq ADR 0331 §5).
+	// Nor are the bus's users placed: the bus is adopted, so they are the nats module's, and the
+	// installer's bridge is not run again (novox/hq ADR 0331 §5).
 	if runtime.ran("broker accounts") || runtime.ran("accounts.conf") {
 		t.Errorf("the bus's users were placed on a re-run: %v", runtime.commands)
 	}
@@ -100,6 +103,8 @@ func TestAMeshThatHasHeardFromAMachineWithNoAgentStartsOne(t *testing.T) {
 	runtime := &asked{answer: func(name string, args []string) (string, error) {
 		joined := strings.Join(args, " ")
 		switch {
+		case strings.Contains(joined, "module list"):
+			return "nats                 1\n", nil // the bus is adopted: no users to place
 		case strings.Contains(joined, "node list"):
 			// Exactly what enrolling leaves behind, with nothing running.
 			return "anchor               here           01J0\n", nil
@@ -164,6 +169,8 @@ func TestAHostThatIsRunningAndUnheardOfIsNotAnInstall(t *testing.T) {
 	runtime := &asked{answer: func(_ string, args []string) (string, error) {
 		joined := strings.Join(args, " ")
 		switch {
+		case strings.Contains(joined, "module list"):
+			return "nats                 1\n", nil // the bus is adopted: no users to place
 		case strings.Contains(joined, "node list"):
 			// Enrolled, and never spoken.
 			return "anchor               never spoken    01J0\n", nil
@@ -195,6 +202,8 @@ func TestAMachineWithNoHostServiceIsRefusedRatherThanGivenOne(t *testing.T) {
 	runtime := &asked{answer: func(_ string, args []string) (string, error) {
 		joined := strings.Join(args, " ")
 		switch {
+		case strings.Contains(joined, "module list"):
+			return "nats                 1\n", nil // the bus is adopted: no users to place
 		case strings.Contains(joined, "node list"):
 			return "anchor               never spoken    01J0\n", nil
 		case args[0] == "show":
@@ -319,3 +328,49 @@ func TestAFirstNodeIsLetOntoTheBusItRaised(t *testing.T) {
 			"(placed at %v, enrolled at %d):\n%s", placed, enrol, strings.Join(runtime.commands, "\n"))
 	}
 }
+
+// A re-run on a machine that enrolled and whose bus is not yet the nats module places the
+// controller's list again (novox/hq ADR 0331 §5, review of mesh-host #96). The apply just wrote the
+// bundle's list — the controller alone — over the one placed at enrolment; left so, the next time the
+// bus re-reads it every node and the enrolment account are refused.
+func TestARerunOnAnEnrolledMachineWhoseBusIsNotAdoptedPlacesTheUsersAgain(t *testing.T) {
+	runtime := &asked{answer: func(name string, args []string) (string, error) {
+		joined := strings.Join(args, " ")
+		switch {
+		case strings.Contains(joined, "node list"):
+			return "anchor               here           01J0\n", nil
+		case strings.Contains(joined, "module list"):
+			return "mesh-controller      1\npostgres             1\n", nil
+		case strings.Contains(joined, "broker accounts"):
+			return "accounts {\n  MESH { users = [ { user: \"node.anchor\" } ] }\n}\n", nil
+		case name == "pgrep":
+			return "4242\n", nil
+		case name == "sh":
+			return "", nil
+		}
+		return "", fmt.Errorf("unexpected: %s %v", name, args)
+	}}
+
+	out, err := Enrol(context.Background(), Options{
+		Node: "anchor", State: alreadyEnrolled(t, "anchor"), Timeout: time.Second,
+		Host: "/usr/local/bin/mesh-host", HostInBackground: true,
+	}, arch(t), controlPlane{container: "temp-mesh-controller", run: runtime.run, timeout: time.Second},
+		func(string) {})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if out.Joined || runtime.ran("token issue") {
+		t.Errorf("a re-run enrolled again: %v", runtime.commands)
+	}
+	placed := false
+	for _, c := range runtime.commands {
+		if strings.HasPrefix(c, "sh -c") && strings.Contains(c, "node.anchor") &&
+			strings.Contains(c, "/var/lib/mesh-bus-conf/accounts.conf") && strings.Contains(c, "kill -s HUP mesh-broker") {
+			placed = true
+		}
+	}
+	if !placed {
+		t.Fatalf("the controller's list was not placed again on a re-run before adoption:\n%s",
+			strings.Join(runtime.commands, "\n"))
+	}
+}

internal/bootstrap/phase_packages.go

@@ -144,8 +144,8 @@ func RaisePackageRegistry(ctx context.Context, o Options, d Deps, control contro
 // recordTheForgesPort makes the port given for the package registry a setting of the module that
 // serves it, on this node (novox/hq 04-ISSUES/085, ADR 0100).
 //
-// **The forge is the one foundation port that was not a setting.** The store's, the bus's, the
-// broker's management port and the registry's each become a `ports` setting of the module that
+// **The forge is the one foundation port that was not a setting.** The store's, the bus's and the
+// registry's each become a `ports` setting of the module that
 // binds them, set where that module is registered and assigned; the forge is raised by hand here,
 // long before its module can be built, so there was no registration to hang it on and the number
 // lived in rewritten manifest text instead. So the manifest is registered here — not assigned, and

internal/bootstrap/ports.go

@@ -20,24 +20,21 @@ import (
 // FoundationPorts are the machine's ports the foundation binds (novox/hq ADR 0100).
 //
 // **The node's, not the catalogue's.** A machine in use may already hold one — a predecessor's
-// registry on 5000, its broker's management port — and a port fixed in the bundle and the manifests
+// registry on 5000, its bus on 5671 — and a port fixed in the bundle and the manifests
 // surfaces as a container that fails to bind, and one changed at genesis would be changed back when
 // the foundation is adopted as modules. So each is an input here, checked free, rewritten into the
 // bundle, and handed to the controller as that node's setting for the module that binds it.
 type FoundationPorts struct {
-	Store      int `json:"store"`
-	Bus        int `json:"bus"`
-	AMQP       int `json:"amqp"`
-	Management int `json:"management"`
-	Registry   int `json:"registry"`
-	Packages   int `json:"packages"`
-	Hub        int `json:"hub"`
+	Store    int `json:"store"`
+	Bus      int `json:"bus"`
+	Registry int `json:"registry"`
+	Packages int `json:"packages"`
+	Hub      int `json:"hub"`
 }
 
 // DefaultPorts are the catalogue's numbers.
 func DefaultPorts() FoundationPorts {
-	return FoundationPorts{Store: 5432, Bus: 5671, AMQP: 5672, Management: 15672, Registry: 5000,
-		Packages: 3000, Hub: 51820}
+	return FoundationPorts{Store: 5432, Bus: 5671, Registry: 5000, Packages: 3000, Hub: 51820}
 }
 
 // DefaultOverlayRange is the controller's default private-network range.
@@ -47,7 +44,7 @@ const DefaultOverlayRange = "10.42.0.0/16"
 func (p FoundationPorts) orDefaults() FoundationPorts {
 	d := DefaultPorts()
 	for _, f := range []struct{ got, def *int }{
-		{&p.Store, &d.Store}, {&p.Bus, &d.Bus}, {&p.AMQP, &d.AMQP}, {&p.Management, &d.Management},
+		{&p.Store, &d.Store}, {&p.Bus, &d.Bus},
 		{&p.Registry, &d.Registry}, {&p.Packages, &d.Packages}, {&p.Hub, &d.Hub},
 	} {
 		if *f.got == 0 {
@@ -60,8 +57,7 @@ func (p FoundationPorts) orDefaults() FoundationPorts {
 // named is each port with what it is and its protocol, in a fixed order.
 func (p FoundationPorts) named() []namedPort {
 	return []namedPort{
-		{"the store", "tcp", p.Store}, {"the bus", "tcp", p.Bus}, {"the broker's AMQP", "tcp", p.AMQP},
-		{"the broker's management", "tcp", p.Management}, {"the registry", "tcp", p.Registry},
+		{"the store", "tcp", p.Store}, {"the bus", "tcp", p.Bus}, {"the registry", "tcp", p.Registry},
 		{"the package registry", "tcp", p.Packages}, {"the private network's hub", "udp", p.Hub},
 	}
 }
@@ -88,8 +84,11 @@ func (p FoundationPorts) Check() error {
 }
 
 // moduleSettings is what each foundation module is told about its ports on this node: the port it
-// declares, to the machine's port it is given. Only what differs from the catalogue — a converged
-// genesis on the defaults sets nothing, and so changes nothing it did before.
+// declares, to the machine's port it is given. Only what differs from the catalogue's manifest is
+// set. For the store, the registry and the forge the manifest declares the foundation's default, so
+// a genesis on the defaults sets nothing for them. The bus is the exception: the nats module
+// declares 4222 and the foundation publishes the bus on 5671, the port every enrolment token
+// names, so the bus always carries a setting.
 func (p FoundationPorts) moduleSettings() map[string]map[string]int {
 	d := DefaultPorts()
 	out := map[string]map[string]int{}
@@ -320,8 +319,8 @@ func PortsFree(ctx context.Context, run Runner, p FoundationPorts, ours func(rea
 	}
 	if len(problems) > 0 {
 		return fmt.Errorf("the foundation's ports must be free before anything is raised:\n  - %s\n"+
-			"Give it another with the matching flag (--store-port, --bus-port, --amqp-port, "+
-			"--management-port, --registry-port, --packages-port, --hub-port); nothing was changed",
+			"Give it another with the matching flag (--store-port, --bus-port, "+
+			"--registry-port, --packages-port, --hub-port); nothing was changed",
 			strings.Join(problems, "\n  - "))
 	}
 	return nil
@@ -508,8 +507,8 @@ func CheckTheMachine(ctx context.Context, o Options, run Runner, bundle *declara
 	if err := PortsFree(ctx, run, p, ours); err != nil {
 		return err
 	}
-	say(fmt.Sprintf("  ports free        store %d, bus %d, amqp %d, management %d, registry %d, packages %d, hub %d/udp",
-		p.Store, p.Bus, p.AMQP, p.Management, p.Registry, p.Packages, p.Hub))
+	say(fmt.Sprintf("  ports free        store %d, bus %d, registry %d, packages %d, hub %d/udp",
+		p.Store, p.Bus, p.Registry, p.Packages, p.Hub))
 	if o.Tunnel != "" {
 		// One tunnel and one range: the mesh's range IS the found tunnel's, so the rule that the
 		// two must not overlap applies only where a found tunnel is left running beside the mesh's

internal/bootstrap/ports_test.go

@@ -60,7 +60,7 @@ func TestTheDefaultPortsLeaveTheBundleAsItWas(t *testing.T) {
 
 func TestGivenPortsMoveOnlyTheMachinesSide(t *testing.T) {
 	r := producedBundle(t)
-	p := FoundationPorts{Store: 5433, Bus: 5771, AMQP: 5772, Management: 15673, Registry: 5100}
+	p := FoundationPorts{Store: 5433, Bus: 5771, Registry: 5100}
 	got, err := RewritePorts(&r, p, "10.77.0.0/16")
 	if err != nil {
 		t.Fatal(err)
@@ -168,20 +168,20 @@ func noneOurs(reachable.Reach) bool { return false }
 func TestABusyPortIsRefusedNamingItsHolder(t *testing.T) {
 	m := machineRunner{
 		ss: "tcp LISTEN 0 4096 0.0.0.0:5000 0.0.0.0:* users:((\"docker-proxy\",pid=1,fd=7))\n" +
-			"tcp LISTEN 0 4096 127.0.0.1:15672 0.0.0.0:* users:((\"beam.smp\",pid=2,fd=7))\n",
+			"tcp LISTEN 0 4096 0.0.0.0:5671 0.0.0.0:* users:((\"beam.smp\",pid=2,fd=7))\n",
 		ps: "predecessor-registry\t0.0.0.0:5000->5000/tcp\n",
 	}
 	err := PortsFree(context.Background(), m.run, DefaultPorts(), noneOurs)
 	if err == nil {
 		t.Fatal("held ports were not refused")
 	}
-	for _, want := range []string{"predecessor-registry", "beam.smp", "tcp/5000", "tcp/15672", "--registry-port"} {
+	for _, want := range []string{"predecessor-registry", "beam.smp", "tcp/5000", "tcp/5671", "--registry-port"} {
 		if !strings.Contains(err.Error(), want) {
 			t.Errorf("the refusal does not say %q: %v", want, err)
 		}
 	}
 	p := DefaultPorts()
-	p.Registry, p.Management = 5100, 15673
+	p.Registry, p.Bus = 5100, 5771
 	if err := PortsFree(context.Background(), m.run, p, noneOurs); err != nil {
 		t.Errorf("other ports given and still refused: %v", err)
 	}