Stop a git checkout being able to change what nginx serves - #301
Merged
Merged
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
nginx mounted
deploy/nginxstraight 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. A feature branch's/mcproute was found sitting in the production-serving container this way, inert only because nginx reads its configuration at start.What shipped
deploy/nginx/Dockerfilebakes the configuration into an image, which is what the other three services in this compose file (render,content-node,mcp) already do. The running config is a property of an image, not of whatever is checked out.Certificates stay as runtime mounts — they're host secrets, and nothing that can't be rebuilt from this repository belongs in the image.
NGINX_ENVis a build argument defaulting todev, exactly as the old mount did, so behaviour on the indexed host is unchanged. It still matters: building with the dev config on the production host would deindex reactome.org.Deploying a config change
Not a build-time
RUN nginx -t: the config references certificates mounted at run time, so it would fail on every host for the wrong reason.Side effect worth knowing
The old single-file bind mount followed the inode, and
git checkoutreplaces files rather than rewriting them — so the container's environment config had been four days stale while the directory beside it was current. That's what made a/mcpreload fail withhost not found in upstream "mcp". Baking removes the mount entirely.Verified
Built, validated with
nginx -t, deployed; site 200,/mcpinitialises, the Java exporter and/RenderServicerender, and the container has no configuration mounts left.Also caught a live regression that wasn't this PR's:
/ContentService/data/species/mainanswered 404 because #289 added its nginx route while content-node still ran the image from before those endpoints existed. Rebuilt and redeployed content-node; fixed.🤖 Generated with Claude Code