Skip to content

feat!: drop legacy central tunnel support (fallbacks + image gate) - #504

Draft
venkatamutyala wants to merge 4 commits into
mainfrom
chore/drop-legacy-tunnel-support
Draft

feat!: drop legacy central tunnel support (fallbacks + image gate)#504
venkatamutyala wants to merge 4 commits into
mainfrom
chore/drop-legacy-tunnel-support

Conversation

@venkatamutyala

Copy link
Copy Markdown
Contributor

DRAFT — do not merge until the preconditions below hold. This is the largest piece of retiring the legacy central tunnel.

Removed:

  • DEFAULT_TUNNEL_ENDPOINT and every fallback to it.
  • The legacy branch in cdeAccessUrl — URLs are always <name>.<region>.tunnels.cde….
  • getTunnelEndpoint's error/missing fallbacks: it now throws, and createServer fails the request with a clear Slack message. With no central tunnel left, falling back would mint a VM whose URL points at a dead host, so failing loudly is the only honest outcome.
  • The image gate (imageSupportsRegionalTunnel, parseImageTag, compareImageTags) and the REGIONAL_TUNNEL_MIN_IMAGE_TAG env var (also dropped from example.env).
  • /vm list no longer renders an access link for a CDE VM whose tags lack tunnel_endpoint — those predate regional tunnels and have no reachable URL.

Preconditions — all must be true

  • Every region declares tunnel_endpoint (companion: feat!: require tunnel_endpoint on every region (legacy tunnel removal) provisioner#236). Check GET /v1/regions for any "tunnel_endpoint": null.
  • No VM still on the central tunnel. Check GET /v1/list for CDE VMs (those with a cde_token tag) whose tags either lack tunnel_endpoint or set it to tunnels.glueopshosted.com. Those VMs lose their /vm list link (or get a wrong URL) the moment this merges — they must be drained first.
  • No pre-v0.155.1 image offered or in use. Removing the gate means an old image in a regional region would tunnel centrally while its URL points regionally — the exact defect the gate was added to prevent. The provisioner offers the latest 60 releases, so this is the slowest precondition; consider narrowing the offered list first.
  • Central sish box has zero connected clients (check its admin console) before decommissioning.

Companion PRs: GlueOps/provisioner#236, GlueOps/codespaces#554.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ErBUiAYTosnpbF9hvUj3Dn

Removes DEFAULT_TUNNEL_ENDPOINT and every fallback to it: the access
URL is always <name>.<region>.tunnels.cde..., getTunnelEndpoint throws
when a region has no usable endpoint (creation fails cleanly instead of
advertising a dead URL), and the REGIONAL_TUNNEL_MIN_IMAGE_TAG image
gate is gone along with its env var. /vm list renders no access link
for a CDE VM that predates the tunnel_endpoint tag, since there is no
longer a host it could point at.

BREAKING CHANGE: every region must declare tunnel_endpoint, and only
images that read /etc/glueops/tunnel_endpoint may be offered.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErBUiAYTosnpbF9hvUj3Dn
Those VMs bind under a cde- prefix on the old host, so building a URL
from the tag alone advertised a link that resolves to nothing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErBUiAYTosnpbF9hvUj3Dn
@venkatamutyala

Copy link
Copy Markdown
Contributor Author

Multi-agent review of the legacy-removal PRs (4 reviewers, findings adversarially verified)

23 raw findings → 7 confirmed. The headline one was a self-inflicted regression, now fixed:

  • 🔴 critical (codespaces): the first commit's edit deleted the adjacent cde-boot docker-exec block along with the tunnel code — the only caller of cde-boot in the repo. Every VM built from that branch would have booted with no gh auth, no repo clone, no AutoGlue profiles, and no retry marker. Restored verbatim. The same rebase also recovered IdentitiesOnly=yes, which had been lost by restoring the file from a stale local main.
  • 🟠 major (cross-repo): the codespaces precondition was the wrong condition — "the slackbot writes the file" is true since v4.3.0, but the file can legitimately contain the legacy host. dev() now hard-fails on tunnels.glueopshosted.com explicitly, so a mis-sequenced release is loud instead of silently binding bare against a sish that prefixes usernames.
  • 🟡 minor (slackbot): /vm list hid the link only when the tag was absent, but every VM created since feat: regional tunnel endpoints for CDE tunnels and access URLs #499 has a tag — set to the legacy host whenever the gate was off or the region unconfigured. Those would have rendered confidently-wrong URLs. Now a tag equal to the retired host is treated like a missing one.
  • 🟡 minor (provisioner): every documented BAREMETAL_SERVER_CONFIGS example omitted the now-required field, so following the docs produced a config that refuses to start. Examples + prose fixed.

Deliberately not changed (verified as intended behavior): the hard-fail on a transient /v1/regions error (fail-loud is the design; retry/typed-error refinement noted for later), and the per-VM resolve in batch creates (latency only).

Merge order remains the operative risk, and it is unchanged: provisioner first (after every region declares tunnel_endpoint), then the codespaces release, then the slackbot. Merging the slackbot first turns every CDE create in an unconfigured region into a hard failure; merging the provisioner before the config is complete refuses to start the API.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the patch label Aug 3, 2026
getTunnelEndpoint now rejects tunnels.glueopshosted.com so the create
path fails as loudly as /vm list already renders (the constant moved
here and is shared). Transport errors, timeouts and a region briefly
absent from /v1/regions are tagged transient, so the user is asked to
retry instead of being told their region is misconfigured and to
escalate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErBUiAYTosnpbF9hvUj3Dn
@venkatamutyala

Copy link
Copy Markdown
Contributor Author

Final review — lead reviewers + requested specialists (29 agents, findings adversarially verified)

23 raw findings → 10 confirmed → all fixed. Structure: four lead reviewers (one per PR + a cross-repo seams lead) each assessed the post-fix branches and requested domain specialists; the specialists they asked for (GlueOps deployment config; sish flag semantics as actually deployed) were fanned out and settled their questions empirically; everything then went through adversarial verification.

The previous round's five fixes were re-verified as correct — the cde-boot block is byte-identical to origin/main, IdentitiesOnly=yes is intact, and the full dev() body was diffed hunk-by-hunk against origin/main with every hunk confirmed intentional.

Fixed in this round

  • ⚠️ Guard ordering (codespaces): the endpoint hard-fail sat before the cde-boot exec, so a tunnel misconfiguration also blocked gh auth, the repo clone and AutoGlue setup — and because dev is the documented entrypoint, every re-run hit the same early return 1, making the bootstrap unreachable. Validation moved inside the CDE_TOKEN block, after cde-boot: a bad endpoint now costs only the tunnel, and the VM stays reachable over the tailnet.
  • Retired-host asymmetry (slackbot + provisioner): tunnels.glueopshosted.com was rejected by dev() and suppressed by /vm list, but silently accepted and advertised by the create path and the provisioner validator. Now rejected where the value is minted — getTunnelEndpoint throws on it (constant shared with /vm list), and the provisioner refuses it at config load in every case/trailing-dot variant.
  • Misdiagnosed failures (slackbot): transport errors, timeouts, and a region briefly omitted from /v1/regions (the provisioner deliberately skips a region whose Waggle backend is unreachable) all reported "no tunnel endpoint configured … report this to the platform team" — a false claim about config, plus an escalation instruction, for a self-healing condition. Those are now tagged transient and tell the user to retry; the config message is reserved for a genuinely invalid endpoint.
  • Docs (provisioner): .ai/AGENTS.md's test recipe built a ProxmoxConfig that now raises; the datacenter-onboarding runbook omitted tunnel_endpoint and had no step for standing up the region's tunnel endpoint. Both fixed.

Correction to PR #554's precondition (please read before merging)

The stated precondition — "the slackbot writes /etc/glueops/tunnel_endpoint, true since v4.3.0" — is the wrong condition. What matters is that the deployed slackbot writes a regional value; shipping this image while the running slackbot still resolves some regions to the central host hard-fails every brand-new CDE VM in those regions. The safe order across all three PRs is:

  1. provisioner fix(deps): update dependency dotenv to v17 #236 — only after every region in BAREMETAL_SERVER_CONFIGS declares a valid tunnel_endpoint (check the raw config, not /v1/regions: a disabled or Waggle-unreachable region never appears there, yet still fails startup).
  2. codespaces #554 — cut the release.
  3. slackbot feat!: drop legacy central tunnel support (fallbacks + image gate) #504 — deploy last.

🤖 Generated with Claude Code

A rotated PROVISIONER_API_TOKEN answers 401, which was being reported to
users as "please try again" forever instead of routing to the escalate
path, and the re-wrapped error made the log line lose status and body.
Classify on the response and carry the original as cause. Also corrects
the cloud-init comment that still described the deleted fallback.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErBUiAYTosnpbF9hvUj3Dn
@venkatamutyala

Copy link
Copy Markdown
Contributor Author

Round 3 — six-lens sweep + completeness critic (28 agents, adversarially verified)

21 raw findings → 9 confirmed → all fixed. Zero critical, zero major (round 1 had a critical, round 2 a major), and the nine deduplicated to five real issues — three independent lenses landed on the same transient-tagging defect, two on the same stale comment. That convergence is the signal this has bottomed out.

Fixed

  • Transient classification was too broad (slackbot) — the round-2 catch tagged every axios rejection transient, so a rotated PROVISIONER_API_TOKEN (the provisioner answers 401) or a wrong PROVISIONER_URL (404) told users "please try again" forever, never routing to the escalate path — the single most likely permanent misconfiguration hit the retry branch. Re-wrapping also discarded the AxiosError, so the log line lost status and the provisioner's detail body. Now: transient only when there is no response at all, a 5xx, or a 429; the original error rides along as cause so the logger recovers status and body.
  • A bad endpoint withheld the IDE (codespaces) — round 2's guards return 1'd before code serve-web, contradicting their own rationale that the VM stays usable over the tailnet. They now set TUNNEL_OK=0, skip only autossh, and tell the user to reach the VM over Tailscale. Verified in isolation: valid endpoint → tunnel + IDE; empty or retired endpoint → IDE only, loud error.
  • Stale cloud-init comment (slackbot) — still documented the deleted central-tunnel fallback as the fail-safe for a dropped runcmd, in the very file implementing the contract these PRs rest on.
  • Test recipe rejected by its own validator (provisioner).ai/AGENTS.md used <REGION_NAME>.tunnels…; angle brackets fail the hostname pattern, so the documented snippet raised. Now an obviously-fake but valid value.
  • Onboarding step 6 was too thin (provisioner) — it said "stand up the tunnel endpoint" without the ACME scoping, cert/distribution, validation record, or wildcard steps that actually gate issuance. Now all four artifacts in the order cert issuance requires.

One correction for PR #504's body (not code)

Decommission precondition 4 says to check the central sish's admin console — that box does not run --admin-console. Use docker logs --since 10m sish on the box, or ss -tnp state established '( sport = :2222 )'. Note sish_users/ lists every username that ever registered a key, not who is currently connected.

Assessment

Three rounds, 84 agents: critical → major → all-minor, with the last round's findings concentrated in wording, docs, and one behavioral nuance in code the previous round had just added. Further review rounds are unlikely to pay for themselves; the remaining risk in this change set is operational (the drain preconditions and deploy order), not defect-shaped.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant