From 045ff924a8384214ce2fe2ef65cc1cca89e5149c Mon Sep 17 00:00:00 2001 From: Julien Gautier Date: Mon, 1 Jun 2026 13:07:08 +0200 Subject: [PATCH] fix(infra): exclude serve-static from 'dev.sh up all' (port collision with apps) (#261) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fix `./infra/local/dev.sh up all` failing on `port is already allocated` for port 4200: the `apps` profile (ADR-0030 — Angular dev-server on 4200) and the `serve-static` profile (Caddy reverse proxy for the production build, also defaulting to 4200) cannot run together. `up all` expanded to "every profile" without taking that conflict into account. After this PR, `./infra/local/dev.sh up all` brings up the comprehensive dev stack — infra + `dbtools` + `observability` + `apps` — and `serve-static` stays available via the explicit `./infra/local/dev.sh up serve-static` invocation when a developer actually wants to test a production build. ## Root cause `dev.compose.yml` publishes port 4200 in two services: - `portal-shell` (profile `apps`): `${SHELL_PORT:-4200}:4200` — Angular dev-server. - `serve-static` (profile `serve-static`): `${SERVE_STATIC_PORT:-4200}:4200` — Caddy serving the production build. `dev.sh`'s `ALL_PROFILES` array was used both as: 1. the teardown / status / logs scope (so a manually-started profile is still reachable for `down` and friends), **and** 2. the expansion target for `up all`. Conflating the two meant `up all` always tried to start `serve-static` even when `apps` was in scope — and the Docker port-publishing collision aborted the whole `up`. ## Fix Split the two concerns: ```bash # Every profile that exists — teardown / status / logs scope. ALL_PROFILES=(dbtools observability serve-static apps) # What `up all` expands to — serve-static excluded. UP_ALL_PROFILES=(dbtools observability apps) ``` `serve-static` stays accessible: - via explicit `dev.sh up serve-static` (the original way), - via `dev.sh down` / `status` / `logs` (still in `ALL_PROFILES`). Why exclude `serve-static` from `up all` rather than `apps`: - `apps` is the new "no native toolchain" dev mode (ADR-0030) — it _is_ what a comprehensive dev `up` should boot. - `serve-static` has zero value without a prior `nx build --configuration=production` (Caddy serves an empty `dist/` → 404 everywhere). Auto-starting it in `up all` would put a 404-machine in the stack by default. - The port number can stay at the Angular convention (4200) for the dev-server, which matches the devcontainer's `forwardPorts`. ## What lands | File | Change | | --- | --- | | `infra/local/dev.sh` | Split `ALL_PROFILES` (teardown scope) and `UP_ALL_PROFILES` (`up all` scope). Inline comment explains the exclusion of `serve-static`. Usage text updated. | | `infra/README.md` | Cheat-sheet row for `up all` clarifies the new behaviour. | ## Out of scope The script is **environment-agnostic** — it works identically when invoked on the local workstation or on `vm-dev`. Port publishing is on `0.0.0.0` by default, so services are reachable from a remote browser via the VM IP without any script change. The user-visible difference is only the URL host (`localhost:4200` locally vs `:4200` from the workstation against the VM). No local-vs-vm mode in the script. ## Test plan - [x] `bash -n infra/local/dev.sh` passes. - [x] `./infra/local/dev.sh help` shows the updated `up all` description. - [ ] **On vm-dev**: `./infra/local/dev.sh up all` brings up postgres / redis / otel-collector / pgweb / jaeger / apps-deps (exits 0) / portal-bff / portal-shell / portal-admin without port conflicts; `serve-static` is **not** started. - [ ] `./infra/local/dev.sh up serve-static` (explicit) still starts Caddy on 4200, provided `apps` is **not** already up. - [ ] `./infra/local/dev.sh down` and `status` still see serve-static if it was started manually (ALL_PROFILES retains it). ## Related - Surfaced by the ADR-0030 dockerised dev mode VM validation — `up all` was added to the user-facing surface in that PR; this PR finishes wiring it. - The port collision was flagged as a caveat in the ADR-0030 PR body ("don't run `apps` and `serve-static` together"). This PR turns the caveat into a script invariant instead of relying on the user to remember. --------- Co-authored-by: Julien Gautier Reviewed-on: https://git.unespace.com/julien/apf_portal/pulls/261 --- infra/README.md | 28 ++++++++++++++-------------- infra/local/dev.sh | 25 +++++++++++++++++++++---- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/infra/README.md b/infra/README.md index bedc9bb..87340bd 100644 --- a/infra/README.md +++ b/infra/README.md @@ -186,20 +186,20 @@ $EDITOR infra/local/.env Run `./infra/local/dev.sh help` for the full reference. Cheat-sheet: -| Command | Effect | -| ------------------------------------------------------------------------------- | ------------------------------------------------------------ | -| `./infra/local/dev.sh up` | Core only (postgres + redis + otel-collector) | -| `./infra/local/dev.sh up all` | Core + every profile | -| `./infra/local/dev.sh up dbtools` | Core + pgweb | -| `./infra/local/dev.sh up observability` | Core + Jaeger | -| `./infra/local/dev.sh up serve-static` | Core + Caddy serving `dist/.../browser/` per ADR-0019 | -| `./infra/local/dev.sh up apps` | Core + the three Nx dev servers in Docker (ADR-0030) | -| `./infra/local/dev.sh down` | Tear down the whole stack (every profile in scope) | -| `./infra/local/dev.sh down -v` | Tear down + wipe named volumes (incl. audit-roles bootstrap) | -| `./infra/local/dev.sh stop pgweb` | Stop one service (containers stay around) | -| `./infra/local/dev.sh status` | `docker compose ps`, with every profile visible | -| `./infra/local/dev.sh logs otel-collector` | Follow logs | -| `./infra/local/dev.sh exec postgres psql -U "$POSTGRES_USER" -d "$POSTGRES_DB"` | Run a command inside a service | +| Command | Effect | +| ------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------- | +| `./infra/local/dev.sh up` | Core only (postgres + redis + otel-collector) | +| `./infra/local/dev.sh up all` | Core + dbtools + observability + apps (full dev stack). serve-static is excluded — it would collide with apps on port 4200 | +| `./infra/local/dev.sh up dbtools` | Core + pgweb | +| `./infra/local/dev.sh up observability` | Core + Jaeger | +| `./infra/local/dev.sh up serve-static` | Core + Caddy serving `dist/.../browser/` per ADR-0019 | +| `./infra/local/dev.sh up apps` | Core + the three Nx dev servers in Docker (ADR-0030) | +| `./infra/local/dev.sh down` | Tear down the whole stack (every profile in scope) | +| `./infra/local/dev.sh down -v` | Tear down + wipe named volumes (incl. audit-roles bootstrap) | +| `./infra/local/dev.sh stop pgweb` | Stop one service (containers stay around) | +| `./infra/local/dev.sh status` | `docker compose ps`, with every profile visible | +| `./infra/local/dev.sh logs otel-collector` | Follow logs | +| `./infra/local/dev.sh exec postgres psql -U "$POSTGRES_USER" -d "$POSTGRES_DB"` | Run a command inside a service | Anything not matching one of the named verbs is passed through to `docker compose -f dev.compose.yml ...` (with every profile flagged in), so you keep the full Compose surface available — `./dev.sh config`, `./dev.sh top`, `./dev.sh inspect …`, etc. diff --git a/infra/local/dev.sh b/infra/local/dev.sh index febaa77..51a2d82 100755 --- a/infra/local/dev.sh +++ b/infra/local/dev.sh @@ -14,10 +14,23 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" COMPOSE_FILE="${SCRIPT_DIR}/dev.compose.yml" -# Profiles defined in dev.compose.yml. Keep in sync if a new profile -# is added. +# Every profile defined in dev.compose.yml, used as the teardown / +# status / logs scope so a profile started under a custom flag is +# always reachable later. Keep in sync if a new profile is added. ALL_PROFILES=(dbtools observability serve-static apps) +# What `up all` expands to — everything you want running for a full +# dev session. `serve-static` is deliberately excluded: +# - it would collide with `apps` on host port 4200 (both publish the +# SPA there by default — Caddy serves the production build, the +# `apps` profile runs the Angular dev-server); +# - without a prior `nx build --configuration=production`, Caddy +# just 404s every request, so it adds nothing to a comprehensive +# `up all` invocation. Users who actually need it run +# `./infra/local/dev.sh up serve-static` (and `down` still tears +# it down because it's in ALL_PROFILES above). +UP_ALL_PROFILES=(dbtools observability apps) + # Build "--profile p1 --profile p2 …" as separate arguments. build_all_profile_flags() { local p @@ -56,7 +69,11 @@ Commands: up [target...] Bring up the stack. Targets: (none) core only (postgres + redis + otel-collector) - all core + all profiles + all core + dbtools + observability + apps — + the full dev stack. serve-static is + EXCLUDED because it collides with apps + on port 4200; run it explicitly when + you actually want to serve a prod build. dbtools core + pgweb observability core + jaeger serve-static core + caddy (production-build reverse proxy) @@ -119,7 +136,7 @@ case "$cmd" in if [[ $# -eq 0 ]]; then dc_core up -d elif [[ "$1" == "all" ]]; then - dc_all up -d + up_with_profiles "${UP_ALL_PROFILES[@]}" else up_with_profiles "$@" fi