Add unix socket for daemon access for privileged containers #25347
Add unix socket for daemon access for privileged containers #25347crosbymichael wants to merge 4 commits intomoby:masterfrom
Conversation
Signed-off-by: Michael Crosby <[email protected]>
This adds a separate socket for providing introspection to containers. It is automatically added to all privileged containers as well as the current copy of the docker client binary for use with the socket. The socket will be placed in `/run/docker.sock`. All linux systems/images should have a symlink from `/var/run` to `/run`. The introspection socket does not include the swarm/1.12 endpoints as these require advanced authentication for use. The consumers will get a 404 if they try to access a swarm endpoint. Signed-off-by: Michael Crosby <[email protected]>
Signed-off-by: Michael Crosby <[email protected]>
|
I like it 👍 cc @justincormack |
|
Overall LGTM Wondering if this should be /cc @diogomonica |
|
@aluzzardi ya, it should be another flag as well, waiting for @justincormack to remind me of the good flag name that he thought of, but for testing right now we can add this with privileged containers as it fits the number 1 use case. |
|
earlier discussion #9135 and #9135 (comment) 😇 |
|
@crosbymichael what changed? :) |
container/container_unix.go
Outdated
| return []Mount{ | ||
| { | ||
| Source: filepath.Join(filepath.Dir(path), "docker"), | ||
| Destination: "/usr/bin/docker", |
There was a problem hiding this comment.
Why mount the binary? This is very unlikely to work inside a container with dynamic binaries. Doesn't even work with debian.
There was a problem hiding this comment.
Yes, mounting the binary will not work in general, should not do this (the user can bind mount if they think it will).
There was a problem hiding this comment.
isn't it static?
|
Overall, I'm a bit confused; is this PR for introspection or for controlling docker? For introspection, I'd expect a container to be able to obtain information about itself (published ports on the host, labels, ...?) For controlling docker, this makes sense, but various discussions in the past lead to a conclusion that we should not do this without fine-grained control over what access the container gets. For example, there's many use-cases of containers that only need access to docker events, without the need to be able to control docker. Currently those bind-mount the docker socket (which is obviously insecure), some things from the top of my head;
If we decide on providing full access by default, changing that to a limited set may be complicated in future. I'd far more like a limited introspection API to start with, and opening up more features when needed. I really don't like connecting this to Just my 0.02c. TL;DR; I really like having an introspection API as a feature, but in this form, I don't see it as an improvement over what we currently have |
18b7a42 to
cfcadcf
Compare
|
Won't this break anyone starting docker daemon in a container? |
Signed-off-by: Michael Crosby <[email protected]>
cfcadcf to
9c5e1d2
Compare
|
@justincormack what was the flag name that you were thinking about for this feature ? |
|
@crosbymichael I forget, its annoying me... The janky failure looks real, read only rootfs. @cpuguy83 comment is an issue I think - either we should put the introspection socket on a different path (ugh) or make it optional. |
|
when we figureout a good flag name i'll just remove this from |
|
For the flag name, given it's just a socket to the daemon and not specifically introspection per se, why not call it as such? |
|
I'd prefer a flag that lets us extend this feature in the future: |
|
@ibuildthecloud have you seen this? Do you have any comments/concerns? |
|
Lets close this until we have a better idea on what people expect and want |
This adds a separate socket for providing introspection to containers.
It is automatically added to all privileged containers as well as the
current copy of the docker client binary for use with the socket.
The socket will be placed in
/run/docker.sock. All linuxsystems/images should have a symlink from
/var/runto/run.The introspection socket does not include the swarm/1.12 endpoints as
these require advanced authentication for use. The consumers will get a
404 if they try to access a swarm endpoint.
Signed-off-by: Michael Crosby [email protected]