set annotations when creating sanbodx and containers #369
Conversation
Docker now allows client to set annotaion when creating sandbox and containers that is significant for some low-level runtimes(e.g. kata-containers) as they use it to determine whether the creating is a pod_sandbox or a pod_container. Signed-off-by: cncal <[email protected]>
| // TODO: Currently docker ContainerList api doesn't return annotations, so annotations | ||
| // still need to be extracted from labels here. |
There was a problem hiding this comment.
I'm not super comfortable with having different sources of truth for annotations depending on the CRI API endpoint queried. It would be straightforward to extend the ContainerList API to return container annotations; perhaps we should do that first so support can be added properly to cri-dockerd?
There was a problem hiding this comment.
Actually I'm working on it moby/moby#47866. May I mark this PR a WIP util docker-side supports it ?
There was a problem hiding this comment.
moby/moby#47866 has been merged into master branch, but released in Pre-release https://github.com/moby/moby/releases/tag/v27.0.0-rc.1. May I bump github.com/docker/docker from 26.1.2+incompatible to v27.0.0-rc.1+incompatible? @corhere
| // TODO: Docker supports annotations for now, so labels and annotations might be separated | ||
| // in the future. Keeping them merged is just for backward compatibility. |
There was a problem hiding this comment.
Backward compatibility with what? AIUI no containers will be persisted across an upgrade (daemon restart) so there is no need for containers started by cri-dockerd v0.3.x to be understandable by v0.4.x, or vice versa.
There was a problem hiding this comment.
There are two main considerations for backward compatibility:
- I‘m not sure if some runtimes use the label to make some judgments
- Docker ContainerList api doesn't return annotations, so keeping them merged is needed for later extraction.
There was a problem hiding this comment.
Seems vulnerable to container breakout attacks via systemd properties
Same as GHSA-2cgq-h8xw-2v5j
containerd/CRI needs an explicit allow list for specifying OCI annotations
https://github.com/containerd/containerd/blob/v1.7.17/docs/cri/config.md?plain=1#L310
|
@AkihiroSuda Yes, I noticed that, but docker does not support setting up |
So, cri-dockerd will have to implement the allow list, or at least, check
cri-dockerd side. |
|
How about adding |
Yes, cri-dockerd may run |
Sorry, I don't get it. You mean we get the |
Right.
Every annotation should be assumed unsafe, and should not be propagated in this case. |
|
|
Runtime features are surfaced in the Info API, keyed by the runtime name. moby/moby#46647 |
Thanks @corhere , it sounds good. |
Proposed Changes
Docker now allows client to set annotaion when creating sandbox and containers that is significant for some low-level runtimes(e.g. kata-containers) as they use it to determine whether the creating is a pod_sandbox or a pod_container.