diff --git a/modules/postgres/cmd/postgres-provider/client.go b/modules/postgres/cmd/postgres-provider/client.go index 895e57eb..20fc4611 100644 --- a/modules/postgres/cmd/postgres-provider/client.go +++ b/modules/postgres/cmd/postgres-provider/client.go @@ -122,6 +122,38 @@ func NewClient(conn Conn, dial Dialer) *Client { var unfilled = regexp.MustCompile(`^\$\{[^}]*\}$`) +// portFromEnv is the port postgres's server is published on, on its machine: MESH_POSTGRES_PORT, else +// MESH_PROVISION_POSTGRES_PORT, the seat's twin (mesh-controller's internal/envfile.Placed pattern: which port +// this machine actually put mesh-store at, when that differs from the connection string's; empty, or a +// placeholder the mesh never filled, adds nothing), else the connection string's port, else 5432. It is both +// the port this code reaches the server on and the port its consumers are served (issue 957). +func portFromEnv(env func(string) string, u *url.URL) int { + seatPort := strings.TrimSpace(env("MESH_PROVISION_POSTGRES_PORT")) + if unfilled.MatchString(seatPort) { + seatPort = "" + } + portSource := firstOf(env("MESH_POSTGRES_PORT"), seatPort) + if portSource == "" && u != nil { + portSource = u.Port() + } + port, err := strconv.Atoi(portSource) + if err != nil || port == 0 { + return 5432 + } + return port +} + +// publishedPort is portFromEnv read from the environment alone, its connection string parsed here. +func publishedPort(env func(string) string) int { + var u *url.URL + if raw := env("MESH_PROVISION_POSTGRES"); raw != "" { + if parsed, err := url.Parse(raw); err == nil { + u = parsed + } + } + return portFromEnv(env, u) +} + // ClientFromEnv builds the client from the module's words. MESH_POSTGRES_* first (the documented // names), then the MESH_PROVISION_* keys the manifest sets. Fails without a host and an admin password. func ClientFromEnv(env func(string) string) (*Client, error) { @@ -135,21 +167,7 @@ func ClientFromEnv(env func(string) string) (*Client, error) { if host == "" && u != nil { host = u.Hostname() } - // MESH_PROVISION_POSTGRES_PORT is the seat's twin (mesh-controller's internal/envfile.Placed - // pattern): which port this machine actually put mesh-store at, when that differs from the - // connection string's. Empty, or a placeholder the mesh never filled, adds nothing. - seatPort := strings.TrimSpace(env("MESH_PROVISION_POSTGRES_PORT")) - if unfilled.MatchString(seatPort) { - seatPort = "" - } - portSource := firstOf(env("MESH_POSTGRES_PORT"), seatPort) - if portSource == "" && u != nil { - portSource = u.Port() - } - port, err := strconv.Atoi(portSource) - if err != nil || port == 0 { - port = 5432 - } + port := portFromEnv(env, u) user := env("MESH_POSTGRES_USER") if user == "" && u != nil && u.User != nil { user = u.User.Username() diff --git a/modules/postgres/cmd/postgres-provider/main.go b/modules/postgres/cmd/postgres-provider/main.go index a1eb199e..8a532119 100644 --- a/modules/postgres/cmd/postgres-provider/main.go +++ b/modules/postgres/cmd/postgres-provider/main.go @@ -50,7 +50,7 @@ func main() { }, Node: os.Getenv("MESH_NODE"), } - run, p, bus, err := moved(consumers, h) + run, p, bus, err := moved(consumers, h, publishedPort(os.Getenv)) if err != nil { say("%v", err) } else { diff --git a/modules/postgres/cmd/postgres-provider/moved.go b/modules/postgres/cmd/postgres-provider/moved.go index a639f7b6..e9e4c392 100644 --- a/modules/postgres/cmd/postgres-provider/moved.go +++ b/modules/postgres/cmd/postgres-provider/moved.go @@ -23,13 +23,13 @@ import ( // ProvisionName is what postgres provides, as its manifest says. const ProvisionName = "postgres-database" -// servedPort is the port its manifest serves (`serves.postgres-database.port`). -const servedPort = 5432 - // moved is postgres handed to the provisioner: the SDK's moved path (harness.Moved) answering its consumers, and the // harness (never run) whose retirement and tools it shares. consumersFile is where its machine's client fills its // consumers. What differs from keycloak is only this declaration (issue 849). -func moved(consumersFile string, h *harness.Harness) (*harness.Moved, *provider.Provider, provider.Bus, error) { +// +// It serves its consumers the port its server is published on, on its machine (servedPort), never the container's +// own 5432: on a machine that publishes it elsewhere (6852:5432 on novox) nothing answers 5432 (issue 957). +func moved(consumersFile string, h *harness.Harness, servedPort int) (*harness.Moved, *provider.Provider, provider.Bus, error) { return harness.NewMoved(consumersFile, h, harness.MovedSpec{Module: "postgres", Provision: ProvisionName, Offer: provider.Offer{Credential: provider.PerConsumer, Serves: map[string]any{"port": servedPort}}, Login: func(e provider.Entry) string { return e.User }, diff --git a/modules/postgres/cmd/postgres-provider/published_port_test.go b/modules/postgres/cmd/postgres-provider/published_port_test.go new file mode 100644 index 00000000..ad726d5a --- /dev/null +++ b/modules/postgres/cmd/postgres-provider/published_port_test.go @@ -0,0 +1,28 @@ +package main + +import "testing" + +// postgres serves its consumers the port its server is published on, on its machine, as its own code reaches it: +// on novox the manifest's 5432 is published as 6852, and a consumer handed 5432 reached nothing (issue 957). +func TestPostgresServesThePortItsServerIsPublishedOn(t *testing.T) { + env := func(vars map[string]string) func(string) string { return func(k string) string { return vars[k] } } + for _, c := range []struct { + name string + vars map[string]string + want int + }{ + {"the connection string's port, filled by the machine", map[string]string{ + "MESH_PROVISION_POSTGRES": "postgres://postgres@127.0.0.1:6852/postgres?sslmode=disable"}, 6852}, + {"the seat's twin wins over the connection string", map[string]string{ + "MESH_PROVISION_POSTGRES": "postgres://postgres@127.0.0.1:6852/postgres", + "MESH_PROVISION_POSTGRES_PORT": "7001"}, 7001}, + {"an unfilled twin adds nothing", map[string]string{ + "MESH_PROVISION_POSTGRES": "postgres://postgres@127.0.0.1:6852/postgres", + "MESH_PROVISION_POSTGRES_PORT": "${port:5432}"}, 6852}, + {"nothing said: the server's own", map[string]string{}, 5432}, + } { + if got := publishedPort(env(c.vars)); got != c.want { + t.Errorf("%s: served port %d, want %d", c.name, got, c.want) + } + } +}