diff --git a/internal/catalogue/foundation_manifests_test.go b/internal/catalogue/foundation_manifests_test.go index c07d5f4..0493abe 100644 --- a/internal/catalogue/foundation_manifests_test.go +++ b/internal/catalogue/foundation_manifests_test.go @@ -196,3 +196,57 @@ func TestTheBuildersPackageBindingKeepsItsIdentity(t *testing.T) { } } } + +// **And the forge's own address follows it**, composed from the manifest in the catalogue beside +// this checkout (novox/hq 04-ISSUES/088). +// +// The forge is reached a third way that neither test above covers: by its own sidecar, over the +// machine's loopback, told where to go in its environment. The `2999:3000` mapping that lets the +// forge go on binding 3000 does nothing for a caller dialling the machine — so a literal there is +// wrong on every node whose assignment differs, and wrong for a second reason on a node given the +// port (ADR 0100). Composed through the whole path, because what proves the placeholder resolves +// in an `env` at all is a declaration, not a manifest. +func TestTheForgesOwnAddressFollowsThePortTheNodeGaveIt(t *testing.T) { + forge, err := catalogueManifest(t, "gitea").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{forge}, 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"}, + }} + + // The number this node was given for the forge — the one the machine it is about to run on + // already publishes. + out, err := r.Declaration(Rendering{ + Needed: map[string]map[string]string{"gitea": {"broker": "sealed-broker"}}, + Given: map[string]map[int]int{"gitea": {3000: 2999}}, + }) + if err != nil { + t.Fatalf("the forge does not compose: %v", err) + } + + // What the machine publishes, and what the forge's sidecar is told to dial: one number. + 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, "2999:3000") { + t.Fatalf("the forge is not published on the port this node gave it: %v", server["ports"]) + } + runtime := fileNamed(out, "gitea.runtime") + if runtime == nil { + t.Fatalf("the forge's sidecar is not in the declaration: %v", out) + } + env, _ := runtime["env"].(map[string]any) + if env["MESH_GITEA_URL"] != "http://127.0.0.1:2999" { + t.Fatalf("the forge's sidecar dials %v while the machine publishes the forge on 2999 — "+ + "whatever reads it dials a dead port", env["MESH_GITEA_URL"]) + } +} diff --git a/internal/catalogue/port_into.go b/internal/catalogue/port_into.go new file mode 100644 index 0000000..8cc5866 --- /dev/null +++ b/internal/catalogue/port_into.go @@ -0,0 +1,164 @@ +package catalogue + +import ( + "fmt" + "regexp" + "sort" + "strconv" + "strings" +) + +// Telling a module which port it was given. +// +// **The mesh assigns the machine-side port and a module does not choose one** +// ([ADR 0038](../../02-DECISIONS/0038-the-mesh-assigns-the-port.md)); on a node given one for a +// module it is the operator's number rather than the mesh's +// ([ADR 0100](../../02-DECISIONS/0100-a-node-in-use-is-adopted-before-it-is-converged.md)). For a +// container's own listening socket that is invisible: the mesh rewrites `ports` into +// `assigned:wanted`, the software inside binds the number it has always bound, and the machine +// publishes a different one. +// +// **Two kinds of resource have no such layer.** +// +// - **A process** runs on the machine, there is nothing to rewrite, and it binds whatever its +// configuration says — so without this, every process binds the number written in its own +// config, two modules declaring the same one collide, and the mesh's whole reason for +// assigning ports is defeated by the resource kind that most needs it. +// - **A container that DIALS the machine** — a module's own sidecar reaching the service beside +// it over the machine's loopback — is told that address in its environment, and the mapping +// that saves the listener does nothing for the caller: what it must dial is the machine-side +// number, which is exactly the one the module cannot know (novox/hq 04-ISSUES/088). +// +// So a module asks. `${port:8080}` is "the machine-side port you gave me for the 8080 I said I +// listen on", and the module writes that where it would otherwise have written a literal — in a +// file's content, or in a value of a container's `env`. +// +// **The environment is filled by the control plane, exactly as a bound value is.** A port is not +// secret — the mesh holds it in the clear — so there is nothing for the host to be the only +// witness of, and the host learns no new field. That is what separates this from +// [ADR 0086](../../02-DECISIONS/0086-a-secret-reaches-a-process-as-a-file.md), which refuses a +// `${secret:…}` in an `env` outright: the objection there is to the value being in an environment +// at all, not to who fills it in. +// +// **It answers with the machine's number, wherever it is written.** A container reaching a sibling +// over the runtime's own network reaches it on the port inside that container and goes on writing +// that number literally — it is a number the module does control. This is for the machine side, +// which is the side nobody but the mesh can know. + +// ofPort is where a module asks which port it was given: ${port:}. +var ofPort = regexp.MustCompile(`\$\{port:([0-9]+)\}`) + +// portsUsed are the ports a written value asks about, first appearance first. +func portsUsed(content string) []int { + var used []int + seen := map[int]bool{} + for _, m := range ofPort.FindAllStringSubmatch(content, -1) { + n, err := strconv.Atoi(m[1]) + if err != nil || seen[n] { + continue + } + seen[n] = true + used = append(used, n) + } + return used +} + +// portInto replaces a resource's ${port:…} placeholders with what this machine assigned — in a +// file's content, and in a value of a container's environment. +// +// A port the module did not say it listens on is refused, for the same reason a binding's unknown +// key is: the module is asking about something it never declared, and the answer would be a guess. +// Left alone, the literal would be written into a configuration file, or handed to a process as +// its environment, and read as a port number. +func portInto(resource map[string]any, module string, listens []Listening, with Rendering) error { + switch fmt.Sprint(resource["type"]) { + case "file": + content, ok := resource["content"].(string) + if !ok { + return nil + } + filled, err := portsFilledInto(content, + fmt.Sprintf("%s has a file that", module), module, listens, with) + if err != nil { + return err + } + resource["content"] = filled + + case "container": + env, ok := resource["env"].(map[string]any) + if !ok { + return nil + } + // In a stated order, so a container with two bad values always refuses on the same one. + named := make([]string, 0, len(env)) + for key := range env { + named = append(named, key) + } + sort.Strings(named) + + // **A fresh map, and only when something changes.** This map came out of the module's + // manifest and the resource around it is a shallow copy, so filling a value in place would + // change what the catalogue holds for every other machine running the module — the trap + // withMeshNames is written to avoid, one field along. + var filled map[string]any + for _, key := range named { + written, ok := env[key].(string) + if !ok || len(portsUsed(written)) == 0 { + continue + } + value, err := portsFilledInto(written, + fmt.Sprintf("%s's container %s sets %s to something that", + module, resource["name"], key), module, listens, with) + if err != nil { + return err + } + if filled == nil { + filled = map[string]any{} + for k, v := range env { + filled[k] = v + } + } + filled[key] = value + } + if filled != nil { + resource["env"] = filled + } + } + return nil +} + +// portsFilledInto answers every ${port:…} in one written value, or refuses. `where` names the +// place it was written, so a refusal is one edit from right whichever kind of resource it came +// out of. +func portsFilledInto(written, where, module string, listens []Listening, with Rendering) ( + string, error) { + for _, wanted := range portsUsed(written) { + var declared bool + for _, l := range listens { + if l.Port == wanted { + declared = true + } + } + if !declared { + return "", fmt.Errorf( + "%s says ${port:%d}, and %s does not say it listens on %d. A module is told the "+ + "port it was given for something it declared, and %s", + where, wanted, module, wanted, orNoListens(listens)) + } + written = strings.ReplaceAll(written, fmt.Sprintf("${port:%d}", wanted), + strconv.Itoa(with.machinePort(module, wanted))) + } + return written, nil +} + +// orNoListens says what would have worked, so a refusal is one edit from right. +func orNoListens(listens []Listening) string { + if len(listens) == 0 { + return "it declares no ports at all" + } + said := make([]string, 0, len(listens)) + for _, l := range listens { + said = append(said, strconv.Itoa(l.Port)) + } + return "it declares " + strings.Join(said, ", ") +} diff --git a/internal/catalogue/port_into_files.go b/internal/catalogue/port_into_files.go deleted file mode 100644 index f694cd9..0000000 --- a/internal/catalogue/port_into_files.go +++ /dev/null @@ -1,87 +0,0 @@ -package catalogue - -import ( - "fmt" - "regexp" - "strconv" - "strings" -) - -// Telling a module which port it was given. -// -// **The mesh assigns the machine-side port and a module does not choose one** -// ([ADR 0038](../../02-DECISIONS/0038-the-mesh-assigns-the-port.md)). For a container that is -// invisible: the mesh rewrites `ports` into `assigned:wanted`, the software inside binds the number -// it has always bound, and the machine publishes a different one. -// -// **A process has no such layer.** It runs on the machine, there is nothing to rewrite, and it -// binds whatever its configuration says — so without this, every process binds the number written -// in its own config, two modules declaring the same one collide, and the mesh's whole reason for -// assigning ports is defeated by the resource kind that most needs it. -// -// So a module asks. `${port:8080}` is "the machine-side port you gave me for the 8080 I said I -// listen on", and the module writes that into its own configuration exactly as it writes an -// address it was bound to. - -// ofPort is where a module asks which port it was given: ${port:}. -var ofPort = regexp.MustCompile(`\$\{port:([0-9]+)\}`) - -// portsUsed are the ports a file's content asks about, first appearance first. -func portsUsed(content string) []int { - var used []int - seen := map[int]bool{} - for _, m := range ofPort.FindAllStringSubmatch(content, -1) { - n, err := strconv.Atoi(m[1]) - if err != nil || seen[n] { - continue - } - seen[n] = true - used = append(used, n) - } - return used -} - -// portInto replaces a file's ${port:…} placeholders with what this machine assigned. -// -// A port the module did not say it listens on is refused, for the same reason a binding's unknown -// key is: the module is asking about something it never declared, and the answer would be a guess. -// Left alone, the literal would be written into a configuration file and read as a port number. -func portInto(resource map[string]any, module string, listens []Listening, with Rendering) error { - if fmt.Sprint(resource["type"]) != "file" { - return nil - } - content, ok := resource["content"].(string) - if !ok { - return nil - } - for _, wanted := range portsUsed(content) { - var declared bool - for _, l := range listens { - if l.Port == wanted { - declared = true - } - } - if !declared { - return fmt.Errorf( - "%s has a file that says ${port:%d}, and %s does not say it listens on %d. A "+ - "module is told the port it was given for something it declared, and %s", - module, wanted, module, wanted, orNoListens(listens)) - } - content = strings.ReplaceAll(content, fmt.Sprintf("${port:%d}", wanted), - strconv.Itoa(with.machinePort(module, wanted))) - resource["content"] = content - } - return nil -} - -// orNoListens says what would have worked, so a refusal is one edit from right. -func orNoListens(listens []Listening) string { - if len(listens) == 0 { - return "it declares no ports at all" - } - said := make([]string, 0, len(listens)) - for _, l := range listens { - said = append(said, strconv.Itoa(l.Port)) - } - return "it declares " + strings.Join(said, ", ") -} diff --git a/internal/catalogue/port_into_files_test.go b/internal/catalogue/port_into_files_test.go deleted file mode 100644 index e8bcba7..0000000 --- a/internal/catalogue/port_into_files_test.go +++ /dev/null @@ -1,66 +0,0 @@ -package catalogue - -import ( - "strings" - "testing" -) - -// **A process binds the port the mesh gave it, not the one it wrote down.** -// -// A container never needed this: the mesh rewrites its `ports` into assigned:wanted, so the -// software binds the number it always bound and the machine publishes another. A process runs on -// the machine with nothing to rewrite, so without a way to ask, every process binds the number in -// its own configuration and two modules declaring the same one collide — which is the whole -// problem ADR 0038 exists to prevent, reintroduced by the resource kind that most needs it. -func TestAModuleIsToldWhichPortItWasGiven(t *testing.T) { - file := map[string]any{ - "type": "file", "id": "settings", - "content": "LISTEN=${port:8080}\n", - } - listens := []Listening{{Port: 8080, From: FromMesh}} - with := Rendering{Ports: map[string]map[int]int{"showcase": {8080: 21000}}} - - if err := portInto(file, "showcase", listens, with); err != nil { - t.Fatal(err) - } - if got := file["content"].(string); got != "LISTEN=21000\n" { - t.Fatalf("the module was not told its assigned port: %q", got) - } -} - -// With nothing assigned yet, it is told the port it asked about — so a mesh that has not made an -// assignment still composes something coherent rather than writing a zero. -func TestWithNoAssignmentAModuleIsToldWhatItAskedFor(t *testing.T) { - file := map[string]any{"type": "file", "content": "LISTEN=${port:8080}\n"} - if err := portInto(file, "showcase", []Listening{{Port: 8080}}, Rendering{}); err != nil { - t.Fatal(err) - } - if got := file["content"].(string); got != "LISTEN=8080\n" { - t.Fatalf("an unassigned port did not fall back to what was declared: %q", got) - } -} - -// **Asking about a port it never declared is refused**, and the refusal says what it did declare. -// The module is asking about something the mesh has no opinion on, and answering would be a guess -// written into a configuration file as a port number. -func TestAskingAboutAnUndeclaredPortIsRefused(t *testing.T) { - file := map[string]any{"type": "file", "content": "LISTEN=${port:9999}\n"} - err := portInto(file, "showcase", []Listening{{Port: 8080}}, Rendering{}) - if err == nil { - t.Fatal("a module was told a port it never said it listens on") - } - if !strings.Contains(err.Error(), "8080") { - t.Fatalf("the refusal does not say what would have worked: %v", err) - } -} - -// And a file mentioning no port is left exactly as it was. -func TestAFileWithNoPortIsUntouched(t *testing.T) { - file := map[string]any{"type": "file", "content": "GREETING=hello\n"} - if err := portInto(file, "showcase", nil, Rendering{}); err != nil { - t.Fatal(err) - } - if got := file["content"].(string); got != "GREETING=hello\n" { - t.Fatalf("a file with no port was changed: %q", got) - } -} diff --git a/internal/catalogue/port_into_test.go b/internal/catalogue/port_into_test.go new file mode 100644 index 0000000..1e13cb5 --- /dev/null +++ b/internal/catalogue/port_into_test.go @@ -0,0 +1,170 @@ +package catalogue + +import ( + "reflect" + "strconv" + "strings" + "testing" +) + +// **A process binds the port the mesh gave it, not the one it wrote down.** +// +// A container never needed this: the mesh rewrites its `ports` into assigned:wanted, so the +// software binds the number it always bound and the machine publishes another. A process runs on +// the machine with nothing to rewrite, so without a way to ask, every process binds the number in +// its own configuration and two modules declaring the same one collide — which is the whole +// problem ADR 0038 exists to prevent, reintroduced by the resource kind that most needs it. +func TestAModuleIsToldWhichPortItWasGiven(t *testing.T) { + file := map[string]any{ + "type": "file", "id": "settings", + "content": "LISTEN=${port:8080}\n", + } + listens := []Listening{{Port: 8080, From: FromMesh}} + with := Rendering{Ports: map[string]map[int]int{"showcase": {8080: 21000}}} + + if err := portInto(file, "showcase", listens, with); err != nil { + t.Fatal(err) + } + if got := file["content"].(string); got != "LISTEN=21000\n" { + t.Fatalf("the module was not told its assigned port: %q", got) + } +} + +// With nothing assigned yet, it is told the port it asked about — so a mesh that has not made an +// assignment still composes something coherent rather than writing a zero. +func TestWithNoAssignmentAModuleIsToldWhatItAskedFor(t *testing.T) { + file := map[string]any{"type": "file", "content": "LISTEN=${port:8080}\n"} + if err := portInto(file, "showcase", []Listening{{Port: 8080}}, Rendering{}); err != nil { + t.Fatal(err) + } + if got := file["content"].(string); got != "LISTEN=8080\n" { + t.Fatalf("an unassigned port did not fall back to what was declared: %q", got) + } +} + +// **Asking about a port it never declared is refused**, and the refusal says what it did declare. +// The module is asking about something the mesh has no opinion on, and answering would be a guess +// written into a configuration file as a port number. +func TestAskingAboutAnUndeclaredPortIsRefused(t *testing.T) { + file := map[string]any{"type": "file", "content": "LISTEN=${port:9999}\n"} + err := portInto(file, "showcase", []Listening{{Port: 8080}}, Rendering{}) + if err == nil { + t.Fatal("a module was told a port it never said it listens on") + } + if !strings.Contains(err.Error(), "8080") { + t.Fatalf("the refusal does not say what would have worked: %v", err) + } +} + +// And a file mentioning no port is left exactly as it was. +func TestAFileWithNoPortIsUntouched(t *testing.T) { + file := map[string]any{"type": "file", "content": "GREETING=hello\n"} + if err := portInto(file, "showcase", nil, Rendering{}); err != nil { + t.Fatal(err) + } + if got := file["content"].(string); got != "GREETING=hello\n" { + t.Fatalf("a file with no port was changed: %q", got) + } +} + +// **A container that dials the machine is told the same thing, in its environment.** +// +// The forge's own sidecar reaches the forge over the machine's loopback (novox/hq 04-ISSUES/088). +// The `assigned:wanted` mapping that lets the forge itself go on binding 3000 does nothing for the +// caller: the caller dials the machine, so it must be given the machine's number — here one the +// node was given rather than one the mesh assigned, which is the case that makes the literal wrong +// even on a mesh that never allocates (ADR 0100). +func TestAContainerIsToldWhichPortItWasGiven(t *testing.T) { + sidecar := map[string]any{ + "type": "container", "id": "runtime", "name": "mesh-gitea", + "env": map[string]any{ + "MESH_GITEA_URL": "http://127.0.0.1:${port:3000}", + "MESH_ADMIN_USER": "mesh-admin", + }, + } + listens := []Listening{{Port: 3000, From: FromMesh}} + with := Rendering{Given: map[string]map[int]int{"gitea": {3000: 2999}}} + + if err := portInto(sidecar, "gitea", listens, with); err != nil { + t.Fatal(err) + } + env := sidecar["env"].(map[string]any) + if env["MESH_GITEA_URL"] != "http://127.0.0.1:2999" { + t.Fatalf("the sidecar dials %v, not the port this machine puts the forge on", + env["MESH_GITEA_URL"]) + } + if env["MESH_ADMIN_USER"] != "mesh-admin" { + t.Fatalf("filling one value changed another: %v", env) + } +} + +// **And the manifest it came out of is left alone.** +// +// An `env` map is the catalogue's, shared by every node running the module, and the resource +// around it is a shallow copy. Filled in place, the first node composed would write its own port +// into the catalogue and every node composed after it would be told that one — a fault that is +// invisible in a single composition and wrong in every mesh with two machines. +func TestFillingAContainersEnvironmentLeavesTheManifestAlone(t *testing.T) { + env := map[string]any{"MESH_GITEA_URL": "http://127.0.0.1:${port:3000}"} + manifest := map[string]any{"type": "container", "id": "runtime", "name": "mesh-gitea", "env": env} + + for _, at := range []int{2999, 3100} { + copied := map[string]any{} + for k, v := range manifest { + copied[k] = v + } + with := Rendering{Given: map[string]map[int]int{"gitea": {3000: at}}} + if err := portInto(copied, "gitea", []Listening{{Port: 3000}}, with); err != nil { + t.Fatal(err) + } + want := "http://127.0.0.1:" + strconv.Itoa(at) + if got := copied["env"].(map[string]any)["MESH_GITEA_URL"]; got != want { + t.Fatalf("the second node was told %v, not %v — the first composition wrote its own "+ + "port into the catalogue", got, want) + } + } + if env["MESH_GITEA_URL"] != "http://127.0.0.1:${port:3000}" { + t.Fatalf("the module's own manifest was edited: %v", env) + } +} + +// Asking in an environment about a port it never declared is refused exactly as a file's is, and +// the refusal names the container and the variable — there is no line number to go on. +func TestAContainerAskingAboutAnUndeclaredPortIsRefused(t *testing.T) { + sidecar := map[string]any{ + "type": "container", "id": "runtime", "name": "mesh-gitea", + "env": map[string]any{"MESH_GITEA_URL": "http://127.0.0.1:${port:9999}"}, + } + err := portInto(sidecar, "gitea", []Listening{{Port: 3000}}, Rendering{}) + if err == nil { + t.Fatal("a container was told a port its module never said it listens on") + } + for _, said := range []string{"mesh-gitea", "MESH_GITEA_URL", "3000"} { + if !strings.Contains(err.Error(), said) { + t.Errorf("the refusal does not say %q: %v", said, err) + } + } +} + +// And a container naming no port keeps the environment it was written with — the same map, so +// nothing is copied and no other node's composition is disturbed. +func TestAContainerWithNoPortPlaceholderKeepsItsEnvironment(t *testing.T) { + env := map[string]any{"MESH_GITEA_URL": "http://gitea:3000", "PORT": 3000} + sidecar := map[string]any{"type": "container", "name": "mesh-gitea", "env": env} + if err := portInto(sidecar, "gitea", []Listening{{Port: 3000}}, Rendering{ + Given: map[string]map[int]int{"gitea": {3000: 2999}}, + }); err != nil { + t.Fatal(err) + } + // A sibling reached over the runtime's own network is reached on the port INSIDE it, which the + // module does control: a literal there is right, and moving it would break the one address the + // mapping does not touch. + if got := sidecar["env"].(map[string]any)["MESH_GITEA_URL"]; got != "http://gitea:3000" { + t.Fatalf("an address on the runtime's own network was moved to the machine's port: %v", got) + } + // The same map, not a copy of it: a container that asks for nothing is left alone, and + // comparing the contents would say yes even to a rebuilt map. + if reflect.ValueOf(sidecar["env"]).Pointer() != reflect.ValueOf(env).Pointer() { + t.Fatalf("a container that asked for nothing had its environment rebuilt: %v", sidecar["env"]) + } +}