From a93afe6ed5ee4a8488baf229a01322344c42053b Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Tue, 22 Sep 2026 12:57:25 +0000 Subject: [PATCH 1/2] Stop a git checkout being able to change what nginx serves nginx mounted `deploy/nginx` straight out of the working tree, which makes a working directory a deployment input: check out a branch and the routes inside the running container are that branch's, one reload away from live. Found the hard way. A feature branch's `/mcp` route was sitting inside the production-serving container while that branch was checked out here -- inert only because nginx reads its configuration at start rather than continuously, so any reload for any unrelated reason would have deployed it. I noticed, returned the tree to main, and left the mechanism in place; this removes it. The container now mounts `.staged/`, which is gitignored and so untouched by any checkout, and `scripts/stage-nginx.sh` is the only thing that writes there. It runs `nginx -t` against what it is about to stage, in the same image, so an invalid configuration never reaches the path a reload reads -- verified by appending a bad directive and watching it refuse. Copying **in place** rather than replacing is load-bearing and fixes a second fault at the same time. A single-file bind mount follows the inode and `git checkout` replaces a file rather than rewriting it, which is why this container's environment config was four days stale while the directory beside it was current -- the reason a `/mcp` reload failed earlier with "host not found in upstream". `cp` truncates and writes, so the inode survives and a reload is now sufficient where a recreate used to be required. Proven both ways against the running container: an edit in the working tree is invisible to it, staging makes that same edit visible without recreating, and restoring the file cleans it. `nginx -t` passes inside the container and the site answers 200 throughout. Co-Authored-By: Claude Opus 5 --- .gitignore | 5 ++++ docker-compose.yml | 25 ++++++++++++---- scripts/stage-nginx.sh | 66 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 5 deletions(-) create mode 100755 scripts/stage-nginx.sh diff --git a/.gitignore b/.gitignore index b8f303c9..fe687891 100644 --- a/.gitignore +++ b/.gitignore @@ -89,3 +89,8 @@ tools/svg-export-harness/harness.bundle.js # repository is public. Mounted into the container and read literally; never # parsed, never committed. See docker-compose.yml. .content-node.env + +# Staged nginx configuration: written by scripts/stage-nginx.sh, mounted by the +# nginx service. Deliberately not tracked -- the point is that a checkout +# cannot change what the running container reads. +deploy/nginx/.staged/ diff --git a/docker-compose.yml b/docker-compose.yml index 0a6544bd..5d4b80ea 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -251,11 +251,26 @@ services: # deliberately so nothing else can. network_mode: host volumes: - # Which environment this host is. dev.conf here; production.conf on the - # host that must be indexed, where the blanket bot rule would be - # catastrophic rather than merely wrong. See deploy/nginx/README.md. - - ./deploy/nginx/${NGINX_ENV:-dev}.conf:/etc/nginx/conf.d/default.conf:ro - - ./deploy/nginx/common:/etc/nginx/common:ro + # The **staged** configuration, not the working tree. + # + # Mounting `deploy/nginx` directly made a working directory a deployment + # input: check out a branch and the routes inside the running container + # are that branch's, one reload away from live. That is not hypothetical + # -- a feature branch's /mcp route was found sitting in this container, + # inert only because nginx reads its configuration at start rather than + # continuously. + # + # `.staged/` is gitignored, so no checkout touches it, and + # `scripts/stage-nginx.sh` is the only thing that writes to it -- after + # running `nginx -t` against what it is about to stage, so an invalid + # configuration never reaches the path a reload would read. + # + # Which environment this host is is chosen when staging, by NGINX_ENV. + # dev on this box; production on the host that must be indexed, where the + # blanket bot rule would be catastrophic rather than merely wrong. See + # deploy/nginx/README.md. + - ./deploy/nginx/.staged/default.conf:/etc/nginx/conf.d/default.conf:ro + - ./deploy/nginx/.staged/common:/etc/nginx/common:ro # The one part that cannot live in the repository. Paths are variables so # a laptop can point somewhere else, or at an empty directory when it is # serving plain HTTP and has no certificates at all. diff --git a/scripts/stage-nginx.sh b/scripts/stage-nginx.sh new file mode 100755 index 00000000..45c50303 --- /dev/null +++ b/scripts/stage-nginx.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# +# Put the nginx configuration somewhere a `git checkout` cannot reach. +# +# nginx used to mount `deploy/nginx` straight out of the working tree. That +# makes a working directory a deployment input: check out a branch and the +# routes inside the running container are that branch's, one reload away from +# being live. It happened -- a feature branch's /mcp route was found sitting in +# the production container, inert only because nginx reads its configuration at +# start rather than continuously. +# +# So the container mounts `.staged/`, which is gitignored and therefore +# untouched by any checkout, and this is the only thing that writes to it. +# +# scripts/stage-nginx.sh # validate and stage +# scripts/stage-nginx.sh --check # validate only, change nothing +# +# Then reload: `docker compose exec nginx nginx -s reload` +# +# Files are copied **in place** rather than replaced, and that is load-bearing. +# A single-file bind mount follows the inode, and `git checkout` replaces a file +# rather than rewriting it -- which is why the environment config inside the +# running container was four days stale while the directory beside it was +# current. `cp` truncates and writes, so the inode survives and a reload is +# enough. +set -euo pipefail + +here=$(cd "$(dirname "$0")/.." && pwd) +src="$here/deploy/nginx" +dst="$src/.staged" +env_name=${NGINX_ENV:-dev} + +[ -f "$src/$env_name.conf" ] || { echo "no such environment: $env_name" >&2; exit 1; } + +# Validate the *source* before it can reach the staged copy. A configuration +# that fails here has not been staged, so a reload cannot pick it up. +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' EXIT +cp "$src/$env_name.conf" "$tmp/default.conf" +cp -r "$src/common" "$tmp/common" + +if ! docker run --rm \ + -v "$tmp/default.conf:/etc/nginx/conf.d/default.conf:ro" \ + -v "$tmp/common:/etc/nginx/common:ro" \ + -v "${LETSENCRYPT_DIR:-/etc/letsencrypt}:/etc/letsencrypt:ro" \ + -v "${CLOUDFLARE_CERT_DIR:-/etc/ssl/cloudflare}:/etc/ssl/cloudflare:ro" \ + nginx:alpine nginx -t >/dev/null 2>"$tmp/err"; then + echo "refusing to stage: $env_name.conf does not pass nginx -t" >&2 + grep -E 'emerg|error' "$tmp/err" | head -3 >&2 + exit 1 +fi + +if [ "${1:-}" = "--check" ]; then + echo "$env_name.conf is valid; nothing staged" + exit 0 +fi + +mkdir -p "$dst/common" +cp "$src/$env_name.conf" "$dst/default.conf" +# Delete-then-copy would change inodes under the running container. Refresh the +# contents of the directory instead, and remove only what has gone. +for f in "$src"/common/*; do cp "$f" "$dst/common/$(basename "$f")"; done +for f in "$dst"/common/*; do [ -f "$src/common/$(basename "$f")" ] || rm -f "$f"; done + +echo "staged $env_name.conf and common/ -> deploy/nginx/.staged" +echo "reload with: docker compose exec nginx nginx -s reload" From 98ada6d832d62b41c623aa05e4b0c4060c6202c1 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Tue, 22 Sep 2026 13:07:15 +0000 Subject: [PATCH 2/2] Bake the nginx config into an image, the way the other services do Replaces the staging directory from the previous commit with the standard answer, which is also the one this compose file already uses three times over: `render`, `content-node` and `mcp` all bake their code into an image. nginx was the odd one out, reading its configuration from whatever was checked out. What that cost: a feature branch's /mcp route was found sitting inside the production-serving container, inert only because nginx reads its configuration at start rather than continuously. The staged-directory fix worked, and was something a newcomer had to discover. An image is a thing they already understand, and "which configuration is live" becomes readable off the image rather than inferred from a working tree. Gone with it: scripts/stage-nginx.sh, the .staged directory and its gitignore entry. The validation it did is now a documented step against the built image, which exercises the certificate mounts that a build-time `nginx -t` cannot see: docker compose build nginx docker compose run --rm nginx nginx -t docker compose up -d nginx Certificates stay as runtime mounts. They are host secrets and nothing that cannot be rebuilt from this repository belongs in the image. Verified after deploying: the site answers 200, `/mcp` initialises, the Java exporter renders, `/RenderService` renders, and the container has no configuration mounts left at all. **And it caught a live regression that was not mine.** `/ContentService/data/ species/main` was answering 404 on beta: #289 added the nginx route to node and merged, but content-node was still running the image built before it -- ten endpoints where main has twelve -- so a path Java used to answer had been turned into a 404. Rebuilt and redeployed; all four node-served endpoints now answer 200 and species/main leads with Homo sapiens. That is the third time today a merge sat undeployed. The nginx half now cannot drift, because the image is built from the repository; the services behind it still can. Co-Authored-By: Claude Opus 5 --- .gitignore | 5 ---- deploy/nginx/Dockerfile | 36 ++++++++++++++++++++++ docker-compose.yml | 30 ++++++------------- scripts/stage-nginx.sh | 66 ----------------------------------------- 4 files changed, 45 insertions(+), 92 deletions(-) create mode 100644 deploy/nginx/Dockerfile delete mode 100755 scripts/stage-nginx.sh diff --git a/.gitignore b/.gitignore index fe687891..b8f303c9 100644 --- a/.gitignore +++ b/.gitignore @@ -89,8 +89,3 @@ tools/svg-export-harness/harness.bundle.js # repository is public. Mounted into the container and read literally; never # parsed, never committed. See docker-compose.yml. .content-node.env - -# Staged nginx configuration: written by scripts/stage-nginx.sh, mounted by the -# nginx service. Deliberately not tracked -- the point is that a checkout -# cannot change what the running container reads. -deploy/nginx/.staged/ diff --git a/deploy/nginx/Dockerfile b/deploy/nginx/Dockerfile new file mode 100644 index 00000000..76a56eb7 --- /dev/null +++ b/deploy/nginx/Dockerfile @@ -0,0 +1,36 @@ +# The site's nginx, with its configuration baked in. +# +# The configuration used to be bind-mounted out of the working tree, which made +# a working directory a deployment input: check out a branch and the routes +# inside the running container were that branch's, one reload away from live. +# That was not hypothetical -- a feature branch's /mcp route was found sitting +# in this container, inert only because nginx reads its configuration at start. +# +# Baked instead, which is what the other three services here already do +# (`render`, `content-node`, `mcp`). The running configuration is then a +# property of an image rather than of whatever happens to be checked out, and +# "which config is live" has an answer you can read off `docker image inspect` +# rather than infer. +# +# What is deliberately NOT baked: the certificates. Those are host secrets and +# stay as runtime mounts, so this image carries nothing that cannot be rebuilt +# from the repository. +FROM nginx:alpine + +# Which deployment this is. dev on the box that must not be indexed, production +# on the one that must -- the blanket bot rule makes that choice expensive to +# get wrong, which is why it is an explicit build argument rather than a +# default someone inherits. +ARG NGINX_ENV=dev + +COPY deploy/nginx/common /etc/nginx/common +COPY deploy/nginx/${NGINX_ENV}.conf /etc/nginx/conf.d/default.conf + +# Not `RUN nginx -t`: the configuration references certificates that are mounted +# at run time, so a build-time test would fail on every host for the wrong +# reason. Validate the built image before deploying it, which reads the same +# way and actually exercises the mounts: +# +# docker compose build nginx +# docker compose run --rm nginx nginx -t +# docker compose up -d nginx diff --git a/docker-compose.yml b/docker-compose.yml index 5d4b80ea..88d8aeb9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -243,7 +243,13 @@ services: # To go back: `docker compose stop nginx && docker start reactome-nginx`. # Both bind the host's ports, so only one can run at a time. nginx: - image: nginx:alpine + build: + context: . + dockerfile: deploy/nginx/Dockerfile + args: + # Which deployment this host is. See deploy/nginx/README.md -- getting + # this wrong on the indexed host would deindex reactome.org. + NGINX_ENV: ${NGINX_ENV:-dev} restart: unless-stopped # Host networking, as the container it replaces used: the services it # proxies -- the site on 4200, content-node on 4400, render on 4310, the @@ -251,26 +257,8 @@ services: # deliberately so nothing else can. network_mode: host volumes: - # The **staged** configuration, not the working tree. - # - # Mounting `deploy/nginx` directly made a working directory a deployment - # input: check out a branch and the routes inside the running container - # are that branch's, one reload away from live. That is not hypothetical - # -- a feature branch's /mcp route was found sitting in this container, - # inert only because nginx reads its configuration at start rather than - # continuously. - # - # `.staged/` is gitignored, so no checkout touches it, and - # `scripts/stage-nginx.sh` is the only thing that writes to it -- after - # running `nginx -t` against what it is about to stage, so an invalid - # configuration never reaches the path a reload would read. - # - # Which environment this host is is chosen when staging, by NGINX_ENV. - # dev on this box; production on the host that must be indexed, where the - # blanket bot rule would be catastrophic rather than merely wrong. See - # deploy/nginx/README.md. - - ./deploy/nginx/.staged/default.conf:/etc/nginx/conf.d/default.conf:ro - - ./deploy/nginx/.staged/common:/etc/nginx/common:ro + # Only the certificates are mounted. The configuration is baked into the + # image above, so a checkout cannot change what this serves. # The one part that cannot live in the repository. Paths are variables so # a laptop can point somewhere else, or at an empty directory when it is # serving plain HTTP and has no certificates at all. diff --git a/scripts/stage-nginx.sh b/scripts/stage-nginx.sh deleted file mode 100755 index 45c50303..00000000 --- a/scripts/stage-nginx.sh +++ /dev/null @@ -1,66 +0,0 @@ -#!/usr/bin/env bash -# -# Put the nginx configuration somewhere a `git checkout` cannot reach. -# -# nginx used to mount `deploy/nginx` straight out of the working tree. That -# makes a working directory a deployment input: check out a branch and the -# routes inside the running container are that branch's, one reload away from -# being live. It happened -- a feature branch's /mcp route was found sitting in -# the production container, inert only because nginx reads its configuration at -# start rather than continuously. -# -# So the container mounts `.staged/`, which is gitignored and therefore -# untouched by any checkout, and this is the only thing that writes to it. -# -# scripts/stage-nginx.sh # validate and stage -# scripts/stage-nginx.sh --check # validate only, change nothing -# -# Then reload: `docker compose exec nginx nginx -s reload` -# -# Files are copied **in place** rather than replaced, and that is load-bearing. -# A single-file bind mount follows the inode, and `git checkout` replaces a file -# rather than rewriting it -- which is why the environment config inside the -# running container was four days stale while the directory beside it was -# current. `cp` truncates and writes, so the inode survives and a reload is -# enough. -set -euo pipefail - -here=$(cd "$(dirname "$0")/.." && pwd) -src="$here/deploy/nginx" -dst="$src/.staged" -env_name=${NGINX_ENV:-dev} - -[ -f "$src/$env_name.conf" ] || { echo "no such environment: $env_name" >&2; exit 1; } - -# Validate the *source* before it can reach the staged copy. A configuration -# that fails here has not been staged, so a reload cannot pick it up. -tmp=$(mktemp -d) -trap 'rm -rf "$tmp"' EXIT -cp "$src/$env_name.conf" "$tmp/default.conf" -cp -r "$src/common" "$tmp/common" - -if ! docker run --rm \ - -v "$tmp/default.conf:/etc/nginx/conf.d/default.conf:ro" \ - -v "$tmp/common:/etc/nginx/common:ro" \ - -v "${LETSENCRYPT_DIR:-/etc/letsencrypt}:/etc/letsencrypt:ro" \ - -v "${CLOUDFLARE_CERT_DIR:-/etc/ssl/cloudflare}:/etc/ssl/cloudflare:ro" \ - nginx:alpine nginx -t >/dev/null 2>"$tmp/err"; then - echo "refusing to stage: $env_name.conf does not pass nginx -t" >&2 - grep -E 'emerg|error' "$tmp/err" | head -3 >&2 - exit 1 -fi - -if [ "${1:-}" = "--check" ]; then - echo "$env_name.conf is valid; nothing staged" - exit 0 -fi - -mkdir -p "$dst/common" -cp "$src/$env_name.conf" "$dst/default.conf" -# Delete-then-copy would change inodes under the running container. Refresh the -# contents of the directory instead, and remove only what has gone. -for f in "$src"/common/*; do cp "$f" "$dst/common/$(basename "$f")"; done -for f in "$dst"/common/*; do [ -f "$src/common/$(basename "$f")" ] || rm -f "$f"; done - -echo "staged $env_name.conf and common/ -> deploy/nginx/.staged" -echo "reload with: docker compose exec nginx nginx -s reload"