From fdf27a420df2d28b63ad14fb5f2925639045d3da Mon Sep 17 00:00:00 2001 From: Venkat Date: Mon, 3 Aug 2026 08:14:13 +0000 Subject: [PATCH 1/4] feat!: drop legacy central tunnel support Removes DEFAULT_TUNNEL_ENDPOINT and every fallback to it: the access URL is always ..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 Claude-Session: https://claude.ai/code/session_01ErBUiAYTosnpbF9hvUj3Dn --- example.env | 9 --- listeners/commands/vm.js | 11 ++- util/libvirt/libvirt-server.js | 133 ++++++++++----------------------- 3 files changed, 43 insertions(+), 110 deletions(-) diff --git a/example.env b/example.env index 7ee0eb8..1353673 100644 --- a/example.env +++ b/example.env @@ -8,15 +8,6 @@ PROVISIONER_URL=https://example.com GUACAMOLE_CONNECTION_URL= APP_ENVIRONMENT= -# Optional. Oldest GlueOps/codespaces image release whose baked dev() reads -# /etc/glueops/tunnel_endpoint. Unset = regional tunnel endpoints disabled: -# every CDE VM uses the legacy central tunnel regardless of region config. -# Set this (e.g. v0.155.0) when enabling per-region tunnel_endpoint values in -# the provisioner's BAREMETAL_SERVER_CONFIGS; VMs created from older images -# still fall back to the central tunnel so their access URLs stay correct. -# Prerelease tags (vX.Y.Z-RC1) are supported on both sides: a stable release -# outranks its own RCs, so min=v0.155.0-RC1 admits the RCs and the stable cut. -REGIONAL_TUNNEL_MIN_IMAGE_TAG= # VM profiles store (required). The app fails to start if any of these are missing. # Must be a full URL including the scheme (https://). A bare host is rejected at startup. diff --git a/listeners/commands/vm.js b/listeners/commands/vm.js index b554389..e9432ed 100644 --- a/listeners/commands/vm.js +++ b/listeners/commands/vm.js @@ -1,4 +1,4 @@ -import libvirt, { DEFAULT_TUNNEL_ENDPOINT, cdeAccessUrl } from '../../util/libvirt/libvirt-server.js'; +import libvirt, { cdeAccessUrl } from '../../util/libvirt/libvirt-server.js'; import vmCreateModal from '../../user-interface/modals/vm-create.js'; import vmProfileModal from '../../user-interface/modals/vm-profile.js'; import buttonBuilder from '../../util/button-builder.js'; @@ -259,11 +259,10 @@ export default { // Build header text with optional CDE URL let headerText = `Server: ${server.serverName}\nRegion: ${server.region}\nDescription: ${description}\nStatus: ${server.status}\nCreated: ${createdDate}\nRepo: ${cloneRepo || 'None'}`; - if (cdeToken) { - // The tunnel_endpoint tag records which sish host this VM's - // tunnel actually connects to; VMs created before regional - // tunnels have no tag and live on the legacy central endpoint. - const tunnelHost = server.tags.tunnel_endpoint || DEFAULT_TUNNEL_ENDPOINT; + // Every CDE VM records where its tunnel connects; one without the + // tag predates regional tunnels and has no reachable URL to show. + const tunnelHost = server.tags.tunnel_endpoint; + if (cdeToken && tunnelHost) { const cdeUrl = cdeAccessUrl(server.serverName, tunnelHost, cdeToken); headerText += `\nAccess: <${cdeUrl}|Cloud Development Environment>`; } diff --git a/util/libvirt/libvirt-server.js b/util/libvirt/libvirt-server.js index bba736f..9b5e139 100644 --- a/util/libvirt/libvirt-server.js +++ b/util/libvirt/libvirt-server.js @@ -21,97 +21,30 @@ const provisionerDetail = (error) => { : ''; }; -// Legacy central sish endpoint. Every VM created before regional tunnels -// existed connects here, so it is the fallback whenever a region doesn't -// declare its own tunnel_endpoint (or the lookup fails outright). -export const DEFAULT_TUNNEL_ENDPOINT = 'tunnels.glueopshosted.com'; - -// Access URL for a CDE VM. The legacy central sish appends the SSH username -// to a "cde" bind (cde-.tunnels...); regional instances let the VM -// bind its bare hostname (..tunnels.cde...). The VM-side rule -// in the codespaces image's developer-setup.sh derives the bind from the -// same endpoint value, so URL and tunnel can never disagree. -export const cdeAccessUrl = (serverName, tunnelEndpoint, cdeToken) => { - const host = tunnelEndpoint === DEFAULT_TUNNEL_ENDPOINT - ? `cde-${serverName}.${tunnelEndpoint}` - : `${serverName}.${tunnelEndpoint}`; - return `https://${host}?folder=/workspaces/glueops&tkn=${cdeToken}`; -}; +// Access URL for a CDE VM: the VM binds its bare hostname at its region's +// sish endpoint, so the URL is ..tunnels.cde... The VM-side +// bind in the codespaces image's developer-setup.sh matches, so URL and +// tunnel cannot disagree. +export const cdeAccessUrl = (serverName, tunnelEndpoint, cdeToken) => + `https://${serverName}.${tunnelEndpoint}?folder=/workspaces/glueops&tkn=${cdeToken}`; // Resolve the sish endpoint for a region from the provisioner's region -// config. Never throws: creation must not fail (or silently go central-only -// with a regional URL) because of a transient /v1/regions error. A value that -// fails the hostname pattern is rejected here, before it fans out to the -// permanent tag, the access URLs, and cloud-init — those consumers must all -// agree on one endpoint, and cloud-init would silently drop a non-hostname. +// config. Throws if the region has no usable endpoint: with no central +// tunnel left to fall back to, a VM without a real endpoint would advertise +// a dead URL, so failing the creation loudly is the only honest outcome. const getTunnelEndpoint = async (region) => { - try { - const res = await axios.get(`${process.env.PROVISIONER_URL}/v1/regions`, { - headers: { 'Authorization': `${process.env.PROVISIONER_API_TOKEN}` }, - timeout: 1000 * 30 - }); - // Normalize case and any trailing dot: DNS treats them as equivalent, - // but the legacy-vs-regional split in cdeAccessUrl and on the VM is an - // exact string comparison against DEFAULT_TUNNEL_ENDPOINT. - const endpoint = res.data?.find(r => r.region_name === region) - ?.tunnel_endpoint?.trim().toLowerCase().replace(/\.$/, ''); - if (!endpoint) return DEFAULT_TUNNEL_ENDPOINT; - if (!TUNNEL_ENDPOINT_PATTERN.test(endpoint)) { - log.error(`Region ${region} has invalid tunnel_endpoint "${endpoint}", using default`); - return DEFAULT_TUNNEL_ENDPOINT; - } - return endpoint; - } catch (error) { - log.error('Failed to resolve tunnel endpoint, using default', axiosError(error)); - return DEFAULT_TUNNEL_ENDPOINT; + const res = await axios.get(`${process.env.PROVISIONER_URL}/v1/regions`, { + headers: { 'Authorization': `${process.env.PROVISIONER_API_TOKEN}` }, + timeout: 1000 * 30 + }); + // Normalize case and any trailing dot; DNS treats them as equivalent but + // the value becomes a permanent tag, a URL, and a cloud-init file. + const endpoint = res.data?.find(r => r.region_name === region) + ?.tunnel_endpoint?.trim().toLowerCase().replace(/\.$/, ''); + if (!endpoint || !TUNNEL_ENDPOINT_PATTERN.test(endpoint)) { + throw new Error(`Region ${region} has no valid tunnel_endpoint (got ${JSON.stringify(endpoint)})`); } -}; - -// Parse a codespaces release tag into its numeric core and optional -// prerelease suffix. Accepts the repo's real tag shapes: vX.Y.Z and -// prereleases like vX.Y.Z-RC1 (nonprod's image picker serves those). The -// strict shape check matters: Number('') is 0, not NaN, so a typo like "v" -// would otherwise parse as [0] and open the gate for every old image. -const parseImageTag = (tag) => { - const m = /^v?(\d+(?:\.\d+)*)(?:-([0-9a-z.-]+))?$/i.exec(String(tag).trim()); - if (!m) return null; - return { core: m[1].split('.').map(Number), pre: m[2]?.toLowerCase() ?? null }; -}; - -const compareImageTags = (a, b) => { - for (let i = 0; i < Math.max(a.core.length, b.core.length); i++) { - const d = (a.core[i] || 0) - (b.core[i] || 0); - if (d) return d; - } - // Equal numeric cores: a stable release outranks its own prereleases (an - // RC may predate fixes in the stable cut); prereleases compare naturally - // so RC2 < RC10. - if (!a.pre && !b.pre) return 0; - if (!a.pre) return 1; - if (!b.pre) return -1; - return a.pre.localeCompare(b.pre, undefined, { numeric: true }); -}; - -// Images older than REGIONAL_TUNNEL_MIN_IMAGE_TAG bake a dev() that ignores -// /etc/glueops/tunnel_endpoint and always tunnels to the legacy central -// endpoint, so giving such a VM a regional endpoint would advertise dead -// access URLs. Unset means regional tunnels stay off entirely; unparseable -// tags fail safe to legacy but are logged loudly, because a set-but-broken -// gate must not be indistinguishable from the feature being off. -const imageSupportsRegionalTunnel = (imageName) => { - const min = process.env.REGIONAL_TUNNEL_MIN_IMAGE_TAG; - if (!min) return false; - const floor = parseImageTag(min); - if (!floor) { - log.error(`REGIONAL_TUNNEL_MIN_IMAGE_TAG "${min}" is not a parseable release tag; regional tunnels stay OFF`); - return false; - } - const image = parseImageTag(imageName); - if (!image) { - log.error(`Image tag "${imageName}" is not a parseable release tag; VM falls back to the legacy tunnel endpoint`); - return false; - } - return compareImageTags(image, floor) >= 0; + return endpoint; }; export default { @@ -125,16 +58,26 @@ export default { // Generate CDE token if Single-Click Experience is enabled const cdeToken = singleClickExperience ? generateCdeToken() : null; - // The sish tunnel only runs on CDE-enabled VMs, so only resolve the - // endpoint for those, and only when the chosen image can actually read - // it — otherwise the VM gets the legacy endpoint so its tag and URLs - // match where the tunnel really connects. Recorded in tags below as - // the permanent truth (the region config may change later). + // The sish tunnel only runs on CDE-enabled VMs, so only resolve for + // those. Recorded in tags below as the permanent truth of where this + // VM's tunnel connects (the region config may change later). A region + // with no usable endpoint fails the creation here rather than handing + // the user a VM whose access URL points nowhere. let tunnelEndpoint = null; if (cdeToken) { - tunnelEndpoint = imageSupportsRegionalTunnel(imageName) - ? await getTunnelEndpoint(region) - : DEFAULT_TUNNEL_ENDPOINT; + try { + tunnelEndpoint = await getTunnelEndpoint(region); + } catch (error) { + log.error('Failed to resolve tunnel endpoint', axiosError(error)); + if (!batch) { + await client.chat.postEphemeral({ + channel: channel_id, + user: body.user.id, + text: `Failed to create server: no tunnel endpoint configured for region ${region}. Please report this to the platform team.` + }); + } + return { success: false, serverName, description: description || 'No description' }; + } } // Call the users.info method using the WebClient From 277b9e3d5229a0bc1e7c5c58944a8c6d55718b90 Mon Sep 17 00:00:00 2001 From: Venkat Date: Mon, 3 Aug 2026 08:28:35 +0000 Subject: [PATCH 2/4] fix: no access link for VMs tagged with the retired central tunnel 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 Claude-Session: https://claude.ai/code/session_01ErBUiAYTosnpbF9hvUj3Dn --- listeners/commands/vm.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/listeners/commands/vm.js b/listeners/commands/vm.js index e9432ed..4513d53 100644 --- a/listeners/commands/vm.js +++ b/listeners/commands/vm.js @@ -12,6 +12,10 @@ import vmEditModal from '../../user-interface/modals/vm-edit.js'; const log = logger(); +// VMs created before regional tunnels tunnel here; the host is retired and +// its URLs used a cde- prefix, so they can no longer be linked. +const RETIRED_CENTRAL_TUNNEL = 'tunnels.glueopshosted.com'; + const MAX_VM_COUNT = 10; const MAX_VM_RAM_MB = 9216; @@ -259,10 +263,12 @@ export default { // Build header text with optional CDE URL let headerText = `Server: ${server.serverName}\nRegion: ${server.region}\nDescription: ${description}\nStatus: ${server.status}\nCreated: ${createdDate}\nRepo: ${cloneRepo || 'None'}`; - // Every CDE VM records where its tunnel connects; one without the - // tag predates regional tunnels and has no reachable URL to show. + // Every CDE VM records where its tunnel connects. A VM with no tag + // predates regional tunnels, and one still tagged with the retired + // central host binds under a cde- prefix there — neither has a URL + // this bot can build, so show the VM without an access link. const tunnelHost = server.tags.tunnel_endpoint; - if (cdeToken && tunnelHost) { + if (cdeToken && tunnelHost && tunnelHost !== RETIRED_CENTRAL_TUNNEL) { const cdeUrl = cdeAccessUrl(server.serverName, tunnelHost, cdeToken); headerText += `\nAccess: <${cdeUrl}|Cloud Development Environment>`; } From 41ecbdb92720e235c89d9af6839c5dfbb98dd8f2 Mon Sep 17 00:00:00 2001 From: Venkat Date: Mon, 3 Aug 2026 08:58:39 +0000 Subject: [PATCH 3/4] fix: reject the retired tunnel at mint time; name the real failure cause 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 Claude-Session: https://claude.ai/code/session_01ErBUiAYTosnpbF9hvUj3Dn --- listeners/commands/vm.js | 6 +----- util/libvirt/libvirt-server.js | 36 ++++++++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/listeners/commands/vm.js b/listeners/commands/vm.js index 4513d53..b9c34dd 100644 --- a/listeners/commands/vm.js +++ b/listeners/commands/vm.js @@ -1,4 +1,4 @@ -import libvirt, { cdeAccessUrl } from '../../util/libvirt/libvirt-server.js'; +import libvirt, { cdeAccessUrl, RETIRED_CENTRAL_TUNNEL } from '../../util/libvirt/libvirt-server.js'; import vmCreateModal from '../../user-interface/modals/vm-create.js'; import vmProfileModal from '../../user-interface/modals/vm-profile.js'; import buttonBuilder from '../../util/button-builder.js'; @@ -12,10 +12,6 @@ import vmEditModal from '../../user-interface/modals/vm-edit.js'; const log = logger(); -// VMs created before regional tunnels tunnel here; the host is retired and -// its URLs used a cde- prefix, so they can no longer be linked. -const RETIRED_CENTRAL_TUNNEL = 'tunnels.glueopshosted.com'; - const MAX_VM_COUNT = 10; const MAX_VM_RAM_MB = 9216; diff --git a/util/libvirt/libvirt-server.js b/util/libvirt/libvirt-server.js index 9b5e139..f6f9a27 100644 --- a/util/libvirt/libvirt-server.js +++ b/util/libvirt/libvirt-server.js @@ -21,6 +21,11 @@ const provisionerDetail = (error) => { : ''; }; +// The retired central tunnel. VMs there bind under a "cde-" prefix, so any +// region still pointing at it would advertise a URL nothing serves — reject +// it where the value is minted, not just where it is rendered. +export const RETIRED_CENTRAL_TUNNEL = 'tunnels.glueopshosted.com'; + // Access URL for a CDE VM: the VM binds its bare hostname at its region's // sish endpoint, so the URL is ..tunnels.cde... The VM-side // bind in the codespaces image's developer-setup.sh matches, so URL and @@ -33,15 +38,27 @@ export const cdeAccessUrl = (serverName, tunnelEndpoint, cdeToken) => // tunnel left to fall back to, a VM without a real endpoint would advertise // a dead URL, so failing the creation loudly is the only honest outcome. const getTunnelEndpoint = async (region) => { - const res = await axios.get(`${process.env.PROVISIONER_URL}/v1/regions`, { - headers: { 'Authorization': `${process.env.PROVISIONER_API_TOKEN}` }, - timeout: 1000 * 30 - }); + let res; + try { + res = await axios.get(`${process.env.PROVISIONER_URL}/v1/regions`, { + headers: { 'Authorization': `${process.env.PROVISIONER_API_TOKEN}` }, + timeout: 1000 * 30 + }); + } catch (error) { + // Transport/timeout/5xx — transient and self-healing, so the caller + // must tell the user to retry rather than blame the region config. + throw Object.assign(new Error(`Could not read the region list: ${error.message}`), { transient: true }); + } + const entry = res.data?.find(r => r.region_name === region); + if (!entry) { + // /v1/regions omits a region whose backend is briefly unreachable, so + // an absent entry is not evidence of a misconfiguration. + throw Object.assign(new Error(`Region ${region} is not currently listed`), { transient: true }); + } // Normalize case and any trailing dot; DNS treats them as equivalent but // the value becomes a permanent tag, a URL, and a cloud-init file. - const endpoint = res.data?.find(r => r.region_name === region) - ?.tunnel_endpoint?.trim().toLowerCase().replace(/\.$/, ''); - if (!endpoint || !TUNNEL_ENDPOINT_PATTERN.test(endpoint)) { + const endpoint = entry.tunnel_endpoint?.trim().toLowerCase().replace(/\.$/, ''); + if (!endpoint || endpoint === RETIRED_CENTRAL_TUNNEL || !TUNNEL_ENDPOINT_PATTERN.test(endpoint)) { throw new Error(`Region ${region} has no valid tunnel_endpoint (got ${JSON.stringify(endpoint)})`); } return endpoint; @@ -69,11 +86,14 @@ export default { tunnelEndpoint = await getTunnelEndpoint(region); } catch (error) { log.error('Failed to resolve tunnel endpoint', axiosError(error)); + const reason = error.transient + ? `couldn't read the region list just now — please try again.` + : `no tunnel endpoint configured for region ${region}. Please report this to the platform team.`; if (!batch) { await client.chat.postEphemeral({ channel: channel_id, user: body.user.id, - text: `Failed to create server: no tunnel endpoint configured for region ${region}. Please report this to the platform team.` + text: `Failed to create server: ${reason}` }); } return { success: false, serverName, description: description || 'No description' }; From b29617e7a7fbd25d43e9d9f66da9a90902689e27 Mon Sep 17 00:00:00 2001 From: Venkat Date: Mon, 3 Aug 2026 09:41:50 +0000 Subject: [PATCH 4/4] fix: only 5xx/timeouts are transient; keep the axios error for logging 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 Claude-Session: https://claude.ai/code/session_01ErBUiAYTosnpbF9hvUj3Dn --- util/get-user-data.js | 13 ++++++++----- util/libvirt/libvirt-server.js | 14 ++++++++++---- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/util/get-user-data.js b/util/get-user-data.js index 8b8595d..2788f45 100644 --- a/util/get-user-data.js +++ b/util/get-user-data.js @@ -29,11 +29,14 @@ export default function configUserData(serverName, cdeToken = null, cdeEnv = {}, // Regional sish endpoint, world-readable like cde_token because the // host-side dev() (developer-setup.sh) reads it as the vscode user — - // codespace.env is root-only so it can't serve this. Absent file -> - // dev() falls back to the legacy central tunnel, which is also why the - // hostname is re-validated here (defence-in-depth; the resolver - // already enforced it): a value that can't round-trip safely through - // this runcmd is dropped rather than escaped. + // codespace.env is root-only so it can't serve this. There is no + // fallback any more: without this file dev() refuses to start the + // tunnel, so the resolver in libvirt-server.js must have accepted the + // value before it gets here. The pattern is re-checked (it must stay + // byte-identical to the resolver's) purely so a value that could not + // round-trip safely through this runcmd is dropped rather than + // escaped — reaching that branch means the resolver let something + // through it should not have. const tunnelEndpoint = cdeEnv?.TUNNEL_ENDPOINT; if (tunnelEndpoint && TUNNEL_ENDPOINT_PATTERN.test(tunnelEndpoint)) { userData += ` diff --git a/util/libvirt/libvirt-server.js b/util/libvirt/libvirt-server.js index f6f9a27..161ad20 100644 --- a/util/libvirt/libvirt-server.js +++ b/util/libvirt/libvirt-server.js @@ -45,9 +45,15 @@ const getTunnelEndpoint = async (region) => { timeout: 1000 * 30 }); } catch (error) { - // Transport/timeout/5xx — transient and self-healing, so the caller - // must tell the user to retry rather than blame the region config. - throw Object.assign(new Error(`Could not read the region list: ${error.message}`), { transient: true }); + // Only genuinely self-healing failures earn the retry message: no + // response at all (transport/timeout), a 5xx, or a 429. A 4xx is a + // real misconfiguration — a rotated PROVISIONER_API_TOKEN answers 401 + // — and must route to the escalate path instead of telling users to + // keep retrying something that will never succeed. The original error + // rides along so the logger can still record status and body. + const status = error.response?.status; + const transient = !error.response || status >= 500 || status === 429; + throw Object.assign(new Error(`Could not read the region list: ${error.message}`), { transient, cause: error }); } const entry = res.data?.find(r => r.region_name === region); if (!entry) { @@ -85,7 +91,7 @@ export default { try { tunnelEndpoint = await getTunnelEndpoint(region); } catch (error) { - log.error('Failed to resolve tunnel endpoint', axiosError(error)); + log.error('Failed to resolve tunnel endpoint', axiosError(error.cause ?? error)); const reason = error.transient ? `couldn't read the region list just now — please try again.` : `no tunnel endpoint configured for region ${region}. Please report this to the platform team.`;