Derive the guard from taken modules only: their published private-network ports and their manifests' guards (hq ADR 0103)

cmd/mesh-controller/plan.go

@@ -539,11 +539,24 @@ func renderingFor(ctx context.Context, open *stores, node string,
 	if err != nil {
 		return catalogue.Rendering{}, inventory.Node{}, err
 	}
+	// And, on an adopted node, which modules were taken there: the guard is derived from those
+	// only (novox/hq ADR 0103).
+	var taken map[string]bool
+	if record.Adopted {
+		list, err := inv.Taken(ctx, node)
+		if err != nil {
+			return catalogue.Rendering{}, inventory.Node{}, err
+		}
+		taken = map[string]bool{}
+		for _, m := range list {
+			taken[m] = true
+		}
+	}
 	return catalogue.Rendering{
 		Settings: settings, Generators: gens, Grants: grants, Needed: needed, Ports: ports,
 		Certificate: certificate, Authority: authority, Mesh: private, Names: names,
 		Suffix: overlay.Suffix(), Foundation: foundation, Kept: kept, Adopted: record.Adopted,
-		Given: given,
+		Given: given, Taken: taken,
 	}, record, nil
 }
 

cmd/mesh-controller/sendable_test.go

@@ -225,3 +225,38 @@ func TestConsumersAreToldTheGivenPort(t *testing.T) {
 		t.Fatalf("a port given for the whole mesh was not refused: %v", err)
 	}
 }
+
+// novox/hq ADR 0103: the guard an adopted node is sent follows what was taken there. A store
+// assigned but not taken is not guarded — its port may still be the predecessor's — and taking it
+// guards it from the next declaration.
+func TestTheGuardIsSentForTakenModulesOnly(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	register(t, open, catalogue.Manifest{Module: "store", Version: "1",
+		Listens: []catalogue.Listening{{Port: 5432, From: catalogue.FromMesh}},
+		Guards:  []int{5432},
+		Resources: []map[string]any{{"id": "server", "type": "container", "name": "mesh-store",
+			"ports": []any{"5432:5432"},
+			"image": "registry.example/pg@sha256:" + strings.Repeat("b", 64)}}})
+	if err := open.inventory.SetAdopted(ctx, "anchor", true); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := assign(ctx, open, "anchor", "store"); err != nil {
+		t.Fatal(err)
+	}
+	if hasID(composed(t, open, "anchor").Resources, catalogue.GuardID()) {
+		t.Fatal("an untaken store is guarded")
+	}
+	if err := open.inventory.Take(ctx, "anchor", "store"); err != nil {
+		t.Fatal(err)
+	}
+	for _, r := range composed(t, open, "anchor").Resources {
+		if r["id"] == catalogue.GuardID() {
+			if r["content"] != catalogue.AsGuard([]int{5432}) {
+				t.Fatalf("the taken store's guard is:\n%s", r["content"])
+			}
+			return
+		}
+	}
+	t.Fatal("a taken store is not guarded")
+}

internal/catalogue/adoption_test.go

@@ -59,6 +59,8 @@ func anchorRendering(adopted bool) Rendering {
 		Mesh:       []string{"10.42.0.1"},
 		Foundation: []int{5671},
 		Adopted:    adopted,
+		// Genesis takes the foundation's modules.
+		Taken: map[string]bool{"postgres": true, "lavinmq": true},
 	}
 }
 
@@ -151,8 +153,9 @@ func TestAnAdoptedNodeLoadsNoFilterOfTheMeshs(t *testing.T) {
 	if guard == nil || got[GuardUnitID()] == nil || got[GuardRunningID()] == nil {
 		t.Fatalf("no guard: %v", keys(got))
 	}
-	if guard["content"] != AsGuard([]int{5432, 15672}) {
+	if guard["content"] != AsGuard([]int{5432, 5672, 15672}) {
-		t.Fatalf("the guard does not guard the store and the management port:\n%s", guard["content"])
+		t.Fatalf("the guard does not guard the store, the broker and its management port:\n%s",
+			guard["content"])
 	}
 	if !reflect.DeepEqual(got[GuardRunningID()]["restart-on"], []any{GuardID(), GuardUnitID()}) {
 		t.Fatalf("the guard is not reloaded when its table changes: %v", got[GuardRunningID()])
@@ -252,7 +255,7 @@ func TestAGivenPortIsUsedEverywhereThePortIs(t *testing.T) {
 		if o := got["adoption.opening-tcp-5433-forwarded"]; o == nil || o["to"] != 5432 {
 			t.Fatalf("no opening for the given port: %v", keys(got))
 		}
-		if guard := got[GuardID()]["content"]; guard != AsGuard([]int{5433, 15673}) {
+		if guard := got[GuardID()]["content"]; guard != AsGuard([]int{5433, 5672, 15673}) {
 			t.Fatalf("the guard does not guard the given ports:\n%s", guard)
 		}
 	}
@@ -281,3 +284,55 @@ func TestAGivenPortIsTheNodesAndReachesSomething(t *testing.T) {
 		t.Fatalf("a given port is called stray: %v", stray)
 	}
 }
+
+// novox/hq ADR 0103: the guard is derived, and from taken modules only — every machine port a taken
+// module publishes that the filter admits from the private network only, and the ports its
+// manifest guards. A module assigned but not taken is not guarded: its port may still be the
+// predecessor's.
+func TestTheGuardIsDerivedFromTakenModulesOnly(t *testing.T) {
+	guardOf := func(with Rendering) string {
+		t.Helper()
+		composed, err := anAdoptedAnchor().Compose(with)
+		if err != nil {
+			t.Fatal(err)
+		}
+		content, _ := byID(composed.Resources)[GuardID()]["content"].(string)
+		return content
+	}
+
+	// The broker taken, the store not: the broker's plain port follows from its listens (from
+	// the mesh, published), its management port from its manifest; the store is not guarded, and
+	// neither is the bus, which the mesh needs from everywhere.
+	with := anchorRendering(true)
+	with.Taken = map[string]bool{"lavinmq": true}
+	if got := guardOf(with); got != AsGuard([]int{5672, 15672}) {
+		t.Fatalf("the guard is not the taken broker's ports:\n%s", got)
+	}
+
+	// A taken module publishing a port admitted from everywhere is not guarded; one admitted from
+	// the mesh is. The registry is exposed everywhere on this node, and hello-web listens from
+	// everywhere.
+	with.Taken = map[string]bool{"distribution": true, "hello-web": true}
+	if got := guardOf(with); got != "" {
+		t.Fatalf("a port admitted from everywhere is guarded:\n%s", got)
+	}
+	with.Settings = nil
+	if got := guardOf(with); got != AsGuard([]int{5000}) {
+		t.Fatalf("the registry, from the mesh only, is not guarded:\n%s", got)
+	}
+
+	// Nothing taken, nothing guarded — and no guard at all rather than an empty set.
+	with = anchorRendering(true)
+	with.Taken = nil
+	if got := guardOf(with); got != "" {
+		t.Fatalf("an untaken store is guarded:\n%s", got)
+	}
+
+	// A given port is followed: where the machine put it is what is refused.
+	with = anchorRendering(true)
+	with.Given = map[string]map[int]int{"lavinmq": {5672: 5682, 15672: 15673}}
+	with.Ports["lavinmq"] = map[int]int{5671: 5671, 5672: 5682}
+	if got := guardOf(with); got != AsGuard([]int{5432, 5682, 15673}) {
+		t.Fatalf("the guard does not follow the given ports:\n%s", got)
+	}
+}

internal/catalogue/declaration.go

@@ -138,6 +138,11 @@ type Rendering struct {
 	// port the software uses (novox/hq ADR 0100) — the foundation's ports, as genesis chose them.
 	// They win over anything the mesh would assign and over a manifest's own long-form mapping.
 	Given map[string]map[int]int
+
+	// Taken is the modules taken on this adopted node (novox/hq ADR 0100). The guard is derived
+	// from these only (ADR 0103): a port of a module assigned but not taken may still be the
+	// predecessor's.
+	Taken map[string]bool
 }
 
 // machinePort is where a module's port lives on this machine, or the port itself when the mesh has
@@ -586,50 +591,100 @@ func (r Resolution) compose(with Rendering, owner map[string]string) ([]map[stri
 		// First, before anything a module declares: what the mesh needs reachable, then its guard.
 		// The order a machine applies is the order written here.
 		ours := Openings(rules, with.Foundation, Published(out))
-		ours = append(ours, GuardResources(r.guarded(out, owner, with))...)
+		ours = append(ours, GuardResources(r.guarded(out, owner, rules, with))...)
 		out = append(ours, out...)
 	}
 	return out, nil
 }
 
-// guarded is the machine ports of every guarded port of the modules here: where each module's
+// guarded is what the mesh's guard refuses on an adopted node (novox/hq ADR 0103): derived, and
-// container publishes it, as composed — or where the machine put it when no container does.
+// for taken modules only.
-func (r Resolution) guarded(out []map[string]any, owner map[string]string, with Rendering) []int {
+//
+// For each taken module, every machine port its containers publish that the filter would admit
+// from the private network only — a published port is forwarded, not received, so a found
+// firewall filtering only what it receives never sees it — together with the ports the module's
+// manifest guards explicitly (the store's port, the broker's management port), wherever the
+// machine put them. A port of a module assigned but not taken is not guarded: it may still be the
+// predecessor's, serving the predecessor's other machines. The foundation's ports are admitted
+// from everywhere and are never guarded.
+func (r Resolution) guarded(out []map[string]any, owner map[string]string, rules []Rule,
+	with Rendering) []int {
+	meshOnly := map[int]bool{}
+	for _, rule := range rules {
+		if rule.Protocol == "tcp" && rule.From == FromMesh {
+			meshOnly[rule.Port] = true
+		}
+	}
+	for _, port := range with.Foundation {
+		delete(meshOnly, port)
+	}
 	seen := map[int]bool{}
 	var ports []int
+	guard := func(at int) {
+		if !seen[at] {
+			seen[at] = true
+			ports = append(ports, at)
+		}
+	}
 	for _, m := range r.Modules {
+		if !with.Taken[m.Module] {
+			continue
+		}
+		var mine []map[string]any
+		for _, resource := range out {
+			if owner[fmt.Sprint(resource["id"])] == m.Module {
+				mine = append(mine, resource)
+			}
+		}
+		for outer := range Published(mine)["tcp"] {
+			if meshOnly[outer] {
+				guard(outer)
+			}
+		}
 		for _, want := range m.Guards {
 			at := with.machinePort(m.Module, want)
-			for _, resource := range out {
+			for _, resource := range mine {
-				if owner[fmt.Sprint(resource["id"])] != m.Module ||
+				if fmt.Sprint(resource["type"]) != "container" {
-					fmt.Sprint(resource["type"]) != "container" {
 					continue
 				}
 				listed, _ := resource["ports"].([]any)
 				for _, entry := range listed {
-					parts := strings.Split(strings.TrimSpace(fmt.Sprint(entry)), ":")
+					outer, inner, _, ok := mapping(fmt.Sprint(entry))
-					if len(parts) < 2 {
+					if ok && inner == want {
-						continue
-					}
-					inner, err := strconv.Atoi(strings.SplitN(parts[len(parts)-1], "/", 2)[0])
-					if err != nil || inner != want {
-						continue
-					}
-					if outer, err := strconv.Atoi(parts[len(parts)-2]); err == nil {
 						at = outer
 					}
 				}
 			}
-			if !seen[at] {
+			guard(at)
-				seen[at] = true
-				ports = append(ports, at)
-			}
 		}
 	}
 	sort.Ints(ports)
 	return ports
 }
 
+// mapping reads a container's port mapping — `[address:]outer:inner[/protocol]`, the address
+// possibly an IPv6 one in brackets — indexing from the end, so an address's own colons never
+// shift the ports. Not ok for a short form or anything that is not a mapping.
+func mapping(written string) (outer, inner int, address string, ok bool) {
+	written = strings.TrimSpace(written)
+	if cut := strings.LastIndex(written, "/"); cut >= 0 {
+		written = written[:cut]
+	}
+	parts := strings.Split(written, ":")
+	if len(parts) < 2 {
+		return 0, 0, "", false
+	}
+	inner, err := strconv.Atoi(parts[len(parts)-1])
+	if err != nil {
+		return 0, 0, "", false
+	}
+	outer, err = strconv.Atoi(parts[len(parts)-2])
+	if err != nil {
+		return 0, 0, "", false
+	}
+	return outer, inner, strings.Join(parts[:len(parts)-2], ":"), true
+}
+
 // Rules is the rule set this node's filter is derived from: every module's listens, what was
 // computed for this machine, and each module's per-node exposure. The same answer whether the node
 // is adopted or converged — the one loads it as a filter, the other declares it as openings.