diff --git a/internal/catalogue/bound_into_files_test.go b/internal/catalogue/bound_into_files_test.go index 278cc6c..011a1ce 100644 --- a/internal/catalogue/bound_into_files_test.go +++ b/internal/catalogue/bound_into_files_test.go @@ -1,6 +1,7 @@ package catalogue import ( + "fmt" "strings" "testing" ) @@ -53,6 +54,69 @@ func TestAConsumerCanWriteAConnectionString(t *testing.T) { } } +// A provider answered on its consumer's own machine is announced at the port the machine PUBLISHES +// it on, not the port the module declared (novox/hq 04-ISSUES/038). +// +// Issue 018 made the same-node provider announced at all; this is the promise after it — that the +// port a co-located consumer is told is the one actually listening at the address it is handed. The +// mesh assigns a bare `ports` mapping a host port (here the container's 5432 becomes 15432 on this +// machine), and it must announce 15432, or the consumer dials workstation.internal:5432 where the +// provider is not published. Cross-node consumers already got this (the plan re-derives the served +// facts with the provider's assignment); the same-node need was settled before the assignment and +// carried the declared port until now. +func TestASameNodeProviderIsAnnouncedAtThePortItIsPublishedOn(t *testing.T) { + provider := Manifest{ + Module: "postgres", Version: "1", + Provides: FromAnywhere("postgres-database"), + Listens: []Listening{{Port: 5432, Protocol: "tcp", From: "mesh"}}, + Serves: map[string]map[string]any{"postgres-database": {"port": 5432}}, + Resources: []map[string]any{{ + "id": "server", "type": "container", "name": "postgres", + "ports": []any{"5432"}, + }}, + } + consumer := Manifest{ + Module: "keycloak", Version: "1", Requires: []string{"postgres-database"}, + Resources: []map[string]any{{ + "id": "dbenv", "type": "file", "path": "/var/lib/keycloak/database.env", "mode": "0600", + "content": "KC_DB_URL=jdbc:postgresql://${bound:postgres-database:at}:" + + "${bound:postgres-database:port}/keycloak\n", + }}, + } + r, err := Resolve(shelf(provider, consumer), + []string{"postgres", "keycloak"}, reachable(), World{}) + if err != nil { + t.Fatal(err) + } + // The mesh assigned the container's 5432 to 15432 on this machine. + out, err := r.Declaration(Rendering{Ports: map[string]map[int]int{"postgres": {5432: 15432}}}) + if err != nil { + t.Fatal(err) + } + + // Where the provider is actually published: the assigned host port, on the machine at large. + server := fileNamed(out, "postgres.server") + if server == nil { + t.Fatalf("the provider container is not in the declaration: %v", out) + } + if published := fmt.Sprint(server["ports"]); !strings.Contains(published, "15432:5432") { + t.Fatalf("the provider was not published on the assigned port: %v", server["ports"]) + } + + // What the consumer was told to dial: the SAME port, at the announced address — not the 5432 the + // module declared, which is closed on that address. + dbenv := fileNamed(out, "keycloak.dbenv") + if dbenv == nil { + t.Fatalf("the consumer's binding file is not in the declaration: %v", out) + } + got := dbenv["content"].(string) + want := "KC_DB_URL=jdbc:postgresql://workstation.internal:15432/keycloak\n" + if got != want { + t.Errorf("the consumer dials a port the provider does not listen on:\n got %q\nwant %q", + got, want) + } +} + // A port is 5432, not 5432.000000 — which is what a number decoded from JSON would write, and what // every connection string in the world would refuse. func TestAPortIsWrittenAsAPort(t *testing.T) { diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 470c8ba..bbebd42 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -141,6 +141,21 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { return nil, err } + // A provider answered on this same machine is announced at the port it *declared*, because the + // need was settled while resolving — before this machine assigned the port a bare `ports` + // mapping is published on (novox/hq 04-ISSUES/038). Redirect those same-node needs to where the + // machine actually publishes the provider, exactly as the cross-node path already does with the + // provider's own assignment, so a co-located consumer is told the port that is really listening. + servedBy := servedByModule(r.Modules) + for i := range r.Needs { + if r.Needs[i].From != r.Node { + continue + } + if module, ok := servedBy[r.Needs[i].Name]; ok { + r.Needs[i].Serves = atMachinePort(r.Needs[i].Serves, module, with.Ports) + } + } + // Once, from every module's listens -- not per module. A module receiving only its own ports // would write a rule set that closed every other module on the machine. Each module's per-node // exposure settings override its listens' source first (novox/hq ADR 0046). @@ -311,6 +326,11 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { // requirement met by the module itself. continue } + // Answered here, so its served port needs the same redirect the resolver's + // same-node needs got above — this fallback builds one afresh, outside r.Needs. + if provider, ok := servedBy[to]; ok { + here.Serves = atMachinePort(here.Serves, provider, with.Ports) + } found = here } file, err := boundFile(*found, m.Binds[to], ConsumerIdentity(r.Node, IdentitySource(m.Slug, m.Module))) @@ -838,3 +858,49 @@ func asPort(v any) (int, bool) { } return 0, false } + +// servedByModule maps each provision offered on this node to the module that offers it. +// +// The same node may both provide and consume a provision, and a consumer's binding names the +// provision, not the module behind it — so redirecting a served port to where the machine put it +// needs this reverse lookup, from the provision back to the module whose assignment moved it. +func servedByModule(mods []Manifest) map[string]string { + out := map[string]string{} + for _, m := range mods { + for provision := range m.Serves { + out[provision] = m.Module + } + } + return out +} + +// atMachinePort redirects a served `port` fact to where this machine actually published the +// provider (novox/hq 04-ISSUES/038). +// +// **Same-node served facts are settled before the machine's port is assigned.** The resolver names +// what a consumer must know (resolve.go's servedHere, declaration.go's here()) while resolving, +// which happens before a bare `ports` declaration is assigned a host port — so it can only carry +// the port the provider *declared*. The cross-node path re-derives the fact after assignment, with +// the provider node's port map (cmd plan.go), and so already tells the truth. This closes the gap +// for the co-located case: without it a consumer is announced `.internal:` while +// the provider is published on `.internal:`, and dials a port nothing listens on. +// +// A fresh map when it changes anything, so the manifest-derived value the resolver handed back is +// never mutated under a caller that still holds it. Keyed by the *declared* port, so applying it a +// second time is a no-op: once redirected the value is the machine port, which is not itself a key. +func atMachinePort(serves map[string]any, module string, ports map[string]map[int]int) map[string]any { + number, ok := asPort(serves["port"]) + if !ok { + return serves + } + at, known := ports[module][number] + if !known || at == number { + return serves + } + copied := make(map[string]any, len(serves)) + for k, v := range serves { + copied[k] = v + } + copied["port"] = at + return copied +}