Add NerdctlClient for Finch and nerdctl support#327
Conversation
This adds full support for AWS Finch as a container runtime, enabling the VS Code Docker extension to work with Finch-managed containers. ## What is Finch? Finch is an open source container development tool from AWS that provides a simple, native client for building, running, and managing containers. It uses containerd and nerdctl under the hood. - Homepage: https://aws.amazon.com/finch/ - GitHub: https://github.com/runfinch/finch - Documentation: https://runfinch.com/docs/ ## Implementation Details ### Core Client (`FinchClient.ts`) - Extends PodmanLikeClient since Finch/nerdctl share similar CLI patterns - Handles containerd-native event format (different from Docker's format) - Implements file read/write using temp file workaround (Finch's cp doesn't support stdio streaming yet) - Custom port exposure argument handling for nerdctl compatibility ### Event Stream Support Finch outputs containerd native events, NOT Docker-compatible events: - Uses `Topic` field (e.g., `/containers/create`) instead of `Type`/`Action` - Event payload is a nested JSON string in the `Event` field - Client-side filtering for types, actions, since/until parameters ### Zod Schemas for CLI Output Parsing - `FinchListContainerRecord` - Container listing - `FinchListImageRecord` - Image listing - `FinchListNetworkRecord` - Network listing - `FinchInspectContainerRecord` - Container inspection - `FinchInspectImageRecord` - Image inspection with date validation - `FinchInspectNetworkRecord` - Network inspection - `FinchInspectVolumeRecord` - Volume inspection - `FinchVersionRecord` - Version information - `FinchEventRecord` - containerd native event format ### Compose Support (`FinchComposeClient.ts`) Uses Finch's built-in compose command (`finch compose`) which is provided by nerdctl's compose implementation. ## Testing E2E tests updated to support Finch via `CONTAINER_CLIENT=finch` env var. All container operations tested and passing: - Container lifecycle (create, start, stop, remove, exec, attach, logs) - Image operations (pull, build, tag, push, remove, inspect) - Network and volume management - File system operations (stat, read, write) using temp file workaround - Event streaming with containerd format parsing - Compose operations (up, down, start, stop, restart, logs, config) - Login/logout with registry credentials ### Known Limitations - Context commands skipped (Docker-only feature) - File streaming uses temp files (Finch cp doesn't support stdin/stdout) - Event filtering done client-side (Finch doesn't support --since/--until) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@microsoft-github-policy-service agree |
Fixes based on CODEX_REVIEW.md recommendations:
**High-Priority Fixes:**
1. Fix parseEventTimestamp - relative time math was inverted
- "1m" now correctly means 1 minute ago (in the past)
- "-1s" now correctly means 1 second from now (in the future)
2. Fix inspect parsing for multi-target calls
- Added parseJsonArrayOrLines() helper to handle both:
- JSON arrays (nerdctl default behavior)
- Newline-separated JSON objects (when --format used)
3. Fix shell injection risks in readFile/writeFile
- Use /bin/sh instead of bash for portability
- Added shellEscapeSingleQuote() for proper path escaping
- Properly escape container paths and command names
**Medium-Priority Improvements:**
4. Fix incorrect override parameter types
- Changed parseInspectNetworksCommandOutput to use InspectNetworksCommandOptions
- Changed parseInspectVolumesCommandOutput to use InspectVolumesCommandOptions
5. Improve epoch fallbacks
- Use new Date() instead of new Date(0) as fallback (less misleading)
- In strict mode, throw errors for missing/invalid dates
6. Fix volume label parsing
- Use parseDockerLikeLabels() to properly handle "key=value" string format
7. Deduplicate size parsing
- Use tryParseSize() utility in FinchListImageRecord for consistency
8. Handle unsupported labels filter in getEventStream
- Throw CommandNotSupportedError when labels filter is provided
- Document limitation clearly in JSDoc
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Given that Finch is basically just nerdctl, I think it would make more sense for this to be a nerdctl client, but with the ability to accept a different configured command name (i.e. nerdctl support has also been requested previously: microsoft/vscode-containers#119 Can you rename the classes/etc. accordingly? |
Since Finch is essentially a wrapper around nerdctl, rename the client
to NerdctlClient for broader applicability. The client now:
- Defaults to 'nerdctl' command (was 'finch')
- Accepts configurable command name via constructor parameter
- Supports both nerdctl and finch (via `new NerdctlClient('finch')`)
This change addresses review feedback and enables support for both
direct nerdctl users and AWS Finch users with the same codebase.
Renamed files:
- FinchClient → NerdctlClient
- FinchComposeClient → NerdctlComposeClient
- All supporting record/schema files
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
f49d1cc to
63b293a
Compare
- NerdctlComposeClient: Override getUpCommandArgs to exclude unsupported flags (--timeout, --no-start, --wait, --watch) - NerdctlComposeClient: Override getDownCommandArgs to exclude --timeout - Export withCommonOrchestratorArgs and withComposeArg from base class - Update orchestrator E2E tests: - Fix version check regex to match docker/podman/nerdctl compose outputs - Skip --no-start tests for finch (not supported) - Skip config --images/--profiles/--volumes for non-Docker clients - Use SIGTERM-responsive containers for faster shutdown Test results: - Docker: 55 passing, 2 pending - Podman: 46 passing, 11 pending - Finch: 40 passing, 10 pending, 7 failing (pre-existing nerdctl issues) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
In nerdctl/finch, containers that exit immediately lose their port mappings and can't be used for exec/filesystem operations. Fixed by: - Add entrypoint/command to keep test containers running - Use SIGTERM trap for fast graceful shutdown - Fix LogsForContainerCommand to use simple echo entrypoint All tests now pass for Docker, Podman, and Finch. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Thanks @bwateratmsft! Great suggestion - you're right that Finch is essentially nerdctl under the hood. I've refactored as suggested:
The client can now support any nerdctl-based CLI: // For Finch
new NerdctlClient('finch', 'Finch', 'Runs container commands using the Finch CLI');
// For nerdctl directly
new NerdctlClient(); // defaults to 'nerdctl'I also added overrides in All E2E tests pass for Docker, Podman, and Finch. |
- Use Zod transform to parse nested Event JSON string during schema validation - Replace passthrough() with looseObject() per Zod v4 migration guide - Rename parseContainerdEventPayload to getActorFromEventPayload since Event is now pre-parsed - Simplify actor extraction logic using nullish coalescing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Create ZodTransforms.ts with reusable transformation schemas: - dateStringSchema / dateStringWithFallbackSchema for date parsing - booleanStringSchema for "true"/"false" string to boolean - labelsStringSchema / labelsSchema for Docker-like label parsing - osTypeStringSchema, architectureStringSchema for enum normalization - protocolStringSchema, numericStringSchema, containerStateStringSchema Apply transforms to Nerdctl record files: - NerdctlListNetworkRecord: boolean strings, labels, dates - NerdctlInspectVolumeRecord: labels, dates - NerdctlInspectNetworkRecord: dates - NerdctlListContainerRecord: labels - NerdctlListImageRecord: dates - NerdctlInspectContainerRecord: dates - NerdctlInspectImageRecord: dates, architecture, OS This moves transformation logic from normalize functions into Zod schemas, leveraging Zod's transform() for cleaner, more declarative parsing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use UTC fallback in dateStringWithFallbackSchema for consistency - Validate EventType and EventAction against schemas before returning from parseContainerdTopic to ensure type safety Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Apologies for how long it's taken to get to reviewing this--I haven't forgotten though! |
- Revert spurious package-lock.json changes and drop CODEX_REVIEW.md - NerdctlComposeClient: hard-code composeV2 = true (drop constructor arg) - ZodTransforms: remove unused schemas (numeric/protocol/portProtocol/ containerState/optionalTransform); use z.stringbool() for booleans; route date parsing through the shared utils/dayjs wrapper so Docker-style list timestamps parse consistently - Extract repeated keep-alive command into a shared KeepAliveShellCommand constant used by both E2E suites - Fix pre-existing lint failures in PR-added files (unsafe JSON.parse assignment, missing curly braces) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
Cover the pure/parsing logic introduced for nerdctl and Finch support: - withNerdctlExposedPortsArg: -p emission gated on publishAllPorts + exposePorts - NerdctlEventRecord: parseContainerdTopic, getActorFromEventPayload, schema - ZodTransforms: date (ISO + Docker-style), boolean, labels, osType, architecture - NerdctlComposeClient: composeV2 prefix + dropping unsupported --timeout on up/down Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
The nerdctl list/inspect parse methods duplicated the same split/parse/ normalize/try-catch loop already present (in slightly different shapes) in DockerClientBase and PodmanClient. Extract three additive, generic protected helpers on the base and route NerdctlClient through them: - parsePerLineJson: newline-delimited JSON (list commands) - parseJsonArrayOrLines: array-or-newline splitting (moved off NerdctlClient) - parseInspectJson: inspect output normalization No existing Docker/Podman behavior changes (helpers are purely additive). NerdctlClient.ts shrinks from 751 to 561 lines. Adds direct unit coverage for the new helpers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
Address correctness issues surfaced by a multi-model review: - DockerClientBase: honor `strict` in parseJsonArrayOrLines/parseInspectJson so malformed inspect JSON propagates instead of being silently dropped. - DockerClientBase: trim lines in parsePerLineJson so CRLF output no longer crashes strict-mode parsing on bare "\r" lines. - NerdctlEventRecord: read `container_id`/`containerId` from containerd task events (task/start, task/exit, ...) so the actor id is populated for container lifecycle events, not just `id`/`key`. - NerdctlInspectContainerRecord: tolerate unrecognized mount types (tmpfs, npipe) via nullable().catch(null) so a single unknown mount no longer drops the entire container from inspect results. - NerdctlClient readFile: name the tar entry after the path basename to match Docker's `cp <container>:<path> -` behavior; add EXIT traps to readFile and writeFile so temp dirs are cleaned up even when cp/tar fails. Adds/extends unit tests for each fix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
Per review feedback, split newline-delimited output on the /\r?\n/ regex so CRLF line endings are handled at the split boundary, instead of relying on a per-line trim(). Applies to parsePerLineJson and parseJsonArrayOrLines in DockerClientBase. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
Brings in the zod/v4 -> zod/mini migration (microsoft#337) and other main changes. New NerdctlClient/ZodTransforms files are converted to zod/mini in a follow-up commit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
Main's PR microsoft#337 migrated vscode-container-client from `zod/v4` to `zod/mini`. Convert all NerdctlClient record schemas and the shared ZodTransforms helpers to match, per maintainer review request. - Switch `import { z } from 'zod/v4'` to `import * as z from 'zod/mini'`. - Rewrite chained builder calls to mini's functional form: `.optional()`/`.nullable()` -> `z.optional()`/`z.nullable()`, `.transform()` -> `z.pipe(schema, z.transform())`, `.catch(null)` -> `z.catch(schema, null)`. - Behavior preserved (dates, labels, booleans, mount catch-to-null, event payload parsing). Build, lint, and 108 unit tests pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
…3.0) nerdctl 2.3.0 added stdin/stdout streaming support for `cp` via `-` (containerd/nerdctl#4704, first released in v2.3.0). That makes the DockerClientBase `cp <container>:<path> -` / `cp - <container>:<path>` implementations work as-is for nerdctl, so the bespoke `/bin/sh` tar-to-temp-file workarounds are no longer needed. - Remove the NerdctlClient readFile/writeFile overrides and the shellEscapeSingleQuote helper; inherit the base implementations. - Drop now-unused imports (byteStreamToGenerator, ReadFile/WriteFile command options, GeneratorCommandResponse, VoidCommandResponse). - This also removes the previous Windows-host `/bin/sh` limitation, since the base path shells out to the container CLI directly. Net -93 lines. Requires nerdctl >= 2.3.0 for file operations; older nerdctl (and Finch bundles still on 2.2.x) no longer support `cp` via this client. Build, lint, and 108 unit tests pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
The existing WriteFileCommand E2E test only exercised the host-side `inputFile` copy path. The newly-relied-upon stdin path (`cp - <container>:<path>`, which NerdctlClient now inherits from DockerClientBase after dropping the `/bin/sh` override) had no dedicated assertion. Add a "WriteFileCommand (streamed to stdin)" case that obtains a real tarball by reading a seeded file back out of the container, then streams those bytes into a different directory via writeFile without `inputFile`, and reads it back to confirm the streamed write created the file. Avoids adding a tar dependency by reusing the readFile-produced tarball. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
One-off integration test to validate the stdin `cp -` write path against a real Docker runtime in a codespace, without adding a tar library. Obtains a genuine tarball via readFile, streams it into /root via writeFile stdin, and reads it back. Run: cd packages/vscode-container-client && npx mocha --grep "TEMP stdin writeFile" To be removed before merge. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
Fixes made while getting the E2E suite green against a real runtime in a codespace: - shellStream: correct stdin piping for the `cp -` write path - parseDockerRawPortString: handle additional port-string shapes - ContainersClientE2E / ContainerOrchestratorClientE2E: test adjustments - parseDockerRawPortString.test: update expectations - launch.json: add `nerdctl` as a selectable container runtime Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
The E2E suites started the long-running test containers with
`runContainer({ detached: true, entrypoint: 'sh', command: ['-c', <loop>] })`.
`runContainer` allocates a TTY for detached runs, and a non-interactive
`sh -c <loop>` exits (via the SIGTERM trap firing) once that pseudo-TTY is
torn down after the CLI detaches with stdin ignored. The container therefore
exited immediately, cascading into failures for InspectContainersCommand
(no published ports on a stopped container), ExecContainerCommand,
ListFilesCommand and StatPathCommand.
Run `tail -f /dev/null` as PID 1 for the `runContainer` keep-alive instead,
which stays alive across every runtime and is still stopped promptly by
`docker stop` (SIGKILL after the timeout). The `KeepAliveShellCommand`
constant is retained for the orchestrator compose files, where the process
runs under `sh -c` without a TTY and benefits from the fast SIGTERM trap.
Also fix two lint errors introduced with the codespace fixes:
- parseDockerRawPortString: drop the needless `\[` escape inside a character
class.
- shellStream: remove the unsafe `throw` inside a `finally` block; the early
-exit cleanup only needs to terminate the child, and normal-completion
errors are already surfaced by the awaited process promise.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
StdinWriteFile.temp.test.ts was a throwaway standalone integration test used to validate the native nerdctl `cp -` stdin streaming path on Unix. The permanent coverage now lives in ContainersClientE2E.test.ts as "WriteFileCommand (streamed to stdin)", so the temporary test is no longer needed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
- Event stream `until`: `nerdctl events` has no `--until`, so it is emulated client-side. Previously the stream only stopped when an event newer than `until` arrived, so a quiet stream would never terminate. Arm a wall-clock timer that closes the readline interface once `untilTimestamp` elapses (matching Docker's `events --until`), and clear it in `finally` so it can't leak or fire after the stream ends. - ListVolumes: drop redundant label/date parsing. The NerdctlInspectVolumeRecordSchema already normalizes Labels (string -> record via labelsSchema) and CreatedAt (string -> Date via dateStringWithFallback), so the `typeof Labels === 'string'` branch was unreachable and the extra `dayjs.utc(...)` re-parsed an already-parsed Date. Rely on the schema output directly and remove the now-unused `dayjs` and `parseDockerLikeLabels` imports. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
Move the canonical comma-aware label parser into `contracts/ZodTransforms`
as `parseLabelsString` and point `labelsStringSchema` at it. The client-side
`parseDockerLikeLabels` now delegates to that single implementation.
Previously `labelsStringSchema` used a naive `split(',')` reducer that
dropped continuation fragments, corrupting label values that contain commas
(e.g. `com.docker.compose.project.config_files` listing multiple files) --
the exact bug fixed for `parseDockerLikeLabels` in microsoft#381. Unifying on the
fixed parser removes that duplicated, divergent logic and gives every schema
consumer (nerdctl volume/inspect/event paths) the correct behavior.
Keeps the contracts -> clients layering intact: the client helper imports
from contracts, never the reverse.
Adds schema-level tests locking in comma- and equals-in-value handling.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
The refactored `parseDockerRawPortString` long-form regex only accepted a host with no colons (`[^:\s[\]]+`) or a bracketed IPv6 host, so Docker's bare IPv6 forms -- notably the common `:::8080->80/tcp` unspecified-address wildcard and `::1:8080->80/tcp` -- no longer matched and returned `undefined`. Because this parser is shared by the existing Docker `ps` list path, IPv6 port bindings were being silently dropped (or throwing in strict mode). Capture the optional host lazily up to the last `:` before the host port so embedded IPv6 colons are preserved; brackets, when present, are stripped by `normalizeIpAddress`. All previously supported forms still parse identically. Adds regression tests for the `:::`, `[::]:`, and bare `::1:` forms. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec267aca-911e-4a63-ac08-728d65ca1605
Summary
This PR adds support for Finch and other nerdctl-based container runtimes, enabling the VS Code Docker extension to work with these alternative container engines.
Architecture
Following reviewer feedback, the implementation uses a generic
NerdctlClientwith configurable command name rather than a Finch-specific client. This allows supporting any nerdctl-based CLI:What is Finch?
Finch is an open source tool for local container development, originally released by AWS in November 2022. It uses containerd and nerdctl under the hood.
Documentation:
Key Implementation Details
containerd Native Events
Finch/nerdctl outputs containerd native events (NOT Docker-compatible):
{ "Timestamp": "2026-01-10T23:38:26.737324778Z", "Topic": "/containers/create", "Event": "{\"id\":\"...\",\"image\":\"...\"}" }The client translates between these formats and performs client-side filtering since nerdctl doesn't support
--since/--untilflags.Compose Limitations
NerdctlComposeClient overrides command generation to exclude unsupported flags:
up: excludes--timeout,--no-start,--wait,--watchdown: excludes--timeoutconfig: only--servicesis supported (not--images,--profiles,--volumes)File Operations Workaround
Nerdctl's
cpcommand advertises stdin/stdout support (-) but it's not fully implemented. This PR uses a temp file workaround.Test Results
All E2E tests pass for all three container clients:
Test Coverage
Known Limitations (reflected in pending tests)
🤖 Generated with Claude Code