diff --git a/cmd/mesh-control/plan.go b/cmd/mesh-control/plan.go index be10c90..18843bd 100644 --- a/cmd/mesh-control/plan.go +++ b/cmd/mesh-control/plan.go @@ -266,16 +266,26 @@ func declarationWith(ctx context.Context, open *stores, node string, ports := map[string]map[int]int{} for _, m := range plan.Modules { for _, l := range m.Listens { - at, err := inv.PortFor(ctx, node, m.Module, l.Port, l.Fixed) - if err != nil { - return nil, fmt.Errorf( - "%s needs %d reachable on %s and it could not be assigned: %w", - m.Module, l.Port, node, err) + // **Only a port the module actually publishes is the mesh's to move.** A container's + // mapping is the thing that translates; without one the software binds what it binds, + // and an assignment would not move the service — it would open the wrong number in the + // rule set and leave the real one shut. Recorded either way, because this map means + // *where this module's port is on this machine* and every reader of it needs that + // answer whether or not the mesh was the one who chose it. + where, mayAssign := m.MachineSide(l.Port) + if mayAssign { + at, err := inv.PortFor(ctx, node, m.Module, l.Port, l.Fixed) + if err != nil { + return nil, fmt.Errorf( + "%s needs %d reachable on %s and it could not be assigned: %w", + m.Module, l.Port, node, err) + } + where = at.Machine } if ports[m.Module] == nil { ports[m.Module] = map[int]int{} } - ports[m.Module][l.Port] = at.Machine + ports[m.Module][l.Port] = where } } diff --git a/internal/catalogue/machineside_test.go b/internal/catalogue/machineside_test.go new file mode 100644 index 0000000..553e1a5 --- /dev/null +++ b/internal/catalogue/machineside_test.go @@ -0,0 +1,60 @@ +package catalogue + +import "testing" + +// A module with nothing that publishes binds what it binds, and the mesh may not move it. +// +// This is the case that made novox/hq 04-ISSUES/028's fix wrong on its first pass: a port was +// assigned to every module that declared one, so a service listening directly had the rule set +// opened on a number nothing was listening on, and its real port shut. The firewall reported +// success and blocked the service, which is the exact failure the mechanism exists to prevent. +func TestAPortNothingPublishesIsNotTheMeshsToMove(t *testing.T) { + m := Manifest{Module: "talker", Listens: []Listening{{Port: 9101, From: FromMesh}}} + at, mayAssign := m.MachineSide(9101) + if mayAssign { + t.Fatal("the mesh took a port it cannot move: nothing translates it, so assigning one " + + "opens the wrong number and leaves the service unreachable") + } + if at != 9101 { + t.Fatalf("a port nothing publishes reaches the machine where it binds, not at %d", at) + } +} + +// A container publishing in short form is exactly the case the mesh may choose. +func TestAContainerPublishingShortIsTheMeshsToChoose(t *testing.T) { + m := Manifest{Module: "store", Resources: []map[string]any{ + {"type": "container", "id": "server", "ports": []any{"5432"}}, + }} + if _, mayAssign := m.MachineSide(5432); !mayAssign { + t.Fatal("a container's mapping is what translates a port, so this one is the mesh's to " + + "choose; refusing it puts every module back on a number it guessed") + } +} + +// A manifest that wrote its own mapping already chose, and the machine side is the outer one. +func TestAMappingTheManifestWroteIsNotReassigned(t *testing.T) { + m := Manifest{Module: "mail", Resources: []map[string]any{ + {"type": "container", "id": "front", "ports": []any{"7080:80"}}, + }} + for _, named := range []int{7080, 80} { + at, mayAssign := m.MachineSide(named) + if mayAssign { + t.Fatalf("%d was reassigned though the manifest published it explicitly, which "+ + "would open a rule on a port the container does not publish", named) + } + if at != 7080 { + t.Fatalf("naming %d gave %d; the machine side of 7080:80 is 7080", named, at) + } + } +} + +// A port some other container publishes is not this port. +func TestAPortNotInTheMappingIsNotFound(t *testing.T) { + m := Manifest{Module: "mail", Resources: []map[string]any{ + {"type": "container", "id": "front", "ports": []any{"25", "7080:80"}}, + }} + if at, mayAssign := m.MachineSide(993); mayAssign || at != 993 { + t.Fatalf("993 is published by nothing here, so it binds where it binds: got %d, %v", + at, mayAssign) + } +} diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index c151e00..910a09f 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -11,6 +11,7 @@ import ( "fmt" "regexp" "sort" + "strconv" "strings" ) @@ -711,3 +712,52 @@ func (m Manifest) OffersAt(scope string) []string { sort.Strings(out) return out } + +// MachineSide says where a module's declared port reaches this machine, and whether the mesh is +// free to choose it. +// +// **The mesh may only move a port it actually publishes** (novox/hq ADR 0038). A container's +// mapping is the thing that translates, so where there is one the mesh can put the machine side +// anywhere it likes. Where there is not, the software binds what it binds: assigning a port then +// does not move the service, it just opens the wrong number in the rule set and leaves the real +// one shut — a firewall that reports success and blocks the thing it was asked to admit. +// +// Three cases, and only the first belongs to the mesh: +// +// - a container publishes it in short form — the mesh chooses +// - a container publishes it as host:container — the manifest already chose +// - nothing publishes it — whatever binds it, binds it +// +// Either side of a long mapping counts as naming it, and the host side is what comes back. A +// module may reasonably read `listens` as the port its software uses or as the port the machine +// exposes, and both readings have the same right answer here. +func (m Manifest) MachineSide(port int) (at int, mayAssign bool) { + for _, r := range m.Resources { + if fmt.Sprint(r["type"]) != "container" { + continue + } + listed, ok := r["ports"].([]any) + if !ok { + continue + } + for _, entry := range listed { + written := strings.TrimSpace(fmt.Sprint(entry)) + host, inside, long := strings.Cut(written, ":") + if !long { + if n, err := strconv.Atoi(written); err == nil && n == port { + return port, true + } + continue + } + outer, err := strconv.Atoi(strings.TrimSpace(host)) + if err != nil { + continue + } + inner, err := strconv.Atoi(strings.TrimSpace(inside)) + if err == nil && (outer == port || inner == port) { + return outer, false + } + } + } + return port, false +}