Merge pull request 'Serve postgres's consumers the port its server is published on, not the container's 5432 (issue 957)' (#339) from fix/957-postgres-serves-published-port into main

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()

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 {

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 },

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)
+		}
+	}
+}