diff --git a/README.md b/README.md index 905eba3..240eb91 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,14 @@ Rootless Podman e2e tests run in GitHub Actions on Ubuntu alongside Docker. They cover image startup, Dockerfile builds, mounted workspace edits, and reopening and editing a container workspace after the extension runtime restarts. +Configured `forwardPorts` are automatically available on host `127.0.0.1` at +those ports, including services listening only on container localhost. Numeric +ports and `"host:port"` destinations are supported. The container must include +socat, Node.js, or Python 3 for the streaming relay. A missing relay or occupied +host port fails startup with an actionable error. Stop, Remove, and disconnecting +the extension close its listeners and active connections; reconnecting restores +the saved forwarding configuration. This does not require Docker `-p` publication. + For local development, use Node from `.nvmrc`, then run `npm ci` and `npm run dev`. This prepares the local extension in `.tmp/dev` with esbuild and starts the development server alongside browser bundle watching. The first bundle is ready @@ -47,7 +55,13 @@ The desktop extension contributes these commands for the active workspace: Docker-backed end-to-end tests run on Ubuntu for every pull request and push to `main`. They cover the official JavaScript/Node 24 and Ubuntu 24.04 images, plus a Dockerfile build with a custom workspace mount. -Each test opens a fresh copy of a workspace fixture, starts the container through Quick Pick, checks its runtime and reads the fixture inside the container. It then writes a file from inside Docker and verifies the file in Explorer and its contents in the editor. A subsequent workspace edit must be readable inside the container. Finally, the test stops the container through Quick Pick and verifies that execution is rejected while stopped. +The forwardPorts acceptance test starts the packaged Dev Containers RPC process +with a localhost-only HTTP server and `forwardPorts: [3000]`. It asserts the +response from the host and checks listener cleanup and restoration across repeated +opening, process disconnect/reconnect, Stop, and Remove. It runs before the browser +suite on Ubuntu, with no Docker publication or manual forwarding. + +Each browser test opens a fresh copy of a workspace fixture, starts the container through Quick Pick, checks its runtime and reads the fixture inside the container. It then writes a file from inside Docker and verifies the file in Explorer and its contents in the editor. A subsequent workspace edit must be readable inside the container. Finally, the test stops the container through Quick Pick and verifies that execution is rejected while stopped. The tests use the `Devcontainer` page object from `@lvce-editor/test-worker` for start, stop, execution, output assertions, and cleanup. Dependency installation loads the declared test-worker dependency into the development server. diff --git a/package.json b/package.json index 45648d1..1a76924 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "build": "node packages/build/src/build.ts", "build:watch": "npm run build:watch --workspace=packages/build", "dev": "node packages/build/src/dev.ts", - "e2e:devcontainer": "npm run e2e:headless --prefix packages/e2e", + "e2e:devcontainer": "npm run test:e2e --workspace=packages/devcontainer-node && npm run e2e:headless --prefix packages/e2e", "format": "prettier --write .", "lint": "eslint . && knip --no-config-hints", "test": "npm test --workspaces --if-present", diff --git a/packages/devcontainer-node/package.json b/packages/devcontainer-node/package.json index 294e762..86d3de9 100644 --- a/packages/devcontainer-node/package.json +++ b/packages/devcontainer-node/package.json @@ -24,7 +24,8 @@ }, "scripts": { "test": "node --unhandled-rejections=warn --experimental-vm-modules ../../node_modules/jest/bin/jest.js --detectOpenHandles --forceExit", - "test:watch": "node --unhandled-rejections=warn --experimental-vm-modules ../../node_modules/jest/bin/jest.js --watch" + "test:watch": "node --unhandled-rejections=warn --experimental-vm-modules ../../node_modules/jest/bin/jest.js --watch", + "test:e2e": "node --test test/ForwardPorts.e2e.ts" }, "jest": { "injectGlobals": false, diff --git a/packages/devcontainer-node/src/devcontainerCli.d.ts b/packages/devcontainer-node/src/devcontainerCli.d.ts index 05b6dcb..91bb942 100644 --- a/packages/devcontainer-node/src/devcontainerCli.d.ts +++ b/packages/devcontainer-node/src/devcontainerCli.d.ts @@ -32,3 +32,12 @@ export function dockerInspectContainer( ): Promise export function getDevcontainerCliPath(): string + +export function forwardPorts( + options: ContainerOptions & { + workspaceFolder?: string + forwardPorts?: unknown + remoteUser?: string + }, +): Promise +export function disposeForwardPorts(): void diff --git a/packages/devcontainer-node/src/parts/DevContainerCli/DevContainerCli.ts b/packages/devcontainer-node/src/parts/DevContainerCli/DevContainerCli.ts index 6da2885..d92788e 100644 --- a/packages/devcontainer-node/src/parts/DevContainerCli/DevContainerCli.ts +++ b/packages/devcontainer-node/src/parts/DevContainerCli/DevContainerCli.ts @@ -3,6 +3,7 @@ import type { ErrorResult } from '../SerializeError/SerializeError.ts' import * as CliError from '../CliError/CliError.ts' import * as CliJson from '../CliJson/CliJson.ts' import * as ContainerFileSystem from '../ContainerFileSystem/ContainerFileSystem.ts' +import * as ForwardPorts from '../ForwardPorts/ForwardPorts.ts' import * as RunProcess from '../RunProcess/RunProcess.ts' export interface CliCommandSuccess { @@ -17,6 +18,7 @@ export interface CliCommandSuccess { export interface CliCommandError extends ErrorResult { commandName: string exitCode?: number | null + json?: unknown missingExecutable?: string ok: false stderr?: string @@ -91,6 +93,7 @@ export const getCliUpArgs = ({ '--workspace-folder', workspaceFolder, '--no-lockfile', + '--include-merged-configuration', '--log-format', 'text', '--log-level', @@ -269,15 +272,53 @@ export const cliReadConfiguration = (options: WorkspaceOptions) => { ) } -export const cliUp = (options: WorkspaceOptions) => { - return runDevcontainerCli( +export const cliUp = async ( + options: WorkspaceOptions, +): Promise => { + const result = await runDevcontainerCli( 'DevContainerNode.cliUp', [getDevcontainerCliPath(), ...getCliUpArgs(options)], options.containerCli, options.onOutput, ) + if (!result.ok) return result + const json = result.json as { + containerId: string + remoteUser?: string + mergedConfiguration?: { forwardPorts?: unknown } + } + try { + await ForwardPorts.ensure({ + containerCli: options.containerCli ?? dockerPath, + containerId: json.containerId, + forwardPorts: json.mergedConfiguration?.forwardPorts, + remoteUser: json.remoteUser, + workspaceFolder: options.workspaceFolder, + }) + return result + } catch (error) { + return { + commandName: result.commandName, + errorCode: 'DEVCONTAINER_FORWARD_PORTS_ERROR', + errorMessage: error instanceof Error ? error.message : String(error), + errorStack: undefined, + json: result.json, + ok: false, + stderr: result.stderr, + stdout: result.stdout, + } + } } +export const forwardPorts = ( + options: ContainerOptions & { + workspaceFolder?: string + forwardPorts?: unknown + remoteUser?: string + }, +): Promise => ForwardPorts.ensure(options) +export const disposeForwardPorts = ForwardPorts.dispose + export const cliExec = (options: ExecOptions) => { return runDevcontainerCommand( 'DevContainerNode.cliExec', @@ -286,7 +327,8 @@ export const cliExec = (options: ExecOptions) => { ) } -export const dockerStopContainer = (options: ContainerOptions) => { +export const dockerStopContainer = async (options: ContainerOptions) => { + await ForwardPorts.remove(options.containerId) return runDocker( 'DevContainerNode.dockerStopContainer', getDockerStopArgs(options), @@ -294,7 +336,8 @@ export const dockerStopContainer = (options: ContainerOptions) => { ) } -export const dockerRemoveContainer = (options: ContainerOptions) => { +export const dockerRemoveContainer = async (options: ContainerOptions) => { + await ForwardPorts.remove(options.containerId) return runDocker( 'DevContainerNode.dockerRemoveContainer', getDockerRemoveArgs(options), diff --git a/packages/devcontainer-node/src/parts/ForwardPorts/ForwardPorts.ts b/packages/devcontainer-node/src/parts/ForwardPorts/ForwardPorts.ts new file mode 100644 index 0000000..f43eb64 --- /dev/null +++ b/packages/devcontainer-node/src/parts/ForwardPorts/ForwardPorts.ts @@ -0,0 +1,262 @@ +import type { ChildProcessWithoutNullStreams } from 'node:child_process' +import type { Server, Socket } from 'node:net' +import { spawn } from 'node:child_process' +import { createServer } from 'node:net' +import * as RunProcess from '../RunProcess/RunProcess.ts' +import { nodeRelay, probe, pythonRelay } from './Relay.ts' + +interface Port { + host: string + port: number +} +export interface Options { + containerCli?: string + containerId: string + forwardPorts?: unknown + remoteUser?: string + workspaceFolder?: string +} +interface Forwarding { + children: Set + key: string + servers: Server[] + sockets: Set +} +interface Host { + runProcess: typeof RunProcess.runProcess + spawn: ( + command: string, + args: readonly string[], + ) => ChildProcessWithoutNullStreams +} + +const parsePorts = (value: unknown): Port[] => { + if (value === undefined) return [] + if (!Array.isArray(value)) + throw new TypeError('forwardPorts must be an array') + const ports = new Map() + for (const entry of value) { + const match = + typeof entry === 'string' + ? /^([a-zA-Z0-9_.-]+):(\d+)$/.exec(entry) + : undefined + const host = match ? match[1] : '127.0.0.1' + const port = match ? Number(match[2]) : entry + if ( + typeof port !== 'number' || + !Number.isSafeInteger(port) || + port < 1 || + port > 65_535 + ) { + throw new TypeError(`Invalid forwardPorts entry: ${String(entry)}`) + } + const existing = ports.get(port) + if (existing && existing.host !== host) + throw new Error( + `forwardPorts declares multiple destinations for host port ${port}`, + ) + ports.set(port, { host, port }) + } + return ports.values().toArray() +} + +const close = async (value: Forwarding): Promise => { + for (const socket of value.sockets) socket.destroy() + for (const child of value.children) child.kill() + await Promise.all( + value.servers.map( + (server) => + new Promise((resolve) => { + server.close(() => resolve()) + }), + ), + ) +} + +const relayArguments = ( + executable: string, + host: string, + port: number, +): string[] => { + if (executable.endsWith('socat')) + return [executable, '-', `TCP:${host}:${port}`] + if (executable.endsWith('python3')) + return [executable, '-c', pythonRelay, host, String(port)] + return [executable, '-e', nodeRelay, host, String(port)] +} + +const createListener = ( + dependencies: Host, + value: Forwarding, + containerCli: string, + execArgs: string[], + relayArgs: string[], + port: number, +): Server => { + return createServer({ allowHalfOpen: true }, (socket) => { + value.sockets.add(socket) + const child = dependencies.spawn(containerCli, [...execArgs, ...relayArgs]) + value.children.add(child) + const dispose = () => { + socket.destroy() + child.kill() + } + socket.once('error', dispose) + child.once('error', dispose) + child.stdin.on('error', dispose) + child.stdout.on('error', dispose) + let errorOutput = '' + child.stderr.on('data', (data: Buffer) => { + errorOutput = (errorOutput + data.toString()).slice(-4096) + }) + child.once('close', (code) => { + value.children.delete(child) + if (code) { + console.error(`Devcontainer port ${port} relay failed: ${errorOutput}`) + socket.destroy() + } else socket.end() + }) + socket.once('close', () => { + value.sockets.delete(socket) + child.stdin.end() + child.kill() + }) + socket.pipe(child.stdin) + child.stdout.pipe(socket) + }) +} + +export const createForwardPorts = (dependencies: Host) => { + const forwarding = new Map() + const workspaces = new Map() + const pending = new Map>() + + const stop = async (containerId: string): Promise => { + for (const [workspace, id] of workspaces) { + if (id === containerId) workspaces.delete(workspace) + } + const current = forwarding.get(containerId) + forwarding.delete(containerId) + if (current) await close(current) + } + + const start = async ({ + containerCli = 'docker', + containerId, + forwardPorts, + remoteUser, + workspaceFolder, + }: Options): Promise => { + const ports = parsePorts(forwardPorts) + const key = JSON.stringify({ containerCli, ports, remoteUser }) + if (workspaceFolder) { + const previous = workspaces.get(workspaceFolder) + if (previous && previous !== containerId) await stop(previous) + } + if (forwarding.get(containerId)?.key === key) return + await stop(containerId) + if (ports.length === 0) return + const execArgs = [ + 'exec', + '-i', + ...(remoteUser ? ['--user', remoteUser] : []), + containerId, + ] + const result = await dependencies.runProcess({ + args: [...execArgs, 'sh', '-c', probe], + command: containerCli, + }) + if ('errorMessage' in result || result.exitCode !== 0) { + throw new Error( + 'Cannot forward devcontainer ports: install socat, Node.js or Python 3 in the container. ' + + ('errorMessage' in result ? result.errorMessage : result.stderr), + ) + } + const executable = result.stdout.trim().split('\n', 1)[0] + const value: Forwarding = { + children: new Set(), + key, + servers: [], + sockets: new Set(), + } + try { + for (const { host, port } of ports) { + const server = createListener( + dependencies, + value, + containerCli, + execArgs, + relayArguments(executable, host, port), + port, + ) + value.servers.push(server) + await new Promise((resolve, reject) => { + const failed = (error: Error) => + reject( + new Error( + `Cannot forward devcontainer port ${port} on 127.0.0.1:${port}: ${error.message}. Free this host port and reopen the container.`, + ), + ) + server.once('error', failed) + server.listen(port, '127.0.0.1', () => { + server.off('error', failed) + resolve() + }) + }) + server.on('error', (error) => + console.error(`Devcontainer port ${port}: ${error.message}`), + ) + } + forwarding.set(containerId, value) + if (workspaceFolder) workspaces.set(workspaceFolder, containerId) + } catch (error) { + await close(value) + throw error + } + } + + // Serialize start/stop for each container, including concurrent restoration. + const enqueue = ( + containerId: string, + operation: () => Promise, + ): Promise => { + const previous = pending.get(containerId) ?? Promise.resolve() + const current = (async () => { + try { + await previous + } catch { + // A failed start must not prevent stop or a repaired start. + } + await operation() + })() + pending.set(containerId, current) + void current + .finally(() => { + if (pending.get(containerId) === current) pending.delete(containerId) + }) + .catch(() => {}) + return current + } + + const ensure = (options: Options): Promise => + enqueue(options.containerId, () => start(options)) + const remove = (containerId: string): Promise => + enqueue(containerId, () => stop(containerId)) + + const dispose = (): void => { + for (const value of forwarding.values()) { + for (const socket of value.sockets) socket.destroy() + for (const child of value.children) child.kill() + for (const server of value.servers) server.close() + } + forwarding.clear() + workspaces.clear() + } + + return { dispose, ensure, remove } +} + +export const { dispose, ensure, remove } = createForwardPorts({ + runProcess: RunProcess.runProcess, + spawn: (command, args) => spawn(command, [...args], { stdio: 'pipe' }), +}) diff --git a/packages/devcontainer-node/src/parts/ForwardPorts/Relay.ts b/packages/devcontainer-node/src/parts/ForwardPorts/Relay.ts new file mode 100644 index 0000000..7266b38 --- /dev/null +++ b/packages/devcontainer-node/src/parts/ForwardPorts/Relay.ts @@ -0,0 +1,49 @@ +// Relay programs are passed as arguments, never interpolated into shell source. +// Node/Python are fallbacks for development images that do not include socat. +export const nodeRelay = ` +const net = require('node:net'); +const socket = net.connect({ host: process.argv[1], port: Number(process.argv[2]), allowHalfOpen: true }); +socket.on('error', error => { console.error(error.message); process.exitCode = 1; socket.destroy(); process.stdin.destroy(); }); +socket.on('connect', () => { process.stdin.pipe(socket); socket.pipe(process.stdout); }); +process.stdin.on('end', () => { socket.end(); socket.setTimeout(5000, () => socket.destroy()); }); +socket.on('end', () => process.stdin.destroy()); +socket.on('close', () => process.stdin.destroy()); +` + +export const pythonRelay = ` +import os, select, socket, sys, threading, time +s = socket.create_connection((sys.argv[1], int(sys.argv[2]))) +s.settimeout(None) +finished = threading.Event() +def send(): + try: + while True: + data = os.read(0, 65536) + if not data: + s.shutdown(socket.SHUT_WR) + finished.set() + return + s.sendall(data) + except OSError: + s.close() +threading.Thread(target=send, daemon=True).start() +drain_until = None +try: + while True: + if finished.is_set() and drain_until is None: + drain_until = time.monotonic() + 5 + if drain_until is not None and time.monotonic() >= drain_until: + break + if not select.select([s], [], [], 1)[0]: + continue + data = s.recv(65536) + if not data: break + sys.stdout.buffer.write(data) + sys.stdout.buffer.flush() +finally: + try: s.shutdown(socket.SHUT_RDWR) + except OSError: pass + s.close() +` + +export const probe = 'command -v socat || command -v node || command -v python3' diff --git a/packages/devcontainer-node/test/DevContainerCli.test.ts b/packages/devcontainer-node/test/DevContainerCli.test.ts index bc6fc77..57516f4 100644 --- a/packages/devcontainer-node/test/DevContainerCli.test.ts +++ b/packages/devcontainer-node/test/DevContainerCli.test.ts @@ -28,6 +28,7 @@ test('getCliUpArgs', () => { '--workspace-folder', '/workspace', '--no-lockfile', + '--include-merged-configuration', '--log-format', 'text', '--log-level', diff --git a/packages/devcontainer-node/test/ForwardPorts.e2e.ts b/packages/devcontainer-node/test/ForwardPorts.e2e.ts new file mode 100644 index 0000000..abb57a2 --- /dev/null +++ b/packages/devcontainer-node/test/ForwardPorts.e2e.ts @@ -0,0 +1,226 @@ +import type { Rpc } from '@lvce-editor/rpc' +import type { Duplex } from 'node:stream' +import { + NodeForkedProcessRpcParent, + WebSocketRpcParent, +} from '@lvce-editor/rpc' +import assert from 'node:assert/strict' +import { cp, mkdtemp, rm } from 'node:fs/promises' +import { createServer } from 'node:http' +import { createServer as createTcpServer } from 'node:net' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { fileURLToPath } from 'node:url' +import { dockerRemoveContainer } from '../src/parts/DevContainerCli/DevContainerCli.ts' + +const connectProcess = async (directory: string) => { + const path = fileURLToPath( + new URL('../../../.tmp/dist/dist/devcontainerProcess.js', import.meta.url), + ) + const control = await NodeForkedProcessRpcParent.create({ + commandMap: {}, + env: { + ...process.env, + LVCE_DEVCONTAINER_CONNECTIONS_DIR: join(directory, 'connections'), + }, + path, + }) + const server = createServer() + const sockets = new Set() + let rpc: Rpc | undefined + const dispose = async (): Promise => { + await rpc?.dispose() + await control.dispose() + for (const socket of sockets) socket.destroy() + await new Promise((resolve) => server.close(() => resolve())) + } + try { + const attached = Promise.withResolvers() + server.on('upgrade', (request, socket) => { + sockets.add(socket) + socket.once('close', () => sockets.delete(socket)) + socket.pause() + void control + .invokeAndTransfer('NodeRpcProcess.handleWebSocket', socket, { + headers: request.headers, + method: request.method, + url: request.url, + }) + .then(() => attached.resolve(), attached.reject) + }) + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(0, '127.0.0.1', resolve) + }) + const address = server.address() + assert.ok(address && typeof address === 'object') + const webSocket = new WebSocket(`ws://127.0.0.1:${address.port}`) + rpc = await WebSocketRpcParent.create({ commandMap: {}, webSocket }) + await attached.promise + return { dispose, rpc } + } catch (error) { + await dispose() + throw error + } +} + +const poll = async (check: () => Promise): Promise => { + const deadline = Date.now() + 30_000 + let lastError: unknown + while (Date.now() < deadline) { + try { + await check() + return + } catch (error) { + lastError = error + await new Promise((resolve) => setTimeout(resolve, 100)) + } + } + throw lastError +} + +const shouldRespond = (): Promise => + poll(async () => { + const response = await fetch('http://127.0.0.1:3000/', { + signal: AbortSignal.timeout(2000), + }) + assert.equal(response.status, 200) + assert.equal( + await response.text(), + 'devcontainer forwardPorts acceptance\n', + ) + }) + +const shouldReleasePort = (): Promise => + poll(async () => { + const server = createTcpServer() + try { + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(3000, '127.0.0.1', resolve) + }) + } finally { + await new Promise((resolve) => server.close(() => resolve())) + } + }) + +for (const fixture of ['forward-ports', 'forward-ports-python']) { + await test( + `${fixture}: forwardPorts exposes container localhost and cleans up across reopen, stop, remove and disconnect`, + { timeout: 180_000 }, + async () => { + const directory = await mkdtemp( + join(tmpdir(), 'devcontainer-forward-ports-'), + ) + const workspaceFolder = join(directory, 'workspace') + let connection: Awaited> | undefined + let containerId: string | undefined + try { + await cp( + new URL(`../../e2e/fixtures/${fixture}/`, import.meta.url), + workspaceFolder, + { recursive: true }, + ) + await shouldReleasePort() + connection = await connectProcess(directory) + const opened = await connection.rpc.invoke( + 'DevContainer.openWorkspace', + { + workspaceFolder, + }, + ) + assert.equal(opened.ok, true, JSON.stringify(opened)) + const state = await connection.rpc.invoke('DevContainer.getState', { + workspaceFolder, + }) + ;({ containerId } = state) + assert.deepEqual(state.forwardPorts, [3000]) + await shouldRespond() + const reopened = await connection.rpc.invoke( + 'DevContainer.openWorkspace', + { workspaceFolder: opened.workspaceUri }, + ) + assert.equal(reopened.ok, true, JSON.stringify(reopened)) + await shouldRespond() + + await connection.dispose() + connection = undefined + await shouldReleasePort() + connection = await connectProcess(directory) + const restored = await connection.rpc.invoke('DevContainer.getState', { + workspaceFolder: opened.workspaceUri, + }) + assert.equal(restored.status, 'running') + await shouldRespond() + const stopped = await connection.rpc.invoke('DevContainer.stop', { + workspaceFolder: opened.workspaceUri, + }) + assert.equal(stopped.ok, true, JSON.stringify(stopped)) + await shouldReleasePort() + const restarted = await connection.rpc.invoke( + 'DevContainer.openWorkspace', + { workspaceFolder }, + ) + assert.equal(restarted.ok, true, JSON.stringify(restarted)) + const restartedState = await connection.rpc.invoke( + 'DevContainer.getState', + { workspaceFolder }, + ) + ;({ containerId } = restartedState) + await shouldRespond() + const removed = await connection.rpc.invoke('DevContainer.remove', { + workspaceFolder: restarted.workspaceUri, + }) + assert.equal(removed.ok, true, JSON.stringify(removed)) + await shouldReleasePort() + } catch (error) { + if (connection) { + try { + const state = await connection.rpc.invoke('DevContainer.getState', { + workspaceFolder, + }) + const logs = await connection.rpc.invoke('DevContainer.exec', { + args: ['/tmp/forward-ports.log'], + command: 'cat', + workspaceFolder, + }) + console.error('Forwarding fixture diagnostics', { + containerId: state.containerId, + lastResult: state.lastResult, + logs, + remoteWorkspaceFolder: state.remoteWorkspaceFolder, + }) + } catch (diagnosticError) { + console.error( + 'Could not read forwarding fixture diagnostics', + diagnosticError, + ) + } + } + throw error + } finally { + try { + if (connection) { + // Cleanup also works when startup reports a forwarding error. + const state = await connection.rpc.invoke('DevContainer.getState', { + workspaceFolder, + }) + const { containerId: currentId } = state + containerId = currentId ?? containerId + } + if (containerId) { + const removed = await dockerRemoveContainer({ containerId }) + assert.ok( + removed.ok || removed.stderr?.includes('No such container'), + JSON.stringify(removed), + ) + } + } finally { + await connection?.dispose() + await rm(directory, { force: true, recursive: true }) + } + } + }, + ) +} diff --git a/packages/devcontainer-node/test/ForwardPorts.test.ts b/packages/devcontainer-node/test/ForwardPorts.test.ts new file mode 100644 index 0000000..6135360 --- /dev/null +++ b/packages/devcontainer-node/test/ForwardPorts.test.ts @@ -0,0 +1,244 @@ +import type { Server, Socket } from 'node:net' +import { afterEach, expect, jest, test } from '@jest/globals' +import { spawn as realSpawn } from 'node:child_process' +import { connect, createServer } from 'node:net' + +const children: ReturnType[] = [] +let probeResult = { exitCode: 0, stderr: '', stdout: '/usr/bin/node\n' } +const probe = jest.fn(async () => probeResult) +const spawn = jest.fn((_command: string, args: readonly string[]) => { + const index = args.indexOf('-e') + const child = realSpawn(process.execPath, args.slice(index), { + stdio: 'pipe', + }) + children.push(child) + return child +}) +const { createForwardPorts } = + await import('../src/parts/ForwardPorts/ForwardPorts.ts') +const ForwardPorts = createForwardPorts({ runProcess: probe, spawn }) +const servers: Server[] = [] +const sockets: Socket[] = [] + +const listen = async (server: Server): Promise => { + servers.push(server) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + const address = server.address() + if (!address || typeof address !== 'object') + throw new Error('Missing server address') + return address.port +} +const freePort = async (): Promise => { + const server = createServer() + const port = await listen(server) + await new Promise((resolve) => server.close(() => resolve())) + return port +} +const read = async (port: number, content: Buffer): Promise => { + const socket = connect(port, '127.0.0.1') + sockets.push(socket) + const chunks: Buffer[] = [] + return new Promise((resolve, reject) => { + socket.once('error', reject) + socket.on('data', (chunk) => { + chunks.push(chunk) + }) + socket.once('end', () => resolve(Buffer.concat(chunks))) + socket.end(content) + }) +} + +afterEach(async () => { + await ForwardPorts.remove('one') + await ForwardPorts.remove('two') + ForwardPorts.dispose() + for (const socket of sockets.splice(0)) socket.destroy() + for (const child of children.splice(0)) { + if (!(child.exitCode === null && child.signalCode === null)) { + continue + } + + const closed = new Promise((resolve) => child.once('close', resolve)) + child.kill() + await closed + } + for (const server of servers.splice(0)) + await new Promise((resolve) => server.close(() => resolve())) + probeResult = { exitCode: 0, stderr: '', stdout: '/usr/bin/node\n' } + jest.clearAllMocks() +}) + +test('no configuration does not launch any container commands', async () => { + await ForwardPorts.ensure({ containerId: 'one' }) + expect(probe).not.toHaveBeenCalled() + expect(spawn).not.toHaveBeenCalled() +}) + +test('reports invalid ports and conflicting destinations', async () => { + for (const forwardPorts of [ + [0], + [65_536], + ['3000'], + [1.5], + {}, + ['db:3000', 3000], + ]) { + await expect( + ForwardPorts.ensure({ containerId: 'one', forwardPorts }), + ).rejects.toThrow(/forwardPorts/) + } +}) + +test('relays binary data after a client half-close and reuses concurrent starts', async () => { + const events: string[] = [] + const record = (event: string) => { + events.push(`${Date.now()}: ${event}`) + } + const timer = setTimeout( + () => console.error('Half-close events', events), + 4000, + ) + try { + const target = await listen( + createServer({ allowHalfOpen: true }, (socket) => { + sockets.push(socket) + const chunks: Buffer[] = [] + socket.on('data', (chunk) => { + chunks.push(chunk) + }) + socket.on('end', () => { + record(`target end: ${Buffer.concat(chunks).length} bytes`) + socket.end(Buffer.concat(chunks)) + }) + }), + ) + const port = await freePort() + // Route the real relay to the test service, retaining the production listener. + spawn.mockImplementationOnce((_command, args) => { + const index = args.indexOf('-e') + const relayArgs = args.slice(index) + relayArgs[relayArgs.length - 1] = String(target) + const child = realSpawn(process.execPath, relayArgs, { stdio: 'pipe' }) + child.once('spawn', () => record('relay spawned')) + child.stdin.once('finish', () => record('relay stdin finish')) + child.stdout.once('end', () => record('relay stdout end')) + child.once('close', (code) => record(`relay close: ${code}`)) + child.stderr.on('data', (data) => record(`relay stderr: ${data}`)) + children.push(child) + return child + }) + const options = { + containerCli: 'podman', + containerId: 'one', + forwardPorts: [port, port], + remoteUser: 'node', + } + await Promise.all([ + ForwardPorts.ensure(options), + ForwardPorts.ensure(options), + ]) + expect(probe).toHaveBeenCalledTimes(1) + const payload = Buffer.alloc(1024 * 1024, 0x80) + record('reading binary response') + expect(await read(port, payload)).toEqual(payload) + record('binary response received') + expect(spawn.mock.calls[0].slice(0, 2)).toEqual([ + 'podman', + expect.arrayContaining(['--user', 'node', 'one']), + ]) + record('removing listener') + await ForwardPorts.remove('one') + record('listener removed') + await expect(read(port, Buffer.from('closed'))).rejects.toThrow() + record('closed listener rejected connection') + await ForwardPorts.ensure(options) + record('listener reopened') + await ForwardPorts.remove('one') + } finally { + clearTimeout(timer) + } +}, 10_000) + +test('host conflicts roll back all newly opened listeners and preserve the other owner', async () => { + const occupied = await listen( + createServer((socket) => { + sockets.push(socket) + socket.end('owner') + }), + ) + const first = await freePort() + await expect( + ForwardPorts.ensure({ + containerId: 'one', + forwardPorts: [first, occupied], + }), + ).rejects.toThrow(/Free this host port/) + await expect(read(first, Buffer.from('closed'))).rejects.toThrow() + const response = await read(occupied, Buffer.from('test')) + expect(response.toString()).toBe('owner') +}) + +test('reports a missing relay before opening any listeners', async () => { + probeResult = { exitCode: 127, stderr: 'not found', stdout: '' } + await expect( + ForwardPorts.ensure({ + containerId: 'one', + forwardPorts: [await freePort()], + }), + ).rejects.toThrow(/install socat, Node.js or Python 3/) +}) + +test('disposing an active connection releases its listener and engine process', async () => { + const port = await freePort() + const accepted = Promise.withResolvers() + const engineClosed = Promise.withResolvers() + spawn.mockImplementationOnce(() => { + const child = realSpawn( + process.execPath, + ['-e', 'process.stdin.resume()'], + { stdio: 'pipe' }, + ) + children.push(child) + child.once('close', () => engineClosed.resolve()) + accepted.resolve() + return child + }) + await ForwardPorts.ensure({ containerId: 'one', forwardPorts: [port] }) + const socket = connect(port, '127.0.0.1') + sockets.push(socket) + const closed = new Promise((resolve, reject) => { + // Destroying the peer may deliver a reset before close, particularly on macOS. + socket.once('error', (error: NodeJS.ErrnoException) => { + if (error.code !== 'ECONNRESET') reject(error) + }) + socket.once('close', () => resolve()) + }) + await new Promise((resolve, reject) => { + socket.once('connect', resolve) + socket.once('error', reject) + }) + await accepted.promise + ForwardPorts.dispose() + await closed + await engineClosed.promise + await expect(read(port, Buffer.from('closed'))).rejects.toThrow() + await ForwardPorts.ensure({ containerId: 'one', forwardPorts: [port] }) +}) + +test('replacing a workspace container closes its previous listener', async () => { + const first = await freePort() + const second = await freePort() + await ForwardPorts.ensure({ + containerId: 'one', + forwardPorts: [first], + workspaceFolder: '/workspace', + }) + await ForwardPorts.ensure({ + containerId: 'two', + forwardPorts: [second], + workspaceFolder: '/workspace', + }) + await expect(read(first, Buffer.from('closed'))).rejects.toThrow() + await ForwardPorts.remove('two') + await expect(read(second, Buffer.from('closed'))).rejects.toThrow() +}) diff --git a/packages/devcontainer-worker/src/devcontainerWorkerModule.ts b/packages/devcontainer-worker/src/devcontainerWorkerModule.ts index d95c12b..35dd8ff 100644 --- a/packages/devcontainer-worker/src/devcontainerWorkerModule.ts +++ b/packages/devcontainer-worker/src/devcontainerWorkerModule.ts @@ -7,7 +7,12 @@ import * as DevContainerNodeClient from './parts/DevContainerNodeClient/DevConta import * as GetDockerInstallCommand from './parts/GetDockerInstallCommand/GetDockerInstallCommand.ts' import * as Progress from './parts/Progress/Progress.ts' +let initialized = false const initialize = () => { + if (!initialized) { + process.once('exit', DevContainerCli.disposeForwardPorts) + initialized = true + } DevContainerNodeClient.setNodeApi(DevContainerCli) } diff --git a/packages/devcontainer-worker/src/parts/ConnectionStorage/ConnectionStorage.ts b/packages/devcontainer-worker/src/parts/ConnectionStorage/ConnectionStorage.ts index f9fae0d..df025d1 100644 --- a/packages/devcontainer-worker/src/parts/ConnectionStorage/ConnectionStorage.ts +++ b/packages/devcontainer-worker/src/parts/ConnectionStorage/ConnectionStorage.ts @@ -6,6 +6,7 @@ import { isAbsolute, join } from 'node:path' interface Connection { containerCli?: string containerId: string + forwardPorts?: unknown remoteUser?: string remoteWorkspaceFolder: string workspaceFolder: string diff --git a/packages/devcontainer-worker/src/parts/CreateDevContainer/CreateDevContainer.ts b/packages/devcontainer-worker/src/parts/CreateDevContainer/CreateDevContainer.ts index 6a16fff..5bd83c5 100644 --- a/packages/devcontainer-worker/src/parts/CreateDevContainer/CreateDevContainer.ts +++ b/packages/devcontainer-worker/src/parts/CreateDevContainer/CreateDevContainer.ts @@ -134,6 +134,10 @@ export const createDevContainer = ({ if (cancelled()) return cancellation if (!isOk(result) || !result.ok) { DevContainerState.set(workspaceFolder, { + containerCli, + containerId: isOk(result) + ? getStringProperty(result.json, 'containerId') + : undefined, lastResult: result, status: 'error', }) @@ -150,6 +154,10 @@ export const createDevContainer = ({ DevContainerState.set(workspaceFolder, { containerCli, containerId, + forwardPorts: ( + result.json as + { mergedConfiguration?: { forwardPorts?: unknown } } | undefined + )?.mergedConfiguration?.forwardPorts, lastResult: result, remoteUser, remoteWorkspaceFolder, diff --git a/packages/devcontainer-worker/src/parts/DevContainerNodeClient/DevContainerNodeClient.ts b/packages/devcontainer-worker/src/parts/DevContainerNodeClient/DevContainerNodeClient.ts index 2a205fd..1cb8764 100644 --- a/packages/devcontainer-worker/src/parts/DevContainerNodeClient/DevContainerNodeClient.ts +++ b/packages/devcontainer-worker/src/parts/DevContainerNodeClient/DevContainerNodeClient.ts @@ -38,6 +38,12 @@ export interface NodeApi { containerCli?: string containerId: string }): Promise + forwardPorts?(options: { + containerCli?: string + containerId: string + forwardPorts?: unknown + remoteUser?: string + }): Promise } const missingNodeApi = () => { @@ -122,3 +128,14 @@ export const dockerInspectContainer = ( } return nodeApi.dockerInspectContainer({ containerCli, containerId }) } + +export const forwardPorts = (options: { + containerCli?: string + containerId: string + forwardPorts?: unknown + remoteUser?: string +}): Promise => { + if (!nodeApi.forwardPorts) + return Promise.reject(new Error('Port forwarding is not available')) + return nodeApi.forwardPorts(options) +} diff --git a/packages/devcontainer-worker/src/parts/DevContainerState/DevContainerState.ts b/packages/devcontainer-worker/src/parts/DevContainerState/DevContainerState.ts index 8a3def8..d26c2a4 100644 --- a/packages/devcontainer-worker/src/parts/DevContainerState/DevContainerState.ts +++ b/packages/devcontainer-worker/src/parts/DevContainerState/DevContainerState.ts @@ -6,6 +6,7 @@ type DevContainerStatus = 'error' | 'running' | 'starting' | 'stopped' export interface DevContainerState { containerCli?: string containerId?: string + forwardPorts?: unknown lastResult?: unknown remoteUser?: string remoteWorkspaceFolder?: string @@ -61,13 +62,26 @@ export const restore = async (key: string): Promise => { ) // Another request may already have restored or changed this workspace. if (!state.has(connection.workspaceFolder)) { - state.set(connection.workspaceFolder, { + const restored = set(connection.workspaceFolder, { containerCli: connection.containerCli, containerId: connection.containerId, + ...(connection.forwardPorts !== undefined && { + forwardPorts: connection.forwardPorts, + }), remoteUser: connection.remoteUser, remoteWorkspaceFolder: connection.remoteWorkspaceFolder, status: running ? 'running' : 'stopped', }) + if (running && connection.forwardPorts !== undefined) { + try { + await DevContainerNodeClient.forwardPorts(connection) + } catch (error) { + if (state.get(connection.workspaceFolder) === restored) { + set(connection.workspaceFolder, { ...restored, status: 'error' }) + } + throw error + } + } } return connection.workspaceFolder } @@ -80,6 +94,9 @@ export const persist = async (workspaceFolder: string): Promise => { await ConnectionStorage.save({ containerCli: current.containerCli, containerId: current.containerId, + ...(current.forwardPorts !== undefined && { + forwardPorts: current.forwardPorts, + }), remoteUser: current.remoteUser, remoteWorkspaceFolder: current.remoteWorkspaceFolder, workspaceFolder, diff --git a/packages/devcontainer-worker/test/DevContainer.test.ts b/packages/devcontainer-worker/test/DevContainer.test.ts index 40f4e59..f59b3ae 100644 --- a/packages/devcontainer-worker/test/DevContainer.test.ts +++ b/packages/devcontainer-worker/test/DevContainer.test.ts @@ -293,3 +293,25 @@ test('openWorkspace rejects an inaccessible remote workspace', async () => { 'workspace missing', ) }) + +test('forwarding failures retain the container id for cleanup', async () => { + const workspaceFolder = await createWorkspace() + DevContainerNodeClient.setNodeApi({ + cliExec: async () => ({ ok: true }), + cliReadConfiguration: async () => ({ ok: true }), + cliUp: async () => ({ + errorCode: 'DEVCONTAINER_FORWARD_PORTS_ERROR', + json: { containerId: 'abc123' }, + ok: false, + }), + dockerRemoveContainer: async (options) => { + expect(options.containerId).toBe('abc123') + return { ok: true } + }, + dockerStopContainer: async () => ({ ok: true }), + }) + expect(await DevContainer.up({ workspaceFolder })).toMatchObject({ + ok: false, + }) + expect(await DevContainer.remove({ workspaceFolder })).toEqual({ ok: true }) +}) diff --git a/packages/devcontainer-worker/test/DevContainerState.test.ts b/packages/devcontainer-worker/test/DevContainerState.test.ts index 4c96723..2a0a16d 100644 --- a/packages/devcontainer-worker/test/DevContainerState.test.ts +++ b/packages/devcontainer-worker/test/DevContainerState.test.ts @@ -96,3 +96,82 @@ test('does not redirect an old URI to a replacement container', async () => { 'replacement456', ) }) + +test('restores saved forwarding once for concurrent requests and skips stopped containers', async () => { + const workspaceFolder = join(directory, 'workspace') + const forwarded: unknown[] = [] + DevContainerNodeClient.setNodeApi({ + cliExec: async () => ({}), + cliReadConfiguration: async () => ({}), + cliUp: async () => ({}), + dockerInspectContainer: async () => running, + dockerRemoveContainer: async () => ({}), + dockerStopContainer: async () => ({}), + forwardPorts: async (options) => { + forwarded.push(options) + }, + }) + DevContainerState.set(workspaceFolder, { + containerId: 'abc123', + forwardPorts: [3000], + remoteUser: 'node', + remoteWorkspaceFolder: '/app', + status: 'running', + }) + await DevContainerState.persist(workspaceFolder) + DevContainerState.reset() + await Promise.all([ + DevContainerState.restore('abc123'), + DevContainerState.restore(workspaceFolder), + ]) + expect(forwarded).toEqual([ + expect.objectContaining({ + containerId: 'abc123', + forwardPorts: [3000], + remoteUser: 'node', + workspaceFolder, + }), + ]) + DevContainerState.reset() + running = false + await DevContainerState.restore('abc123') + expect(forwarded).toHaveLength(1) +}) + +test('a stale restoration does not reopen ports after the workspace state changes', async () => { + const workspaceFolder = join(directory, 'workspace') + const inspected = Promise.withResolvers() + let forwards = 0 + DevContainerNodeClient.setNodeApi({ + cliExec: async () => ({}), + cliReadConfiguration: async () => ({}), + cliUp: async () => ({}), + dockerInspectContainer: async () => { + DevContainerState.set(workspaceFolder, { + containerId: 'replacement', + status: 'stopped', + }) + return inspected.promise + }, + dockerRemoveContainer: async () => ({}), + dockerStopContainer: async () => ({}), + forwardPorts: async () => { + forwards++ + }, + }) + DevContainerState.set(workspaceFolder, { + containerId: 'abc123', + forwardPorts: [3000], + remoteWorkspaceFolder: '/app', + status: 'running', + }) + await DevContainerState.persist(workspaceFolder) + DevContainerState.reset() + const restoring = DevContainerState.restore('abc123') + inspected.resolve(true) + await restoring + expect(forwards).toBe(0) + expect(DevContainerState.get(workspaceFolder)?.containerId).toBe( + 'replacement', + ) +}) diff --git a/packages/e2e/fixtures/forward-ports-python/.devcontainer/devcontainer.json b/packages/e2e/fixtures/forward-ports-python/.devcontainer/devcontainer.json new file mode 100644 index 0000000..ee03df4 --- /dev/null +++ b/packages/e2e/fixtures/forward-ports-python/.devcontainer/devcontainer.json @@ -0,0 +1,9 @@ +{ + "name": "Forward Ports Python", + "forwardPorts": [3000], + "build": { + "context": "..", + "dockerfile": "../Dockerfile" + }, + "overrideCommand": false +} diff --git a/packages/e2e/fixtures/forward-ports-python/Dockerfile b/packages/e2e/fixtures/forward-ports-python/Dockerfile new file mode 100644 index 0000000..3b278c6 --- /dev/null +++ b/packages/e2e/fixtures/forward-ports-python/Dockerfile @@ -0,0 +1,3 @@ +FROM python:3.13-slim-bookworm +COPY server.py /server.py +CMD ["python3", "/server.py"] diff --git a/packages/e2e/fixtures/forward-ports-python/server.py b/packages/e2e/fixtures/forward-ports-python/server.py new file mode 100644 index 0000000..c5dc143 --- /dev/null +++ b/packages/e2e/fixtures/forward-ports-python/server.py @@ -0,0 +1,11 @@ +from http.server import BaseHTTPRequestHandler, HTTPServer + + +class Handler(BaseHTTPRequestHandler): + def do_GET(self): + self.send_response(200) + self.end_headers() + self.wfile.write(b"devcontainer forwardPorts acceptance\n") + + +HTTPServer(("127.0.0.1", 3000), Handler).serve_forever() diff --git a/packages/e2e/fixtures/forward-ports/.devcontainer/devcontainer.json b/packages/e2e/fixtures/forward-ports/.devcontainer/devcontainer.json new file mode 100644 index 0000000..23a81f5 --- /dev/null +++ b/packages/e2e/fixtures/forward-ports/.devcontainer/devcontainer.json @@ -0,0 +1,9 @@ +{ + "name": "Forward Ports", + "forwardPorts": [3000], + "build": { + "context": "..", + "dockerfile": "../Dockerfile" + }, + "overrideCommand": false +} diff --git a/packages/e2e/fixtures/forward-ports/Dockerfile b/packages/e2e/fixtures/forward-ports/Dockerfile new file mode 100644 index 0000000..89810cf --- /dev/null +++ b/packages/e2e/fixtures/forward-ports/Dockerfile @@ -0,0 +1,3 @@ +FROM mcr.microsoft.com/devcontainers/javascript-node:24 +COPY server.cjs /server.cjs +CMD ["node", "/server.cjs"] diff --git a/packages/e2e/fixtures/forward-ports/server.cjs b/packages/e2e/fixtures/forward-ports/server.cjs new file mode 100644 index 0000000..9b93dea --- /dev/null +++ b/packages/e2e/fixtures/forward-ports/server.cjs @@ -0,0 +1,7 @@ +const http = require('node:http') +http + .createServer((_request, response) => { + response.setHeader('Access-Control-Allow-Origin', '*') + response.end('devcontainer forwardPorts acceptance\n') + }) + .listen(3000, '127.0.0.1')