Skip to content

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

Closed
Babissimo wants to merge 2 commits into
worktree-retire-tower-uifrom
retire-legacy-vhost
Closed

Babissimo wants to merge 2 commits into
worktree-retire-tower-uifrom
retire-legacy-vhost

Conversation

@Babissimo

Copy link
Copy Markdown
Contributor

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

Stacked on #389. Base is worktree-retire-tower-ui, so review that first; this diff is only the vhost removal. Needs a git rebase --onto once #389 squash-merges.

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.

Why this is split out, and what merging it costs

It was part of #389 until review. It carries a risk that the UI removal does not.

Today, 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. After this merges, 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.

So: merge this only once the flip is trusted enough to give that rollback up. Nothing else gates it, and the flip has been live and verified since 2026-09-14.

Worth doing separately

A catch-all default_server on 443 that refuses unmatched hostnames outright would turn that failure loud rather than silent, and would be worth having regardless of this PR. It is not included here because it changes behaviour for every unmatched name, not just this one, and I could not establish what currently relies on the fallthrough.

Verification

  • 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.

🤖 Generated with Claude Code

Babissimo and others added 2 commits September 15, 2026 15:09
tower-finder-service has owned the tower API since the monolith's copy was
deleted, and it now owns the UI too: it builds its own SPA and serves it from
its own edge. On 2026-09-14 the Cloudflare origin rules moved towers.retina.fm,
its staging and test counterparts and the fleet's tower-finder.retina.fm onto
port 8443, so all four names reach the service and none of them reach us.

That leaves the copy here dead rather than merely redundant. frontend/dist is
served on HOST_MAIN, HOST_MAP and HOST_TESTMAP only, and isMapDomain already
hid the tower tab on the latter two, so with HOST_MAIN flipped away the tab was
reachable on no live surface. The service's copy had also moved well ahead of
it, gaining address geocoding, rank tiers and a working frequencies parameter,
so keeping ours invited exactly the split-brain that 86capx9mv was raised to
end.

Every surface this bundle still serves is a live map, so the hostname-dependent
header goes with it and App renders one surface unconditionally. That also
settles, by deletion, the "Tower Finder" display label left open during the
rename: it named a surface that no longer ships from here. Because .map-surface
is now always set, the dark physics-tab background it used to override was dead
too, and collapses into the light rule.

Nothing about nginx changes here. Every vhost keeps forwarding /api/towers,
/api/elevation and /api/config to the service exactly as before, including the
${HOST_LEGACY_REDIRECT} vhost that carries the fleet's name. Retiring that vhost
is a separate change, deliberately held back: while it exists, deleting the
Cloudflare origin rule is still a working rollback for the fleet.

The entry chunks every map surface loads drop from 56.66 to 43.84 kB of JS
(gzip 18.04 to 14.08) and 26.45 to 19.23 kB of CSS (gzip 5.67 to 4.13).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
${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 55s —— View job


Review: Retire the fleet's legacy vhost

Todo list

  • Gather context (diff against origin/worktree-retire-tower-ui, read CLAUDE.md/ONBOARDING)
  • Understand the request (custom instruction: code review)
  • Run code review
  • Post findings

Summary

Scoped and clean. This diff (against worktree-retire-tower-ui, per the PR's stacking note) touches exactly what the description says: HOST_LEGACY_REDIRECT and its dedicated tower-finder.retina.fm vhost, nowhere else.

Checked:

  • Repo-wide grep for HOST_LEGACY_REDIRECT — zero hits after this diff, including backend/.env.example (never had the key) and docker-compose.yml base file.
  • EXPECTED_TLS_VHOSTS 8 → 7 in deploy/check-env-parity.py matches the template's actual vhost count post-removal (HOST_MAIN, HOST_API, HOST_MAP, HOST_TESTMAP, HOST_DASH, HOST_ADMIN, HOST_DATA).
  • test_towers_vhost_coverage.py: the - {"HOST_LEGACY_REDIRECT"} exemption removal is correct — routed_roles is derived from the template itself, so once the vhost is gone the set naturally excludes it and the plain equality assertion holds.
  • The rollback-hazard claim in the PR body checks out against the actual template: deploy/nginx/nginx.conf.template's first listen 443 ssl block is ${HOST_MAIN} ("tower finder frontend"), and the only _ catch-all is in the port-80 redirect block — there is no default_server on 443. So the sequencing warning (don't merge until the Cloudflare flip is trusted, since the old rollback would now silently 200 an HTML page instead of failing loud) is accurate, not just asserted.
  • Found one pre-existing wording quirk in docker-compose.prod.yml:70 ("render the same seven vhosts") that already said seven while the count was still 8 pre-PR — not introduced by this change, and this PR incidentally makes it correct.

No bugs, no dangling references, no inconsistency between the template, the two Python scripts, the four compose overlays, and the test fixtures.

Not verified

I could not execute anything in this sandboxed environment — no backend/.venv (so no pytest/pre-commit), and no docker/nginx binaries available to run deploy/check-env-parity.py or render the template through nginx -t. My check is static (diff review + grep + reading the rendered logic by hand); I did not independently re-run the verification steps the PR description lists. Those steps look mutually consistent with the code, but a maintainer with a working .venv/docker should still confirm the actual CI matrix is green.

Process note (not a code issue)

The PR is explicit that merging is gated on the Cloudflare flip being trusted, not on code review — worth a human confirming that gate before merging, since nothing in CI enforces it.

@Babissimo

Copy link
Copy Markdown
Contributor Author

Superseded by #398. GitHub auto-closed this when #389 merged with --delete-branch, since worktree-retire-tower-ui was its base, and a closed PR cannot be retargeted or reopened once its base is gone. Same commit, rebased onto main; nothing was lost.

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