diff --git a/internal/agent/deploy.go b/internal/agent/deploy.go index 1e08cee..87e3478 100644 --- a/internal/agent/deploy.go +++ b/internal/agent/deploy.go @@ -198,7 +198,10 @@ func (a *Agent) allocateRoutePorts(service, component string, routes []registry. return nil, nil, fmt.Errorf("store host port for route %q: %w", r.Name, err) } - ports = append(ports, fmt.Sprintf("127.0.0.1:%d:%d", hostPort, r.Port)) + // The container port must match hostPort (which is also set as $PORT), + // so the app's listen address matches the podman port mapping. + // r.Port is the mc-proxy listener port, NOT the container port. + ports = append(ports, fmt.Sprintf("127.0.0.1:%d:%d", hostPort, hostPort)) if len(routes) == 1 { env = append(env, fmt.Sprintf("PORT=%d", hostPort)) diff --git a/internal/agent/deploy_test.go b/internal/agent/deploy_test.go new file mode 100644 index 0000000..946a208 --- /dev/null +++ b/internal/agent/deploy_test.go @@ -0,0 +1,159 @@ +package agent + +import ( + "database/sql" + "fmt" + "log/slog" + "os" + "path/filepath" + "testing" + + "git.wntrmute.dev/mc/mcp/internal/registry" +) + +func openTestDB(t *testing.T) *sql.DB { + t.Helper() + db, err := registry.Open(filepath.Join(t.TempDir(), "test.db")) + if err != nil { + t.Fatalf("open db: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + return db +} + +func testAgent(t *testing.T) *Agent { + t.Helper() + return &Agent{ + DB: openTestDB(t), + PortAlloc: NewPortAllocator(), + Logger: slog.New(slog.NewTextHandler(os.Stderr, nil)), + } +} + +// seedComponent creates the service and component in the registry so that +// allocateRoutePorts can store host ports for it. +func seedComponent(t *testing.T, db *sql.DB, service, component string, routes []registry.Route) { + t.Helper() + if err := registry.CreateService(db, service, true); err != nil { + t.Fatalf("create service: %v", err) + } + if err := registry.CreateComponent(db, ®istry.Component{ + Name: component, + Service: service, + Image: "img:latest", + DesiredState: "running", + ObservedState: "unknown", + Routes: routes, + }); err != nil { + t.Fatalf("create component: %v", err) + } +} + +func TestAllocateRoutePorts_SingleRoute(t *testing.T) { + a := testAgent(t) + routes := []registry.Route{ + {Name: "default", Port: 443, Mode: "l7"}, + } + seedComponent(t, a.DB, "mcdoc", "mcdoc", routes) + + ports, env, err := a.allocateRoutePorts("mcdoc", "mcdoc", routes) + if err != nil { + t.Fatalf("allocateRoutePorts: %v", err) + } + + if len(ports) != 1 { + t.Fatalf("expected 1 port mapping, got %d", len(ports)) + } + if len(env) != 1 { + t.Fatalf("expected 1 env var, got %d", len(env)) + } + + // Parse the port mapping: should be "127.0.0.1::" + // NOT "127.0.0.1::443" + var hostPort, containerPort int + n, _ := fmt.Sscanf(ports[0], "127.0.0.1:%d:%d", &hostPort, &containerPort) + if n != 2 { + t.Fatalf("failed to parse port mapping %q", ports[0]) + } + if hostPort != containerPort { + t.Errorf("host port (%d) != container port (%d); container port must match host port for $PORT consistency", hostPort, containerPort) + } + + // Env var should be PORT= + var envPort int + n, _ = fmt.Sscanf(env[0], "PORT=%d", &envPort) + if n != 1 { + t.Fatalf("failed to parse env var %q", env[0]) + } + if envPort != hostPort { + t.Errorf("PORT env (%d) != host port (%d)", envPort, hostPort) + } +} + +func TestAllocateRoutePorts_MultiRoute(t *testing.T) { + a := testAgent(t) + routes := []registry.Route{ + {Name: "rest", Port: 8443, Mode: "l4"}, + {Name: "grpc", Port: 9443, Mode: "l4"}, + } + seedComponent(t, a.DB, "metacrypt", "api", routes) + + ports, env, err := a.allocateRoutePorts("metacrypt", "api", routes) + if err != nil { + t.Fatalf("allocateRoutePorts: %v", err) + } + + if len(ports) != 2 { + t.Fatalf("expected 2 port mappings, got %d", len(ports)) + } + if len(env) != 2 { + t.Fatalf("expected 2 env vars, got %d", len(env)) + } + + // Each port mapping should have host port == container port. + for i, p := range ports { + var hp, cp int + n, _ := fmt.Sscanf(p, "127.0.0.1:%d:%d", &hp, &cp) + if n != 2 { + t.Fatalf("port[%d]: failed to parse %q", i, p) + } + if hp != cp { + t.Errorf("port[%d]: host port (%d) != container port (%d)", i, hp, cp) + } + } + + // Env vars should be PORT_REST and PORT_GRPC (not bare PORT). + if env[0][:10] != "PORT_REST=" { + t.Errorf("env[0] = %q, want PORT_REST=...", env[0]) + } + if env[1][:10] != "PORT_GRPC=" { + t.Errorf("env[1] = %q, want PORT_GRPC=...", env[1]) + } +} + +func TestAllocateRoutePorts_L7PortNotUsedAsContainerPort(t *testing.T) { + a := testAgent(t) + routes := []registry.Route{ + {Name: "default", Port: 443, Mode: "l7"}, + } + seedComponent(t, a.DB, "svc", "web", routes) + + ports, _, err := a.allocateRoutePorts("svc", "web", routes) + if err != nil { + t.Fatalf("allocateRoutePorts: %v", err) + } + + // The container port must NOT be 443 (the mc-proxy listener port). + // It must be the host port (which is in range 10000-60000). + var hostPort, containerPort int + n, _ := fmt.Sscanf(ports[0], "127.0.0.1:%d:%d", &hostPort, &containerPort) + if n != 2 { + t.Fatalf("failed to parse port mapping %q", ports[0]) + } + if containerPort == 443 { + t.Errorf("container port is 443 (mc-proxy listener); should be %d (host port)", hostPort) + } + if containerPort < portRangeMin || containerPort >= portRangeMax { + t.Errorf("container port %d outside allocation range [%d, %d)", containerPort, portRangeMin, portRangeMax) + } +} diff --git a/internal/runtime/runtime_test.go b/internal/runtime/runtime_test.go index 89dbbed..153ecf1 100644 --- a/internal/runtime/runtime_test.go +++ b/internal/runtime/runtime_test.go @@ -90,18 +90,19 @@ func TestBuildRunArgs(t *testing.T) { }) t.Run("full spec with env", func(t *testing.T) { + // Route-allocated ports: host port = container port (matches $PORT). spec := ContainerSpec{ Name: "svc-api", Image: "img:latest", Network: "net", - Ports: []string{"127.0.0.1:12345:8443"}, + Ports: []string{"127.0.0.1:12345:12345"}, Volumes: []string{"/srv:/srv"}, Env: []string{"PORT=12345"}, } requireEqualArgs(t, p.BuildRunArgs(spec), []string{ "run", "-d", "--name", "svc-api", "--network", "net", - "-p", "127.0.0.1:12345:8443", + "-p", "127.0.0.1:12345:12345", "-v", "/srv:/srv", "-e", "PORT=12345", "img:latest", diff --git a/proto/mcp/v1/mcp.proto b/proto/mcp/v1/mcp.proto index c137216..b0a9f7f 100644 --- a/proto/mcp/v1/mcp.proto +++ b/proto/mcp/v1/mcp.proto @@ -38,7 +38,7 @@ service McpAgentService { message RouteSpec { string name = 1; // route name (used for $PORT_) - int32 port = 2; // external port on mc-proxy + int32 port = 2; // mc-proxy listener port (e.g. 443, 8443, 9443); NOT the container internal port string mode = 3; // "l4" or "l7" string hostname = 4; // optional public hostname override }