cmd/mesh-controller/sendable_test.go

@@ -272,3 +272,82 @@ func TestTheGuardIsSentForTakenModulesOnly(t *testing.T) {
 	}
 	t.Fatal("a taken store is not guarded")
 }
+
+// novox/hq ADR 0038 and 0100: a module may publish a port the long way — `2222:22`, because the
+// machine's own ssh daemon holds 22 — and say it listens on the machine side of that mapping,
+// which is the number anything reaching it dials. A node moves that port by naming it, and the
+// number has to reach everything derived from it at once: what the runtime is handed, what an
+// adopted node is told to open, what its guard refuses, and what a consumer elsewhere dials.
+//
+// It reached none of them. The setting was refused outright for naming the machine side — so a
+// module's port could not be put back where the machine it replaces had it, and, worse, the
+// node's every push failed for as long as the setting existed.
+func TestTheMachineSideOfAMappingIsMovedEverywhereTheNumberIsUsed(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	register(t, open, catalogue.Manifest{Module: "forge", Version: "1",
+		Provides: []catalogue.Offer{{Name: "git-over-ssh", Scope: catalogue.ScopeMesh}},
+		Serves:   map[string]map[string]any{"git-over-ssh": {"port": 2222}},
+		Listens:  []catalogue.Listening{{Port: 2222, From: catalogue.FromMesh}},
+		Resources: []map[string]any{{"id": "server", "type": "container", "name": "forge",
+			"ports": []any{"2222:22"},
+			"image": "registry.example/forge@sha256:" + strings.Repeat("c", 64)}}})
+	register(t, open, catalogue.Manifest{Module: "app", Version: "1",
+		Requires: []string{"git-over-ssh"}})
+	if err := open.inventory.SetAdopted(ctx, "anchor", true); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := assign(ctx, open, "anchor", "forge"); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := assign(ctx, open, "laptop", "app"); err != nil {
+		t.Fatal(err)
+	}
+	if err := open.inventory.Take(ctx, "anchor", "forge"); err != nil {
+		t.Fatal(err)
+	}
+	if err := open.inventory.SetSettings(ctx, "anchor", "forge",
+		map[string]any{catalogue.PortsSetting: map[string]any{"2222": 222}}); err != nil {
+		t.Fatal(err)
+	}
+
+	resources := composed(t, open, "anchor").Resources
+	var container, opening map[string]any
+	for _, r := range resources {
+		switch r["id"] {
+		case "forge.server":
+			container = r
+		case catalogue.OpeningID("tcp", 222, catalogue.PathForwarded):
+			opening = r
+		}
+		if id, _ := r["id"].(string); strings.HasPrefix(id, "adoption.opening-tcp-2222-") {
+			t.Errorf("the adopted node is told to open the port the forge was moved off: %s", id)
+		}
+	}
+	if container == nil || !reflect.DeepEqual(container["ports"], []any{"222:22"}) {
+		t.Fatalf("the forge's container publishes %v", container["ports"])
+	}
+	if opening == nil || opening["to"] != 22 || opening["from"] != catalogue.OpeningFromMesh {
+		t.Fatalf("no opening for the port this node gave the forge: %v", opening)
+	}
+	for _, r := range resources {
+		if r["id"] == catalogue.GuardID() && r["content"] != catalogue.AsGuard([]int{222}) {
+			t.Fatalf("the guard does not refuse the port the forge is on:\n%s", r["content"])
+		}
+	}
+
+	// And the consumer on the other machine dials the same number.
+	plan, _, err := planFor(ctx, open, "laptop")
+	if err != nil {
+		t.Fatal(err)
+	}
+	var told any
+	for _, n := range plan.Needs {
+		if n.Name == "git-over-ssh" {
+			told = n.Serves["port"]
+		}
+	}
+	if told != 222 {
+		t.Fatalf("the consumer is told the forge answers on %v", told)
+	}
+}

internal/catalogue/adoption_test.go

@@ -429,3 +429,202 @@ func TestAGuardedPortOpenedToEveryoneIsNotGuarded(t *testing.T) {
 		t.Fatalf("a port opened to everyone is still guarded:\n%s", guard)
 	}
 }
+
+// A module that publishes its ssh port the long way — `2222:22`, because the machine's own daemon
+// holds 22 — and says it listens on the machine side of that mapping, which is what anything
+// reaching it dials.
+func aForge() Manifest {
+	return Manifest{Module: "forge",
+		Provides: []Offer{{Name: "git-over-ssh", Scope: ScopeMesh}},
+		Serves:   map[string]map[string]any{"git-over-ssh": {"port": 2222}},
+		Listens:  []Listening{{Port: 3000, From: FromMesh}, {Port: 2222, From: FromMesh}},
+		Resources: []map[string]any{{"id": "server", "type": "container", "name": "forge",
+			"ports": []any{"3000", "2222:22"}}}}
+}
+
+// portsAsThePlanWould is where this machine puts each of a module's ports, derived the way
+// cmd/mesh-controller/plan.go derives it: a given port first, looked up by the port the module
+// says it listens on, and otherwise wherever the manifest's own mapping already put it. Written
+// here because everything below — the filter, the openings, the guard, what a consumer is told —
+// reads that map, and a given port that the lookup does not find moves the container's mapping
+// and nothing else.
+func portsAsThePlanWould(m Manifest, given map[int]int) map[int]int {
+	out := map[int]int{}
+	for _, l := range m.Listens {
+		if at, is := given[l.Port]; is {
+			out[l.Port] = at
+			continue
+		}
+		at, _ := m.MachineSide(l.Port)
+		out[l.Port] = at
+	}
+	return out
+}
+
+// novox/hq ADR 0100 and 0038: the machine side of a long-form mapping is a port the module
+// publishes, so a node may move it — and a module may name a mapping by either end.
+func TestAGivenPortNamesEitherEndOfWhatTheModulePublishes(t *testing.T) {
+	forge := aForge()
+	node := func(v any) []Layer {
+		return []Layer{{From: "node anchor", Values: map[string]any{PortsSetting: v}}}
+	}
+
+	// The machine side — the number this module says it listens on, and the one the predecessor
+	// had somewhere else. Answered under both ends, because the plan looks a given port up by the
+	// port the module declares and the container's mapping is rewritten by the port inside it.
+	given, err := GivenPorts(forge, node(map[string]any{"2222": float64(222)}))
+	if err != nil {
+		t.Fatalf("the machine side of a mapping cannot be given a port: %v", err)
+	}
+	if given[2222] != 222 || given[22] != 222 {
+		t.Fatalf("the forge was given %v, and its mapping has two ends", given)
+	}
+
+	// The container's own port names the same mapping and means the same thing.
+	if inside, err := GivenPorts(forge, node(map[string]any{"22": float64(222)})); err != nil ||
+		inside[2222] != 222 || inside[22] != 222 {
+		t.Fatalf("the container's end of the mapping was given %v: %v", inside, err)
+	}
+
+	// A short form is published on the number it names, and is unchanged by any of this.
+	if short, err := GivenPorts(forge, node(map[string]any{"3000": float64(2999)})); err != nil ||
+		short[3000] != 2999 {
+		t.Fatalf("the short form was given %v: %v", short, err)
+	}
+
+	// A port no container publishes is still refused, in the same words.
+	if _, err := GivenPorts(forge, node(map[string]any{"9000": float64(9100)})); err == nil ||
+		!strings.Contains(err.Error(), "does not publish") {
+		t.Fatalf("a port the forge does not publish was given: %v", err)
+	}
+
+	// And the two ends of one mapping given two different numbers is one setting contradicting
+	// the other: the machine publishes it once.
+	if _, err := GivenPorts(forge, node(map[string]any{
+		"2222": float64(222), "22": float64(300)})); err == nil ||
+		!strings.Contains(err.Error(), "one mapping") {
+		t.Fatalf("the two ends of one mapping were given different ports: %v", err)
+	}
+	// Said at both ends with the same number, it is still said twice, and refused where every
+	// other repeated machine port is — as the inventory refuses it when the setting is stored,
+	// which is the layer that sees it first.
+	if _, err := GivenPorts(forge, node(map[string]any{
+		"2222": float64(222), "22": float64(222)})); err == nil ||
+		!strings.Contains(err.Error(), "to both its 22 and its 2222") {
+		t.Fatalf("one mapping given one machine port at both ends: %v", err)
+	}
+	// A number that names two different mappings names neither: which one to move is not said.
+	twice := aForge()
+	twice.Resources[0]["ports"] = []any{"22", "2222:22"}
+	if _, err := GivenPorts(twice, node(map[string]any{"22": float64(222)})); err == nil ||
+		!strings.Contains(err.Error(), "twice") {
+		t.Fatalf("a number naming two of the module's mappings was accepted: %v", err)
+	}
+}
+
+// And the number reaches everything derived from it. The fault this is written against moved the
+// container's mapping alone: the filter opened the port the software had left, the adopted node's
+// opening named it too, the guard refused it, and a consumer was sent to it.
+func TestAGivenMachineSideReachesTheFilterTheOpeningAndTheConsumer(t *testing.T) {
+	forge := aForge()
+	given, err := GivenPorts(forge, []Layer{{From: "node anchor",
+		Values: map[string]any{PortsSetting: map[string]any{"2222": float64(222)}}}})
+	if err != nil {
+		t.Fatalf("the machine side of a mapping cannot be given a port: %v", err)
+	}
+	r := Resolution{Node: "anchor", Modules: []Manifest{forge}}
+	with := Rendering{
+		Ports:   map[string]map[int]int{"forge": portsAsThePlanWould(forge, given)},
+		Given:   map[string]map[int]int{"forge": given},
+		Mesh:    []string{"10.77.0.1"},
+		Adopted: true,
+		Taken:   map[string]bool{"forge": true},
+	}
+
+	// What the runtime is handed: the machine's own port on the outside, the container's within.
+	composed, err := r.Compose(with)
+	if err != nil {
+		t.Fatalf("the forge does not compose: %v", err)
+	}
+	got := byID(composed.Resources)
+	if ports := got["forge.server"]["ports"]; !reflect.DeepEqual(ports, []any{"3000:3000", "222:22"}) {
+		t.Fatalf("the forge's container publishes %v", ports)
+	}
+
+	// What the filter would open, were the node converged.
+	rules, err := r.Rules(with)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var opened []int
+	for _, rule := range rules {
+		opened = append(opened, rule.Port)
+	}
+	if !reflect.DeepEqual(opened, []int{222, 3000}) {
+		t.Fatalf("the filter opens %v, not where the machine puts the forge", opened)
+	}
+
+	// And what it is declared instead, adopted: an opening on the machine's port, naming the
+	// container's port on the forwarded path — a published port is forwarded, never received.
+	opening := got[OpeningID("tcp", 222, PathForwarded)]
+	if opening == nil || opening["to"] != 22 || opening["from"] != OpeningFromMesh {
+		t.Fatalf("no opening for the port this node gave the forge: %v", keys(got))
+	}
+	for id := range got {
+		if strings.HasPrefix(id, "adoption.opening-tcp-2222-") {
+			t.Errorf("an opening for the port the forge was moved off: %s", id)
+		}
+	}
+
+	// The guard refuses it where the machine put it, and nothing where it used to be.
+	if guard, _ := got[GuardID()]["content"].(string); guard != AsGuard([]int{222, 3000}) {
+		t.Fatalf("the guard does not follow the given port:\n%s", guard)
+	}
+
+	// And a consumer is sent to the same number, which is read from what the module serves.
+	if told := ServedOn(forge, "git-over-ssh", with.Ports["forge"])["port"]; told != 222 {
+		t.Fatalf("a consumer is told the forge answers on %v", told)
+	}
+}
+
+// And the mapping itself moves under either name, because Rendering.Given is a map anybody
+// composing a declaration hands in: keyed by the machine side, which is what a module declaring
+// 2222 calls its port, only the outside moves and the container's own port stays as written.
+func TestAMappingIsMovedUnderEitherOfItsNames(t *testing.T) {
+	for _, c := range []struct {
+		written string
+		given   map[int]int
+		want    string
+	}{
+		{"2222:22", map[int]int{2222: 222}, "222:22"},
+		{"2222:22", map[int]int{22: 222}, "222:22"},
+		{"127.0.0.1:15672:15672/tcp", map[int]int{15672: 15673}, "127.0.0.1:15673:15672/tcp"},
+		{"2222:22", map[int]int{3000: 2999}, "2222:22"},
+		{"2222:22", nil, "2222:22"},
+	} {
+		if got := givenOuter(c.written, c.given); got != c.want {
+			t.Errorf("%s given %v is published as %s, want %s", c.written, c.given, got, c.want)
+		}
+	}
+}
+
+// The same on a node that was given nothing, which is where the derivation was never at fault:
+// a long-form mapping is published where the manifest put it, and an adopted node opens that port
+// on the forwarded path like any other. What broke it was the number reaching only the mapping.
+func TestALongFormPortIsOpenedWhereTheManifestPublishesIt(t *testing.T) {
+	forge := aForge()
+	r := Resolution{Node: "anchor", Modules: []Manifest{forge}}
+	composed, err := r.Compose(Rendering{
+		Ports:   map[string]map[int]int{"forge": portsAsThePlanWould(forge, nil)},
+		Mesh:    []string{"10.77.0.1"},
+		Adopted: true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	got := byID(composed.Resources)
+	opening := got[OpeningID("tcp", 2222, PathForwarded)]
+	if opening == nil || opening["to"] != 22 {
+		t.Fatalf("no opening for the forge's published ssh port: %v", keys(got))
+	}
+}

internal/catalogue/declaration.go

@@ -1289,7 +1289,13 @@ func publishedOn(resource map[string]any, module string, with Rendering) {
 }
 
 // givenOuter rewrites the machine side of a long-form mapping to the port this node was given for
-// its software side, when it was given one.
+// it, when it was given one.
+//
+// **Under either of the mapping's names.** A node gives a port by the number the module names it
+// by, and a module publishing `"2222:22"` may say it listens on 22 or on 2222 — GivenPorts accepts
+// both and answers to both, so looking the machine side up first and the container's port second
+// finds the same number either way. Read only by the container's port, this moved nothing for the
+// module that declares the machine side, and the setting was refused before it got here.
 func givenOuter(written string, given map[int]int) string {
 	if len(given) == 0 {
 		return written
@@ -1299,11 +1305,19 @@ func givenOuter(written string, given map[int]int) string {
 		mapping, protocol = written[:cut], written[cut:]
 	}
 	parts := strings.Split(mapping, ":")
+	if len(parts) < 2 {
+		return written
+	}
 	inner, err := strconv.Atoi(strings.TrimSpace(parts[len(parts)-1]))
 	if err != nil {
 		return written
 	}
 	at, ok := given[inner]
+	if outer, err := strconv.Atoi(strings.TrimSpace(parts[len(parts)-2])); err == nil {
+		if machine, named := given[outer]; named {
+			at, ok = machine, true
+		}
+	}
 	if !ok {
 		return written
 	}

internal/catalogue/filtering.go

@@ -483,12 +483,26 @@ const MeshWideLayer = "the mesh"
 // was configured with, and moving that number would put it in the filter, in the openings and in
 // what consumers are told while the software still listens on the old one — a port that reads as
 // moved and is not.
+//
+// **Either name of a mapping names it.** A short form publishes one number, which is the
+// container's port and the machine's at once. A mapping written the long way — `"2222:22"` — has
+// two, and a module reasonably declares it listens on either: the port its software uses, or the
+// port the machine already serves on. Both are accepted, and both come back, so that whoever
+// reads this — the ports map, the filter, the openings, what a consumer is told, and the mapping
+// the runtime is handed — finds the same number under the key it happens to hold. Keyed one way
+// and read the other, the setting moved the container's mapping and nothing else: a firewall,
+// a set of openings and a consumer all pointing at a port the software had left.
 func GivenPorts(m Manifest, layers []Layer) (map[int]int, error) {
-	known := map[int]bool{}
-	for _, p := range containerPorts(m) {
-		known[p] = true
+	// Every name a setting may use, and the mapping it names.
+	names := map[int][]publishing{}
+	for _, p := range publishedPorts(m) {
+		names[p.inner] = append(names[p.inner], p)
+		if p.machine != p.inner {
+			names[p.machine] = append(names[p.machine], p)
+		}
 	}
-	out := map[int]int{}
+	chose := map[int]int{}        // the port a setting named → the machine port it gave it
+	meant := map[int]publishing{} // and which of the module's mappings that was
 	for _, layer := range layers {
 		raw, ok := layer.Values[PortsSetting]
 		if !ok {
@@ -508,11 +522,22 @@ func GivenPorts(m Manifest, layers []Layer) (map[int]int, error) {
 			if err != nil {
 				return nil, fmt.Errorf("%s gives %q a port, which is not a port", m.Module, portText)
 			}
-			if !known[port] {
+			publishes, known := names[port]
+			if !known {
 				return nil, fmt.Errorf("%s gives port %d a machine port, and no container of its "+
 					"publishes %d — the mesh cannot move a port the module does not publish; the "+
 					"software would go on listening where it was told to", m.Module, port, port)
 			}
+			// The same number naming two different mappings — `"22"` beside `"2222:22"`, say.
+			// Refused rather than picked: moving one of them and leaving the other is a mapping
+			// the operator did not ask for and cannot see, and the two readings differ.
+			for _, other := range publishes[1:] {
+				if other != publishes[0] {
+					return nil, fmt.Errorf("%s gives port %d a machine port, and its containers "+
+						"publish %d twice — as %s and as %s; which one to move is not said",
+						m.Module, port, port, publishes[0], other)
+				}
+			}
 			at, ok := asPort(value)
 			if !ok || at < 1 || at > 65535 {
 				return nil, fmt.Errorf("%s gives port %d the machine port %v, which is not a port",
@@ -522,28 +547,58 @@ func GivenPorts(m Manifest, layers []Layer) (map[int]int, error) {
 				return nil, fmt.Errorf("%s gives port %d the machine port %d, which is ssh's — the "+
 					"one port a machine may never lose", m.Module, port, at)
 			}
-			out[port] = at
+			chose[port] = at
+			meant[port] = publishes[0]
 		}
 	}
-	// One holder per machine port, within the module too.
+	// One holder per machine port, within the module too — which also settles a mapping named at
+	// both ends, because the machine publishes it once whichever end the setting called it.
 	holder := map[int]int{}
-	for port, at := range out {
+	for _, port := range sorted(chose) {
+		at := chose[port]
 		if other, twice := holder[at]; twice {
 			return nil, fmt.Errorf("%s gives machine port %d to both its %d and its %d", m.Module,
 				at, min(port, other), max(port, other))
 		}
 		holder[at] = port
 	}
+	// And one machine port per mapping: `{"22": 222, "2222": 300}` is two numbers for the one
+	// thing the machine publishes, and neither is more right.
+	out := map[int]int{}
+	said := map[publishing]int{}
+	for _, port := range sorted(chose) {
+		at, mapped := chose[port], meant[port]
+		if was, twice := said[mapped]; twice {
+			return nil, fmt.Errorf("%s gives %s the machine ports %d and %d — its %d and its %d "+
+				"are the two ends of one mapping, and it is published once", m.Module, mapped,
+				min(was, at), max(was, at), min(mapped.machine, mapped.inner),
+				max(mapped.machine, mapped.inner))
+		}
+		said[mapped] = at
+		out[mapped.inner] = at
+		out[mapped.machine] = at
+	}
 	if len(out) == 0 {
 		return nil, nil
 	}
 	return out, nil
 }
 
-// containerPorts are the software ports a module's containers publish, whichever form they are
-// written in.
-func containerPorts(m Manifest) []int {
-	var out []int
+// publishing is one mapping a module's container writes: the port the container itself uses, and
+// the machine port the manifest put it on — the same number for a short form, which the runtime
+// publishes on the port it names.
+type publishing struct{ machine, inner int }
+
+func (p publishing) String() string {
+	if p.machine == p.inner {
+		return strconv.Itoa(p.inner)
+	}
+	return fmt.Sprintf("%d:%d", p.machine, p.inner)
+}
+
+// publishedPorts is every mapping a module's containers publish, whichever form it is written in.
+func publishedPorts(m Manifest) []publishing {
+	var out []publishing
 	for _, r := range m.Resources {
 		if fmt.Sprint(r["type"]) != "container" {
 			continue
@@ -551,14 +606,27 @@ func containerPorts(m Manifest) []int {
 		listed, _ := r["ports"].([]any)
 		for _, entry := range listed {
 			written := strings.TrimSpace(fmt.Sprint(entry))
+			if machine, inner, _, ok := mapping(written); ok {
+				out = append(out, publishing{machine: machine, inner: inner})
+				continue
+			}
 			if cut := strings.LastIndex(written, "/"); cut >= 0 {
 				written = written[:cut]
 			}
-			parts := strings.Split(written, ":")
-			if n, err := strconv.Atoi(parts[len(parts)-1]); err == nil {
-				out = append(out, n)
+			if n, err := strconv.Atoi(strings.TrimSpace(written)); err == nil {
+				out = append(out, publishing{machine: n, inner: n})
 			}
 		}
 	}
 	return out
 }
+
+// sorted is a settings map's ports in order, so a refusal reads the same on every run.
+func sorted(of map[int]int) []int {
+	out := make([]int, 0, len(of))
+	for port := range of {
+		out = append(out, port)
+	}
+	sort.Ints(out)
+	return out
+}

internal/catalogue/foundation_manifests_test.go

@@ -250,3 +250,56 @@ func TestTheForgesOwnAddressFollowsThePortTheNodeGaveIt(t *testing.T) {
 			"whatever reads it dials a dead port", env["MESH_GITEA_URL"])
 	}
 }
+
+// **And the port the forge publishes the long way is the node's too** (novox/hq ADR 0100).
+//
+// The forge's ssh port is written `2222:22` — the machine's own daemon holds 22, so the module
+// takes 2222 and says so in `listens`. A node whose predecessor served git on another number
+// cannot be told to leave it there unless the setting may name the machine side of that mapping,
+// which is the number the manifest itself uses everywhere else. Composed from the manifest in the
+// catalogue beside this checkout, because what the mesh can move is a fact about what the module
+// actually writes.
+func TestTheForgesSshPortIsGivenByTheNumberTheForgeCallsIt(t *testing.T) {
+	forge := catalogueManifest(t, "gitea")
+	given, err := GivenPorts(forge, []Layer{{From: "anchor",
+		Values: map[string]any{PortsSetting: map[string]any{"2222": float64(222)}}}})
+	if err != nil {
+		t.Fatalf("the forge's ssh port cannot be given on a node: %v", err)
+	}
+	// Under the number the module listens on, which is how the plan finds it, and under the
+	// container's own port, which is how the mapping is rewritten.
+	if given[2222] != 222 || given[22] != 222 {
+		t.Fatalf("the forge was given %v", given)
+	}
+
+	resolved, err := forge.Resolve([]Built{{
+		Name: "runtime", Kind: ArtifactImage,
+		Reference: "registry.example/gitea-runtime@sha256:" + strings.Repeat("a", 64),
+	}})
+	if err != nil {
+		t.Fatalf("the forge's manifest does not resolve against its own build: %v", err)
+	}
+	r := Resolution{Node: "anchor", Modules: []Manifest{resolved}, Needs: []Needed{
+		{Name: "postgres-database", For: "gitea", From: "anchor", At: "127.0.0.1",
+			Serves: map[string]any{"port": float64(5432)}, Sealed: "sealed-db"},
+		{Name: "route", For: "gitea", From: "anchor"},
+		{Name: "secret", For: "gitea", From: "anchor", Local: "internal-token", Sealed: "sealed-token"},
+		{Name: "secret", For: "gitea", From: "anchor", Local: "admin", Sealed: "sealed-admin"},
+	}}
+	out, err := r.Declaration(Rendering{
+		Needed: map[string]map[string]string{"gitea": {"broker": "sealed-broker"}},
+		Ports:  map[string]map[int]int{"gitea": {3000: 3000, 2222: 222}},
+		Given:  map[string]map[int]int{"gitea": given},
+	})
+	if err != nil {
+		t.Fatalf("the forge does not compose: %v", err)
+	}
+	server := fileNamed(out, "gitea.server")
+	if server == nil {
+		t.Fatalf("the forge's own container is not in the declaration: %v", out)
+	}
+	if published := fmt.Sprint(server["ports"]); !strings.Contains(published, "222:22") ||
+		strings.Contains(published, "2222:22") {
+		t.Fatalf("the forge is published on %v, not the port this node gave it", server["ports"])
+	}
+}