diff --git a/cmd/mesh-controller/sendable_test.go b/cmd/mesh-controller/sendable_test.go index f41f618..749599f 100644 --- a/cmd/mesh-controller/sendable_test.go +++ b/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) + } +} diff --git a/internal/catalogue/adoption_test.go b/internal/catalogue/adoption_test.go index 6de58f1..bf4c405 100644 --- a/internal/catalogue/adoption_test.go +++ b/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)) + } +} diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index b9cfdf7..a9b5549 100644 --- a/internal/catalogue/declaration.go +++ b/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 } diff --git a/internal/catalogue/filtering.go b/internal/catalogue/filtering.go index 674ebe1..994c2b0 100644 --- a/internal/catalogue/filtering.go +++ b/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 +} diff --git a/internal/catalogue/foundation_manifests_test.go b/internal/catalogue/foundation_manifests_test.go index 0493abe..ff4ce36 100644 --- a/internal/catalogue/foundation_manifests_test.go +++ b/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"]) + } +}