Repository navigation
ci: every nginx upstream must be a container the production web role runs - #3333
Merged
Merged
Conversation
…runs The 2.43.0 deploy answered 502 for every static asset at the origin because nginx sent those paths to a file-server container that production web boxes never start; the hotfix sent them back through Node. Nothing would have caught the same mistake again. A new check, `make check-nginx-upstreams` (also called verbatim by a hosted workflow), parses every upstream host from the nginx templates and the rendered Rust route locations, reads the services the web role starts from the deploy hook and the Compose file, and fails naming the upstream, the config file and the hook line when an upstream names a service the web role does not run. An empty, commented exception list is the only way around it. The 2.43.0 shape fails it; edge passes. The hook reader joins continued lines, tokenizes only the container-start lines, finishes one-line helpers, and names a line in every error. The README states what it does not read: includes of files that are not .conf or .template, and shell constructs it cannot follow. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…continued lines and one-line helpers The reader matched compose only as a bare docker-compose or docker compose followed by config, so the hook's own /usr/local/bin/docker-compose start was neither refused nor counted, while plugin-style forms (a COMPOSE array, a hook moved to docker compose, command -v) were refused. It now matches the executable's basename, skips the global options before the subcommand, and accepts config or no subcommand. It also stops a comment's trailing backslash from swallowing the next line, closes one-line helper functions (including those ending in "& }"), refuses direct docker compose starts, reviews docker by absolute path, parses spaced and function-keyword declarations, and names the line in every error, including the ROLE_IMAGES one. Six fixture groups cover these; the 2.43.0 shape still fails naming file-server and its template. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The only conflict was the Makefile: edge added five local check targets at the same place this branch adds check-nginx-upstreams; both sets are kept.
The only conflict was the Makefile again: edge added check-client-errors at the place this branch adds check-nginx-upstreams; both are kept.
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
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.
Observable change: none in the product. A new local check,
make check-nginx-upstreams, and a hosted workflow that calls it verbatim. No schema change, no nginx behaviour change, no route change; the deploy hook is read, not changed.Why: the 2.43.0 deploy answered 502 for every static asset at the origin because nginx sent those paths to a file-server container that production web boxes never start (fixed by #3325, which sent them back through Node). Nothing would catch the same mistake again.
What it checks: every upstream host in the nginx templates and the rendered Rust route locations must be a service the production web role starts (read from the deploy hook's container-start lines and the Compose service names), or be on an explicit, commented exception list, which is empty. A failure names the upstream, the config file and the hook line.
Evidence: 23 tests; the 2.43.0 shape (the static proxy from #3302) fails naming file-server and its template; edge passes; 17 topology mutations behave as expected (pool member, direct proxy, rendered Rust map target, removed startup line, removed Compose service, and more); ordinary hook edits (a continued line, a multi-line string, a one-line helper, extra flags on the migration line) pass. Two independent review rounds; the reader's limits (includes of files that are not .conf/.template; shell constructs it cannot follow) are stated in the README and the source.
🤖 Generated with Claude Code