Skip to content

Retire the fleet's legacy vhost, now Cloudflare routes that name - #398

Merged
Babissimo merged 1 commit into
mainfrom
retire-legacy-vhost
Sep 15, 2026
Merged

Babissimo merged 1 commit into
mainfrom
retire-legacy-vhost

Conversation

@Babissimo

Copy link
Copy Markdown
Contributor

Ticket: https://app.clickup.com/t/123zgec2zw1

Supersedes #391, which GitHub auto-closed when its base branch was deleted by #389's merge. Same commit, rebased onto main with git rebase --onto; no longer stacked.

What this does

${HOST_LEGACY_REDIRECT} proxied tower-finder.retina.fm, the name the fleet calls, to tower-finder-service. Since the origin flip of 2026-09-14 Cloudflare sends that name straight to the service's own edge on 8443, so nothing reaches this vhost. It goes, along with the variable in all four compose overlays, the substitution allowlist, the test fixtures, and the HOST_LEGACY_REDIRECT exemption in test_towers_vhost_coverage.py.

EXPECTED_TLS_VHOSTS drops 8 to 7. It is hardcoded, and left behind it fails parity on all three environments at once.

What merging this costs

Held back from #389 deliberately, because it carries a risk the UI removal did not.

Until this merges, deleting the Cloudflare origin rule is a working rollback: tower-finder.retina.fm lands on this repo's nginx on 443 and this vhost proxies it to the service. Afterwards that rollback is not merely unavailable, it is harmful. The template declares no default_server on 443 (the _ catch-all is only in the port-80 redirect list), so nginx falls back to the first 443 server block, which is the towers SPA. A node calling /api/towers would get an HTML page with a 200 and fail on decode rather than on a status it can branch on.

Merging is therefore a decision to give that rollback up. The flip has been live and verified since 2026-09-14.

Worth doing separately

A catch-all default_server on 443 refusing unmatched hostnames would turn that failure loud rather than silent, and is worth having regardless. Not included here because it changes behaviour for every unmatched name, not just this one.

Verification (re-run after the rebase, not carried over)

  • pre-commit run --all-files passes.
  • test_towers_vhost_coverage.py and test_nginx_rewrite_ordering.py pass (8 tests) against the shared nginx_helpers fixture with the variable removed.
  • deploy/check-env-parity.py passes; staging and test in parity with production.
  • The rendered template passes nginx -t ("syntax is ok") and renders 7 TLS vhosts.
  • A repo-wide grep for HOST_LEGACY_REDIRECT returns nothing.
  • Diff confirmed to contain only the 9 vhost files, with none of Retire this repo's tower search UI, now the service serves it #389's frontend changes duplicated by the rebase.

🤖 Generated with Claude Code

${HOST_LEGACY_REDIRECT} existed to proxy tower-finder.retina.fm, the name the
fleet calls, to tower-finder-service. Since the origin flip of 2026-09-14
Cloudflare sends that name to the service's own edge on 8443, so nothing
reaches this vhost and it is dead weight.

Held back from the UI removal it was originally part of, because it carries a
risk that change does not. Until this merges, deleting the Cloudflare origin
rule is a working rollback: the name lands here and this vhost proxies it on.
Afterwards it is not, and worse than merely absent. The template declares no
default_server on 443, so nginx falls back to the FIRST 443 block, which is the
towers SPA. A node calling /api/towers would then be answered with an HTML page
and a 200, and fail on decode rather than on a status it can branch on.

Merge this only once the flip is trusted enough to give that rollback up. A
catch-all default_server that refused unmatched hostnames outright would turn
the failure loud, and is worth doing, but it changes behaviour for every
unmatched name rather than this one and belongs in its own change.

EXPECTED_TLS_VHOSTS drops 8 to 7 alongside the removed vhost; it is hardcoded
and fails all three environments at once if left behind.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 15, 2026

Copy link
Copy Markdown

Claude finished @Babissimo's task in 1m 25s —— View job


Review: Retire the fleet's legacy vhost

Summary: Clean, well-scoped removal. I traced every reference to HOST_LEGACY_REDIRECT across the repo and confirmed it's fully gone (grep -rn HOST_LEGACY_REDIRECT returns nothing), and verified the deletions are consistent across all 9 files:

  • deploy/nginx/nginx.conf.template: the standalone ${HOST_LEGACY_REDIRECT} server block and its port-80 server_name entry are removed cleanly, with no orphaned comments left behind.
  • The # RETINA_INCLUDE snippets/tower-finder.conf include used by the other vhost (the API vhost's /towers/api/towers rewrite, nginx.conf.template:137) is untouched and still valid — the snippet file isn't orphaned, it has a second, still-live consumer.
  • EXPECTED_TLS_VHOSTS dropping from 8 → 7 matches reality: I counted the rendered template and it now has exactly 7 listen 443 ssl; blocks.
  • backend/tests/test_towers_vhost_coverage.py: removing the HOST_LEGACY_REDIRECT exemption is correct — routed_roles is computed by scanning for blocks containing towers-proxy.conf or tower-finder.conf, and the deleted vhost was the only block matching on the removed name, so routed_roles no longer contains that role and the equality assertion holds without the carve-out.
  • All four compose overlays (local/prod/staging/test) and backend/tests/nginx_helpers.py drop the corresponding env var/fixture value in lock-step with the template and the two Python scripts (render-nginx-config.py's SUBSTITUTIONS, check-env-parity.py's HOST_VARS).

Risk called out in the PR body is real and correctly assessed: I confirmed ${HOST_MAIN} (the towers SPA) is the first server { listen 443 ssl; ... } block in the template and there's no default_server on 443 — only the port-80 redirect block has the _ catch-all. So the PR description's claim ("an unmatched TLS SNI name / a restored Cloudflare origin rule would silently land on the SPA instead of erroring") checks out against the actual template. Deferring the default_server hardening to a separate PR is reasonable since it's a behavior change with broader blast radius than this cleanup.

No bugs, no inconsistencies, no dead code left behind. I wasn't able to execute pre-commit run --all-files or the pytest suite in this sandbox (no backend venv present, and shell commands requiring package-manager/network access aren't approved here), so I verified correctness by static tracing of every affected identifier instead — the change is small and mechanical enough that this gives good confidence. Recommend the PR author (or CI) still confirm the venv-based verification steps listed in the PR body before merging, per ONBOARDING's "Before you push" gate.

@Babissimo
Babissimo merged commit 178aa45 into main Sep 15, 2026
14 checks passed
@Babissimo
Babissimo deleted the retire-legacy-vhost branch September 15, 2026 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant