Skip to content

feat!: require a tunnel endpoint; always bind the bare hostname (legacy removal) - #554

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

feat!: require a tunnel endpoint; always bind the bare hostname (legacy removal)#554
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. Part of retiring the legacy central tunnel.

  • The tunnels.glueopshosted.com default is gone: dev() reads /etc/glueops/tunnel_endpoint and, on a CDE VM, errors out if it's missing or empty instead of silently tunneling to the central host.
  • The TUNNEL_BIND legacy branch is gone: the VM always binds its bare $HOSTNAME, so URLs are always <hostname>.<region>.tunnels.cde.glueopshosted.com.

Preconditions

Note on old images

This only affects VMs built from this release onward. Older images keep their baked-in dev() and remain safe as long as the slackbot still hands them a working endpoint — which is exactly what the slackbot's REGIONAL_TUNNEL_MIN_IMAGE_TAG gate governs, so retire that gate (see the slackbot PR) only once pre-v0.155.1 images are no longer offered or in use.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ErBUiAYTosnpbF9hvUj3Dn

Drops the legacy central fallback and the "cde" bind branch. dev() now
fails loudly if /etc/glueops/tunnel_endpoint is missing on a CDE VM,
rather than tunneling somewhere unexpected.

BREAKING CHANGE: images from this release only work with a slackbot
that writes /etc/glueops/tunnel_endpoint for every CDE VM.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErBUiAYTosnpbF9hvUj3Dn
The previous commit's edit removed the adjacent cde-boot docker-exec
block (gh auth, repo clone, AutoGlue setup) along with the tunnel
code — restored verbatim. Also rebases the change onto the current
release so IdentitiesOnly=yes is preserved, and adds an explicit
failure when the endpoint is still the retired central host, which
would otherwise bind bare while that sish prefixes the username.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErBUiAYTosnpbF9hvUj3Dn
@github-actions github-actions Bot added the patch label Aug 3, 2026
@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 chore(fallback): update glueops/github-workflows #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

A tunnel misconfiguration should cost only the tunnel: cde-boot (gh
auth, repo clone, AutoGlue) is unrelated, the VM stays reachable over
the tailnet, and every dev re-run previously hit the same early return
so the bootstrap could never run.

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 chore(deps): update dependency argoproj/argo-cd to v2.13.6 #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 feat!: require a tunnel endpoint; always bind the bare hostname (legacy removal) #554 — cut the release.
  3. slackbot feat: install claude code cli in devcontainer image #504 — deploy last.

🤖 Generated with Claude Code

The earlier guards returned before code serve-web, so a tunnel
misconfiguration also withheld the editor — contradicting their own
rationale that the VM stays reachable over the tailnet. They now set
TUNNEL_OK=0, skip only autossh, and tell the user to reach the VM over
Tailscale.

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