-
Notifications
You must be signed in to change notification settings - Fork 624
Enable FUSE in workerd's local-dev Docker container client #6596
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -1698,6 +1698,24 @@ kj::Promise<void> ContainerClient::createContainer(kj::StringPtr effectiveImage, | |
| } | ||
| } | ||
|
|
||
| // Docker doesn't grant FUSE access by default — enable the minimum permissions for it. | ||
| { | ||
| auto capAdd = hostConfig.initCapAdd(1); | ||
| capAdd.set(0, "SYS_ADMIN"); | ||
| } | ||
| { | ||
| auto devices = hostConfig.initDevices(1); | ||
| auto fuseDev = devices[0]; | ||
| fuseDev.setPathOnHost("/dev/fuse"); | ||
| fuseDev.setPathInContainer("/dev/fuse"); | ||
| fuseDev.setCgroupPermissions("rwm"); | ||
| } | ||
| { | ||
| // Linux-only: no-op on hosts without AppArmor (e.g. macOS). | ||
| auto securityOpt = hostConfig.initSecurityOpt(1); | ||
| securityOpt.set(0, "apparmor:unconfined"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 67aeb3b |
||
| } | ||
|
Comment on lines
+1701
to
+1717
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This unconditionally grants A few options to consider:
I'd defer to the maintainers on which approach they prefer, but option 1 seems safest for a first pass — it avoids surprising users who don't need FUSE with elevated container privileges. |
||
|
|
||
| auto response = co_await dockerApiRequest(network, kj::str(dockerPath), kj::HttpMethod::POST, | ||
| kj::str("/containers/create?name=", containerName), codec.encode(jsonRoot)); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,9 +32,9 @@ struct Docker { | |
| } | ||
|
|
||
| struct DeviceMapping { | ||
| pathOnHost @0 :Text; | ||
| pathInContainer @1 :Text; | ||
| cgroupPermissions @2 :Text; | ||
| pathOnHost @0 :Text $Json.name("PathOnHost"); | ||
| pathInContainer @1 :Text $Json.name("PathInContainer"); | ||
| cgroupPermissions @2 :Text $Json.name("CgroupPermissions"); | ||
| } | ||
|
Comment on lines
34
to
38
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Docker's API uses PascalCase keys for these fields ( |
||
|
|
||
| struct DeviceRequest { | ||
|
|
||
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.
Since the sidecar container (line 1976) already uses
initCapAdd(1)forNET_ADMIN, this follows the existing pattern well. However, if FUSE support is eventually gated behind an opt-in, this block should be conditional. Even if it stays unconditional, consider adding a brief rationale comment explaining whySYS_ADMINspecifically (i.e., FUSE needs themount()syscall) so future readers don't mistake it for an oversight.