fix(docker-host): pass through loopback TCP DOCKER_HOST for ARC/DinD orchestration#4832
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates AWF’s Docker-host handling to support ARC RunnerScaleSet + DinD sidecars where DOCKER_HOST=tcp://localhost:2375 is the standard endpoint, ensuring AWF’s own docker/compose orchestration can reach the sidecar daemon instead of incorrectly falling back to a missing /var/run/docker.sock.
Changes:
- Allow loopback TCP Docker hosts (
tcp://localhost:*,tcp://127.0.0.1:*) incheckDockerHost()and--docker-hostvalidation. - Stop clearing
DOCKER_HOSTfor AWF’s own docker CLI operations (intended to preserve ARC/DinD loopback TCP). - Update docs and tests to reflect the new accepted Docker host forms and behaviors.
Show a summary per file
| File | Description |
|---|---|
| src/types/container-image-options.ts | Updates awfDockerHost JSDoc to document loopback TCP support. |
| src/option-parsers.ts | Adds loopback TCP detection and treats loopback TCP as a valid Docker host. |
| src/option-parsers-misc.test.ts | Expands unit tests for loopback TCP validity and DinD hinting behavior. |
| src/docker-manager-lifecycle.test.ts | Updates lifecycle tests to expect loopback TCP DOCKER_HOST passthrough. |
| src/docker-host.ts | Changes getLocalDockerEnv() behavior to preserve DOCKER_HOST (no longer clearing TCP). |
| src/commands/validators/network-options.ts | Updates validator comments/warnings around external Docker hosts and DinD hints. |
| src/commands/validators/config-assembly.ts | Allows --docker-host to accept loopback TCP URIs. |
| src/commands/validators/config-assembly.test.ts | Updates mocks and adds test coverage for loopback TCP acceptance/rejection. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 4
| if (awfDockerHostOverride !== undefined) { | ||
| // Explicit CLI override — always use this socket for AWF operations | ||
| // Explicit CLI override — always use this value for AWF operations | ||
| env.DOCKER_HOST = awfDockerHostOverride; | ||
| } else { | ||
| const dockerHost = env.DOCKER_HOST; | ||
| if (dockerHost && !dockerHost.startsWith('unix://')) { | ||
| // Non-unix DOCKER_HOST (e.g. tcp://localhost:2375 from a DinD sidecar). | ||
| // Clear it so AWF's docker commands target the local daemon, not the DinD one. | ||
| delete env.DOCKER_HOST; | ||
| } | ||
| } | ||
| // Otherwise, preserve whatever DOCKER_HOST is set in the environment. |
| function isLoopbackTcpDockerHost(dockerHost: string): boolean { | ||
| if (!dockerHost.startsWith('tcp://')) return false; | ||
| const rest = dockerHost.slice('tcp://'.length); | ||
| return rest.startsWith('localhost:') || rest.startsWith('localhost/') || | ||
| rest === 'localhost' || | ||
| rest.startsWith('127.0.0.1:') || rest.startsWith('127.0.0.1/') || | ||
| rest === '127.0.0.1'; | ||
| } |
| * When not set, AWF uses the current `DOCKER_HOST` environment variable | ||
| * unchanged. Loopback TCP and unix:// values are passed through as-is; | ||
| * non-loopback TCP endpoints emit a warning and fall back to the default | ||
| * socket. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@copilot address review feedback |
This comment has been minimized.
This comment has been minimized.
|
@copilot upgrade and recompile workflows |
Done — ran |
|
@copilot resolve the merge conflicts in this pull request |
…st-deletion-issue # Conflicts: # .github/workflows/smoke-chroot.lock.yml
Done — merged |
🧪 Smoke Test: Copilot PAT — PASS
PR: fix(docker-host): pass through loopback TCP DOCKER_HOST for ARC/DinD orchestration Note 🔒 Integrity filter blocked 1 itemThe following item was blocked because it doesn't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
🔬 Smoke Test Results — PASS
PR: fix(docker-host): pass through loopback TCP DOCKER_HOST for ARC/DinD orchestration Note 🔒 Integrity filter blocked 1 itemThe following item was blocked because it doesn't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
|
Smoke test results:
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Smoke Test: Copilot BYOK (Direct) Mode — PASS ✅
Mode: Direct BYOK ( Overall: PASS
|
|
feat(api-proxy): implement OTLP fan-out to multiple endpoints: ✅
|
Chroot Smoke Test Results
Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments.
|
|
feat(api-proxy): implement OTLP fan-out to multiple endpoints
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL
|
AWF unconditionally deleted any non-
unix://DOCKER_HOSTvalue, breaking ARC RunnerScaleSet pods whereDOCKER_HOST=tcp://localhost:2375is the canonical way to reach the DinD sidecar — causing AWF to fall back to/var/run/docker.sockwhich doesn't exist in runner containers.Core fix
src/docker-host.ts—getLocalDockerEnv()now selectively filtersDOCKER_HOST:unix://sockets and loopback TCP endpoints (tcp://localhost:*,tcp://127.0.0.1:*) are passed through unchanged; non-loopback TCP endpoints (e.g.tcp://192.168.1.100:2375) are cleared so the docker CLI falls back to the default local socket, preserving AWF's network isolation model.Loopback TCP treated as local/valid
src/option-parsers.ts—isLoopbackTcpDockerHost()now uses proper URI parsing (viaURL) and requires: schemetcp://, hostname exactlylocalhostor127.0.0.1, a non-empty numeric port, and no path/query/auth components. Malformed values such astcp://localhost(no port) ortcp://localhost:2375/pathare correctly rejected.checkDockerHost()accepts these validated loopback endpoints as valid.isSiblingDaemonSocket()emitsdindHint=truefor loopback TCP so the split-filesystem prefix warning fires for ARC users.src/commands/validators/config-assembly.ts—--docker-hostflag now accepts loopback TCP URIs in addition tounix://, and the validation error message lists both accepted loopback forms:Type documentation
src/types/container-image-options.ts—awfDockerHostJSDoc updated to accurately describe the three-way passthrough decision: unix:// and loopback TCP pass through unchanged; non-loopback TCP is cleared to fall back to the default socket.