diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index dd66ac9..f6198f7 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -1102,6 +1102,7 @@ func (r Resolution) contributions(settings SettingsBy, grants []Grant, if err != nil { return nil, fmt.Errorf("%s contributing to %s: %w", m.Module, to, err) } + portOfEndpoint(values, endpointPorts(m)) composeName(values, r.PublicDomain, r.At, reaches, endpointPorts(m), blocks) out[to] = append(out[to], Contribution{From: m.Module, Values: values}) } @@ -1124,6 +1125,7 @@ func (r Resolution) contributions(settings SettingsBy, grants []Grant, if err != nil { return nil, fmt.Errorf("%s contributing %s to %s: %w", m.Module, local, to, err) } + portOfEndpoint(values, endpointPorts(m)) composeName(values, r.PublicDomain, r.At, reaches, endpointPorts(m), blocks) out[to] = append(out[to], Contribution{From: m.Module, Values: values}) } @@ -1837,3 +1839,32 @@ func endpointPorts(m Manifest) map[string]int { } return out } + +// portOfEndpoint fills in the port of the endpoint a contribution names, in place. +// +// **A contribution that names an endpoint must still carry that endpoint's port**, because everything +// downstream reads the port: the provider is told where to reach the consumer, and the machine-side +// redirection that turns a declared port into the number the machine published is keyed on it +// (atMachinePort). A route that named only its endpoint left the proxy with no port at all, and a +// proxy with no port has nothing to dial. +// +// Found before it shipped and after the catalogue had already been changed to name endpoints — the +// manifests were merged and the mesh had not yet picked them up, so nothing was broken yet. The +// declared port, not the machine one: the redirection happens later and is keyed on the declared +// number, so filling in the machine port here would be redirected a second time or not at all. +func portOfEndpoint(values map[string]any, ports map[string]int) { + if values == nil { + return + } + if _, already := values["port"]; already { + // A route that says both is its own answer; the older shape repeated the port and is still read. + return + } + name, ok := values[RouteEndpoint].(string) + if !ok { + return + } + if port, found := ports[strings.TrimSpace(name)]; found { + values["port"] = port + } +} diff --git a/internal/catalogue/endpoint_test.go b/internal/catalogue/endpoint_test.go index 38352a1..d05f040 100644 --- a/internal/catalogue/endpoint_test.go +++ b/internal/catalogue/endpoint_test.go @@ -145,3 +145,61 @@ func TestAnUnnamedEndpointIsStillValid(t *testing.T) { t.Fatalf("a module with no route was refused: %v", got) } } + +// **A route that names an endpoint still carries that endpoint's port.** +// +// Everything downstream reads the port: the provider is told where to reach the consumer, and the +// redirection that turns a declared port into the number the machine published is keyed on it. A route +// naming only its endpoint left the proxy with no port, and a proxy with no port has nothing to dial. +// +// Caught after the catalogue had already been changed to name endpoints, and before the mesh picked +// those manifests up — which is the only reason nothing broke. +func TestARouteNamingAnEndpointStillCarriesItsPort(t *testing.T) { + m := aMediaServer() + r := Resolution{Node: "anchor", Modules: []Manifest{m}, + PublicDomain: "example.test", At: "anchor.internal"} + given, err := r.contributions(nil, nil, nil) + if err != nil { + t.Fatal(err) + } + var saw bool + for _, c := range given["route"] { + saw = true + port, ok := asPort(c.Values["port"]) + if !ok { + t.Fatalf("the route carries no port, so the proxy has nothing to dial: %v", c.Values) + } + if port != 80 { + t.Fatalf("the route carries port %d, want the web endpoint's 80", port) + } + } + if !saw { + t.Fatal("the module contributed no route") + } +} + +// And the declared port, not the machine one: the redirection to where the machine published it +// happens later and is keyed on the declared number, so filling the machine port in here would be +// redirected twice or not at all. +func TestTheEndpointsDeclaredPortIsFilledInNotTheMachineOne(t *testing.T) { + m := aMediaServer() + values := map[string]any{RouteEndpoint: "web", "label": "media"} + portOfEndpoint(values, endpointPorts(m)) + if got, _ := asPort(values["port"]); got != 80 { + t.Fatalf("filled in port %d, want the declared 80", got) + } + // Then the ordinary redirection puts it where the machine published it. + moved := atMachinePort(values, m.Module, map[string]map[int]int{"media": {80: 20009}}) + if got, _ := asPort(moved["port"]); got != 20009 { + t.Fatalf("after redirection the port is %d, want the machine's 20009", got) + } +} + +// A route that repeats a port keeps it, because that is the older shape and still read. +func TestARouteThatRepeatsItsPortKeepsIt(t *testing.T) { + values := map[string]any{RouteEndpoint: "web", "port": 8080} + portOfEndpoint(values, map[string]int{"web": 80}) + if got, _ := asPort(values["port"]); got != 8080 { + t.Fatalf("the port it stated was overwritten with %d", got) + } +}