diff --git a/src/lib/actions/sandbox/host-aliases.ts b/src/lib/actions/sandbox/host-aliases.ts index a6cb451602d..653e1e6bd81 100644 --- a/src/lib/actions/sandbox/host-aliases.ts +++ b/src/lib/actions/sandbox/host-aliases.ts @@ -3,12 +3,18 @@ import { isIP } from "node:net"; -import { dockerExecFileSync } from "../../adapters/docker"; +import { dockerExecFileSync, dockerSpawnSync } from "../../adapters/docker"; import { CLI_NAME } from "../../cli/branding"; import * as registry from "../../state/registry"; const K3S_CONTAINER = "openshell-cluster-nemoclaw"; const HOST_ALIAS_KUBECTL_TIMEOUT_MS = 10_000; +const HOST_ALIAS_DOCKER_PROBE_TIMEOUT_MS = 5_000; + +type LegacyGatewayProbe = + | { state: "present" } + | { state: "absent" } + | { state: "unknown"; reason: string }; // Drivers that run a per-sandbox direct container (openshell-...) // instead of the legacy k3s gateway. They have no openshell-cluster-nemoclaw @@ -85,11 +91,71 @@ function assertLegacyGatewayHostAliasSupport(sandboxName: string): void { hostAliasesFail([ ` Host aliases are not supported on the '${driver}' driver sandbox '${sandboxName}'.`, " This command edits aliases on the legacy Kubernetes gateway sandbox resource,", - ` which the ${driver} driver does not run (there is no openshell-cluster-nemoclaw container).`, + ` which the ${driver} driver does not run (there is no ${K3S_CONTAINER} container).`, " OpenShell does not yet expose a persistent host-alias API for this driver, and a", " one-time /etc/hosts edit would not survive a sandbox restart or rebuild.", ]); } + // Registry entries from older NemoClaw releases predate the openshellDriver + // field, and a kubernetes-driver sandbox whose legacy gateway never came up + // also slips past the driver branch above. Without this probe both fall + // through to `docker exec openshell-cluster-nemoclaw kubectl ...` and the + // user sees an opaque `Error response from daemon: No such container: + // openshell-cluster-nemoclaw`. Classify the probe result so a docker daemon + // outage, timeout, or permission error does not get misreported as a + // missing gateway container. + const probe = probeLegacyGatewayContainer(); + if (probe.state === "absent") { + const driverLabel = driver ?? "unspecified"; + hostAliasesFail([ + ` Host aliases require the legacy OpenShell gateway container '${K3S_CONTAINER}' to be running.`, + ` The legacy gateway container is not running on this host (sandbox '${sandboxName}', driver: ${driverLabel}).`, + " Newer OpenShell drivers run per-sandbox direct containers instead of the legacy gateway", + " and do not yet expose a persistent host-alias API. A one-time /etc/hosts edit inside the", + " direct container would not survive a sandbox restart or rebuild.", + ]); + } + if (probe.state === "unknown") { + hostAliasesFail([ + ` Could not verify the legacy OpenShell gateway container '${K3S_CONTAINER}'.`, + ` Docker probe failed: ${probe.reason}`, + " Check whether the Docker daemon is reachable with `docker info`.", + ]); + } +} + +function probeLegacyGatewayContainer(): LegacyGatewayProbe { + // `docker ps --filter name=...` accepts only substring or anchored regex + // syntax (`name=^/$`) per the Docker CLI reference, and the + // anchor form is fragile across daemon versions. Mirror the unfiltered + // `docker ps --format '{{.Names}}'` pattern used in + // src/lib/sandbox/privileged-exec.ts and do the exact match in code so + // there is no doubt about substring overlap or anchor support. + const result = dockerSpawnSync(["ps", "--format", "{{.Names}}"], { + stdio: ["ignore", "pipe", "pipe"], + encoding: "utf-8", + timeout: HOST_ALIAS_DOCKER_PROBE_TIMEOUT_MS, + }); + if (result.error) { + const code = (result.error as NodeJS.ErrnoException).code ?? ""; + if (code === "ETIMEDOUT") { + return { state: "unknown", reason: "docker ps timed out" }; + } + return { state: "unknown", reason: `docker ps could not launch: ${result.error.message}` }; + } + if (typeof result.status === "number" && result.status !== 0) { + const stderr = String(result.stderr || "").trim(); + return { + state: "unknown", + reason: stderr || `docker ps exited with status ${result.status}`, + }; + } + const stdout = result.stdout == null ? "" : String(result.stdout); + const present = stdout + .split("\n") + .map((line) => line.trim()) + .some((line) => line === K3S_CONTAINER); + return present ? { state: "present" } : { state: "absent" }; } function validateHostAliasHostname(hostname: string): boolean { @@ -239,12 +305,7 @@ export function listSandboxHostAliases(sandboxName: string): void { } } -export function addSandboxHostAlias( - sandboxName: string, - options: AddSandboxHostAliasOptions = {}, -): void { - assertLegacyGatewayHostAliasSupport(sandboxName); - const dryRun = Boolean(options.dryRun); +function validateAddOptions(options: AddSandboxHostAliasOptions): { hostname: string; ip: string } { const { hostname: rawHostname, ip } = options; if (!rawHostname || !ip) { hostAliasesFail(` Usage: ${CLI_NAME} hosts-add [--dry-run]`); @@ -256,6 +317,28 @@ export function addSandboxHostAlias( if (isIP(ip) === 0) { hostAliasesFail(` Invalid IP address '${ip}'.`); } + return { hostname, ip }; +} + +function validateRemoveOptions(options: RemoveSandboxHostAliasOptions): { hostname: string } { + const { hostname: rawHostname } = options; + if (!rawHostname) { + hostAliasesFail(` Usage: ${CLI_NAME} hosts-remove [--dry-run]`); + } + const hostname = normalizeHostAliasHostname(rawHostname); + if (!validateHostAliasHostname(hostname)) { + hostAliasesFail(` Invalid hostname '${hostname}'.`); + } + return { hostname }; +} + +export function addSandboxHostAlias( + sandboxName: string, + options: AddSandboxHostAliasOptions = {}, +): void { + const dryRun = Boolean(options.dryRun); + const { hostname, ip } = validateAddOptions(options); + assertLegacyGatewayHostAliasSupport(sandboxName); const resource = getSandboxResource(sandboxName); const buildAliases: BuildHostAliases = (currentResource) => { @@ -286,16 +369,9 @@ export function removeSandboxHostAlias( sandboxName: string, options: RemoveSandboxHostAliasOptions = {}, ): void { - assertLegacyGatewayHostAliasSupport(sandboxName); const dryRun = Boolean(options.dryRun); - const { hostname: rawHostname } = options; - if (!rawHostname) { - hostAliasesFail(` Usage: ${CLI_NAME} hosts-remove [--dry-run]`); - } - const hostname = normalizeHostAliasHostname(rawHostname); - if (!validateHostAliasHostname(hostname)) { - hostAliasesFail(` Invalid hostname '${hostname}'.`); - } + const { hostname } = validateRemoveOptions(options); + assertLegacyGatewayHostAliasSupport(sandboxName); const resource = getSandboxResource(sandboxName); const buildAliases: BuildHostAliases = (currentResource) => { diff --git a/test/cli.test.ts b/test/cli.test.ts index 1d6ef9fb58f..092160672e7 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -382,6 +382,7 @@ function writeHostAliasDockerStub( localBin: string, dockerLog: string, hostAliases: { ip: string; hostnames: string[] }[], + { gatewayRunning = true }: { gatewayRunning?: boolean } = {}, ): void { const resource = JSON.stringify({ metadata: { resourceVersion: "123" }, @@ -393,6 +394,10 @@ function writeHostAliasDockerStub( "#!/usr/bin/env bash", `log_file=${JSON.stringify(dockerLog)}`, 'printf "%s\\n" "$@" >> "$log_file"', + 'if [ "$1" = "ps" ]; then', + gatewayRunning ? ' printf "%s\\n" "openshell-cluster-nemoclaw"' : " :", + " exit 0", + "fi", 'if printf "%s\\n" "$@" | grep -q "^get$"; then', ` printf "%s\\n" ${JSON.stringify(resource)}`, "fi", @@ -3096,6 +3101,10 @@ describe("CLI dispatch", () => { "#!/usr/bin/env bash", `log_file=${JSON.stringify(dockerLog)}`, 'printf "%s\\n" "$@" >> "$log_file"', + 'if [ "$1" = "ps" ]; then', + ' printf "%s\\n" "openshell-cluster-nemoclaw"', + " exit 0", + "fi", 'if printf "%s\\n" "$@" | grep -q "^get$"; then', ' printf "%s\\n" \'{"metadata":{"resourceVersion":"123"},"spec":{"podTemplate":{"spec":{"hostAliases":[{"ip":"10.0.0.5","hostnames":["old.local"]}]}}}}\'', "fi", @@ -3112,10 +3121,22 @@ describe("CLI dispatch", () => { expect(r.code).toBe(0); expect(r.out).toContain("Added host alias searxng.local -> 192.168.1.105"); const log = fs.readFileSync(dockerLog, "utf8").trim().split(/\n/); - // The docker invocation must start with the `exec` subcommand. Without - // it, docker parses kubectl's `-n` as a docker flag and exits 125 - // ("unknown shorthand flag: 'n' in -n"). - expect(log[0]).toBe("exec"); + // The docker invocation targeting the legacy gateway container must use + // the `exec` subcommand. Without it, docker parses kubectl's `-n` as a + // docker flag and exits 125 ("unknown shorthand flag: 'n' in -n"). The + // legacy-gateway runtime probe runs `docker ps --format {{.Names}}` + // first, so check the subcommand position relative to `kubectl` rather + // than at index 0, and check that the probe argv has the expected + // unfiltered shape (no fragile `--filter name=^...$` regex anchors). + const psIndex = log.indexOf("ps"); + expect(psIndex).toBe(0); + expect(log[psIndex + 1]).toBe("--format"); + expect(log[psIndex + 2]).toBe("{{.Names}}"); + expect(log).not.toContain("--filter"); + const kubectlIndex = log.indexOf("kubectl"); + expect(kubectlIndex).toBeGreaterThan(psIndex); + expect(log[kubectlIndex - 1]).toBe("openshell-cluster-nemoclaw"); + expect(log[kubectlIndex - 2]).toBe("exec"); expect(log).toContain("patch"); expect(log).toContain("--type=json"); const patch = JSON.parse(log[log.indexOf("-p") + 1]); @@ -3146,6 +3167,10 @@ describe("CLI dispatch", () => { "#!/usr/bin/env bash", `log_file=${JSON.stringify(dockerLog)}`, 'printf "%s\\n" "$@" >> "$log_file"', + 'if [ "$1" = "ps" ]; then', + ' printf "%s\\n" "openshell-cluster-nemoclaw"', + " exit 0", + "fi", 'printf "%s\\n" \'{"metadata":{"resourceVersion":"123"},"spec":{"podTemplate":{"spec":{"hostAliases":[{"ip":"192.168.1.105","hostnames":["searxng.local","search.lan"]}]}}}}\'', ].join("\n"), { mode: 0o755 }, @@ -3160,8 +3185,10 @@ describe("CLI dispatch", () => { expect(r.out).toContain("Host aliases for 'alpha'"); expect(r.out).toContain("192.168.1.105 searxng.local, search.lan"); const log = fs.readFileSync(dockerLog, "utf8").trim().split(/\n/); - expect(log[0]).toBe("exec"); - expect(log).toContain("kubectl"); + const kubectlIndex = log.indexOf("kubectl"); + expect(kubectlIndex).toBeGreaterThan(1); + expect(log[kubectlIndex - 1]).toBe("openshell-cluster-nemoclaw"); + expect(log[kubectlIndex - 2]).toBe("exec"); expect(log).toContain("get"); }); @@ -3184,7 +3211,10 @@ describe("CLI dispatch", () => { expect(r.code).toBe(0); expect(r.out).toContain("Removed host alias searxng.local"); const log = fs.readFileSync(dockerLog, "utf8").trim().split(/\n/); - expect(log[0]).toBe("exec"); + const kubectlIndex = log.indexOf("kubectl"); + expect(kubectlIndex).toBeGreaterThan(1); + expect(log[kubectlIndex - 1]).toBe("openshell-cluster-nemoclaw"); + expect(log[kubectlIndex - 2]).toBe("exec"); expect(log).toContain("patch"); const patch = JSON.parse(log[log.lastIndexOf("-p") + 1]); expect(patch[0]).toEqual({ @@ -3298,6 +3328,10 @@ describe("CLI dispatch", () => { `get_count=${JSON.stringify(getCount)}`, `patch_count=${JSON.stringify(patchCount)}`, 'printf "%s\\n" "$@" >> "$log_file"', + 'if [ "$1" = "ps" ]; then', + ' printf "%s\\n" "openshell-cluster-nemoclaw"', + " exit 0", + "fi", 'if printf "%s\\n" "$@" | grep -q "^get$"; then', ' count=$(cat "$get_count" 2>/dev/null || echo 0)', " count=$((count + 1))", @@ -3381,6 +3415,226 @@ describe("CLI dispatch", () => { }); } + it( + "fails host alias commands with an actionable error when the legacy gateway container is not running", + testTimeoutOptions(30_000), + () => { + // A sandbox onboarded by an older NemoClaw release whose registry + // entry predates the openshellDriver field, on a host where the + // legacy `openshell-cluster-nemoclaw` k3s gateway is not running. + // Without the runtime probe, `docker exec openshell-cluster-nemoclaw + // kubectl ...` bubbles up an opaque `Error response from daemon: No + // such container: openshell-cluster-nemoclaw` to the user. + const home = fs.mkdtempSync( + path.join(os.tmpdir(), "nemoclaw-cli-hosts-no-gateway-"), + ); + const localBin = path.join(home, "bin"); + const dockerLog = path.join(home, "docker.log"); + fs.mkdirSync(localBin, { recursive: true }); + writeHostAliasDockerStub( + localBin, + dockerLog, + [{ ip: "10.0.0.5", hostnames: ["old.local"] }], + { gatewayRunning: false }, + ); + // Registry omits openshellDriver to mimic a pre-feature sandbox entry. + writeSandboxRegistry(home); + + const env = { HOME: home, PATH: `${localBin}:${process.env.PATH || ""}` }; + const list = runWithEnv("alpha hosts-list", env); + const add = runWithEnv("alpha hosts-add searxng.local 192.168.1.105", env); + const remove = runWithEnv("alpha hosts-remove searxng.local", env); + + for (const result of [list, add, remove]) { + expect(result.code).toBe(1); + expect(result.out).toContain( + "Host aliases require the legacy OpenShell gateway container 'openshell-cluster-nemoclaw' to be running.", + ); + expect(result.out).not.toContain("Error response from daemon"); + expect(result.out).not.toContain("No such container"); + } + + const log = fs.readFileSync(dockerLog, "utf8").trim().split(/\n/); + // Probe argv must be the unfiltered `docker ps --format {{.Names}}` + // shape. No exec/get/patch reached the missing container. + expect(log[0]).toBe("ps"); + expect(log[1]).toBe("--format"); + expect(log[2]).toBe("{{.Names}}"); + expect(log).not.toContain("--filter"); + expect(log).not.toContain("exec"); + expect(log).not.toContain("kubectl"); + expect(log).not.toContain("get"); + expect(log).not.toContain("patch"); + }, + ); + + it( + "validates host alias arguments before probing the legacy gateway", + testTimeoutOptions(30_000), + () => { + // Arg validation (missing args, bad hostname, bad IP) must run before + // the legacy-gateway probe, so a missing legacy gateway never masks + // an invalid-input failure that would otherwise reach the user. + const home = fs.mkdtempSync( + path.join(os.tmpdir(), "nemoclaw-cli-hosts-validate-first-"), + ); + const localBin = path.join(home, "bin"); + const dockerLog = path.join(home, "docker.log"); + fs.mkdirSync(localBin, { recursive: true }); + writeHostAliasDockerStub(localBin, dockerLog, [], { gatewayRunning: false }); + writeSandboxRegistry(home); + + const env = { HOME: home, PATH: `${localBin}:${process.env.PATH || ""}` }; + + const badHostnameAdd = runWithEnv("alpha hosts-add invalid_name!! 1.2.3.4", env); + expect(badHostnameAdd.code).toBe(1); + expect(badHostnameAdd.out).toContain("Invalid hostname 'invalid_name!!'"); + expect(badHostnameAdd.out).not.toContain("Host aliases require the legacy"); + + const badIpAdd = runWithEnv("alpha hosts-add searxng.local not-an-ip", env); + expect(badIpAdd.code).toBe(1); + expect(badIpAdd.out).toContain("Invalid IP address 'not-an-ip'"); + expect(badIpAdd.out).not.toContain("Host aliases require the legacy"); + + const badHostnameRemove = runWithEnv("alpha hosts-remove invalid_name!!", env); + expect(badHostnameRemove.code).toBe(1); + expect(badHostnameRemove.out).toContain("Invalid hostname 'invalid_name!!'"); + expect(badHostnameRemove.out).not.toContain("Host aliases require the legacy"); + + // No docker probe runs when validation fails up front. + expect(fs.existsSync(dockerLog)).toBe(false); + }, + ); + + it( + "classifies docker spawn ENOENT distinctly from a missing gateway", + testTimeoutOptions(30_000), + () => { + // When the docker binary is absent from PATH, spawnSync returns + // error.code === "ENOENT". The probe must surface a docker-could- + // not-launch error rather than the legacy-gateway-missing error. + const home = fs.mkdtempSync( + path.join(os.tmpdir(), "nemoclaw-cli-hosts-docker-enoent-"), + ); + const emptyBin = path.join(home, "nodocker"); + fs.mkdirSync(emptyBin, { recursive: true }); + // The shell that execSync forks needs to find `node`. Symlink the + // running node executable into the otherwise-empty bin so the shell + // can launch the CLI; docker remains absent from this PATH so the + // CLI's `spawnSync("docker", ...)` returns ENOENT. + fs.symlinkSync(process.execPath, path.join(emptyBin, "node")); + writeSandboxRegistry(home); + + const env = { HOME: home, PATH: emptyBin }; + const list = runWithEnv("alpha hosts-list", env); + expect(list.code).toBe(1); + expect(list.out).toContain( + "Could not verify the legacy OpenShell gateway container 'openshell-cluster-nemoclaw'.", + ); + expect(list.out).toContain("Docker probe failed:"); + expect(list.out).toContain("could not launch"); + expect(list.out).not.toContain( + "Host aliases require the legacy OpenShell gateway container 'openshell-cluster-nemoclaw' to be running.", + ); + }, + ); + + it( + "classifies docker probe timeouts distinctly from a missing gateway", + testTimeoutOptions(60_000), + () => { + // When `docker ps` hangs past the probe timeout, spawnSync kills it + // and reports ETIMEDOUT (or a terminating SIGTERM with no exit). + // The probe must surface a docker-timed-out error rather than the + // legacy-gateway-missing error. + const home = fs.mkdtempSync( + path.join(os.tmpdir(), "nemoclaw-cli-hosts-docker-timeout-"), + ); + const localBin = path.join(home, "bin"); + const dockerLog = path.join(home, "docker.log"); + fs.mkdirSync(localBin, { recursive: true }); + fs.writeFileSync( + path.join(localBin, "docker"), + [ + "#!/usr/bin/env bash", + `log_file=${JSON.stringify(dockerLog)}`, + 'printf "%s\\n" "$@" >> "$log_file"', + 'if [ "$1" = "ps" ]; then', + " sleep 20", + " exit 0", + "fi", + "exit 0", + ].join("\n"), + { mode: 0o755 }, + ); + writeSandboxRegistry(home); + + const env = { HOME: home, PATH: `${localBin}:${process.env.PATH || ""}` }; + const list = runWithEnv("alpha hosts-list", env, 45_000); + expect(list.code).toBe(1); + expect(list.out).toContain( + "Could not verify the legacy OpenShell gateway container 'openshell-cluster-nemoclaw'.", + ); + expect(list.out).toContain("Docker probe failed:"); + expect(list.out).not.toContain( + "Host aliases require the legacy OpenShell gateway container 'openshell-cluster-nemoclaw' to be running.", + ); + + const log = fs.readFileSync(dockerLog, "utf8").trim().split(/\n/); + expect(log[0]).toBe("ps"); + expect(log).not.toContain("exec"); + expect(log).not.toContain("kubectl"); + }, + ); + + it( + "classifies docker probe failures distinctly from a missing gateway", + testTimeoutOptions(30_000), + () => { + // When `docker ps` itself fails (daemon down, permission denied, + // timeout), the user must see a docker-probe-failed error rather than + // the legacy-gateway-missing error. + const home = fs.mkdtempSync( + path.join(os.tmpdir(), "nemoclaw-cli-hosts-docker-down-"), + ); + const localBin = path.join(home, "bin"); + const dockerLog = path.join(home, "docker.log"); + fs.mkdirSync(localBin, { recursive: true }); + fs.writeFileSync( + path.join(localBin, "docker"), + [ + "#!/usr/bin/env bash", + `log_file=${JSON.stringify(dockerLog)}`, + 'printf "%s\\n" "$@" >> "$log_file"', + 'if [ "$1" = "ps" ]; then', + ' printf "Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?\\n" >&2', + " exit 1", + "fi", + "exit 0", + ].join("\n"), + { mode: 0o755 }, + ); + writeSandboxRegistry(home); + + const env = { HOME: home, PATH: `${localBin}:${process.env.PATH || ""}` }; + const list = runWithEnv("alpha hosts-list", env); + expect(list.code).toBe(1); + expect(list.out).toContain( + "Could not verify the legacy OpenShell gateway container 'openshell-cluster-nemoclaw'.", + ); + expect(list.out).toContain("Docker probe failed:"); + expect(list.out).toContain("docker info"); + expect(list.out).not.toContain( + "Host aliases require the legacy OpenShell gateway container 'openshell-cluster-nemoclaw' to be running.", + ); + + const log = fs.readFileSync(dockerLog, "utf8").trim().split(/\n/); + expect(log[0]).toBe("ps"); + expect(log).not.toContain("exec"); + expect(log).not.toContain("kubectl"); + }, + ); + it("supports oclif-native sandbox command forms", () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-cli-native-sandbox-")); writeSandboxRegistry(home);