-
Notifications
You must be signed in to change notification settings - Fork 17
fix: handle workflow-scope DinD (DOCKER_HOST=tcp://) without failing AWF startup #1943
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d82b045
fc457b1
f5c2cb9
8f748ec
72ff1cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,6 +16,7 @@ import { | |||||||||||
| preserveIptablesAudit, | ||||||||||||
| fastKillAgentContainer, | ||||||||||||
| collectDiagnosticLogs, | ||||||||||||
| setAwfDockerHost, | ||||||||||||
| } from './docker-manager'; | ||||||||||||
| import { | ||||||||||||
| ensureFirewallNetwork, | ||||||||||||
|
|
@@ -1370,6 +1371,12 @@ program | |||||||||||
| 'Use local images without pulling from registry (requires pre-downloaded images)', | ||||||||||||
| false | ||||||||||||
| ) | ||||||||||||
| .option( | ||||||||||||
| '--docker-host <socket>', | ||||||||||||
| 'Docker socket for AWF\'s own containers (default: auto-detect from DOCKER_HOST env).\n' + | ||||||||||||
| ' Use when Docker is at a non-standard path.\n' + | ||||||||||||
| ' Example: unix:///run/user/1000/docker.sock' | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| // -- Container Configuration -- | ||||||||||||
| .option( | ||||||||||||
|
|
@@ -1602,12 +1609,14 @@ program | |||||||||||
|
|
||||||||||||
| logger.setLevel(logLevel); | ||||||||||||
|
|
||||||||||||
| // Fail fast when DOCKER_HOST points at an external daemon (e.g. workflow-scope DinD). | ||||||||||||
| // AWF's network isolation depends on direct access to the local Docker socket. | ||||||||||||
| // When DOCKER_HOST points at an external TCP daemon (e.g. workflow-scope DinD), | ||||||||||||
| // AWF redirects its own docker calls to the local socket automatically. | ||||||||||||
| // The original DOCKER_HOST value is forwarded into the agent container so the | ||||||||||||
| // agent workload can still reach the DinD daemon. | ||||||||||||
| const dockerHostCheck = checkDockerHost(); | ||||||||||||
| if (!dockerHostCheck.valid) { | ||||||||||||
|
||||||||||||
| if (!dockerHostCheck.valid) { | |
| if (!dockerHostCheck.valid) { | |
| // Apply the AWF-local docker host at the process level before any later | |
| // workflow step shells out to `docker`, so inherited env targets the local daemon. | |
| setAwfDockerHost(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--docker-hostis described as a Unix socket override, but the value is not validated before being applied viasetAwfDockerHost(). If a user passes a TCP/SSH DOCKER_HOST (or an invalid string), AWF will route its container orchestration back to an external daemon and likely fail in the same way this PR is trying to avoid. Consider validating that--docker-hostis aunix://socket URI (and erroring otherwise), or explicitly documenting/handling supported schemes.