Mirantis / cri-dockerd

dockerd as a compliant Container Runtime Interface for Kubernetes
Apache License 2.0
1.01k stars 272 forks source link

set annotations when creating sanbodx and containers #369

Open cncal opened 1 month ago

cncal commented 1 month ago

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.

cncal commented 1 month ago

@corhere @nwneisen PTAL.

cncal commented 1 month ago

@AkihiroSuda Yes, I noticed that, but docker does not support setting up pod_annotations or container_annotations for runtime. Also, filetering annotations should be supported at docker side or cri-dockerd?

AkihiroSuda commented 1 month ago

@AkihiroSuda Yes, I noticed that, but docker does not support setting up pod_annotations or container_annotations for runtime.

So, cri-dockerd will have to implement the allow list, or at least, check potentiallyUnsafeConfigAnnotations https://github.com/opencontainers/runc/blob/v1.2.0-rc.1/features.go#L67-L71

Also, filetering annotations should be supported at docker side or cri-dockerd?

cri-dockerd side.

cncal commented 1 month ago

How about adding potentiallyUnsafeConfigAnnotations as a cri-dockerd flag used to filter annotations from k8s ?

AkihiroSuda commented 1 month ago

How about adding potentiallyUnsafeConfigAnnotations as a cri-dockerd flag used to filter annotations from k8s ?

Yes, cri-dockerd may run runc features and check the potentiallyUnsafeConfigAnnotations field to filter unsafe annotations that are passed via Kubernetes annotations. This needs runc v1.2, which is still RC, though.

cncal commented 1 month ago

Yes, cri-dockerd may run runc features and check the potentiallyUnsafeConfigAnnotations field to filter unsafe annotations that are passed via Kubernetes annotations.

Sorry, I don't get it. You mean we get the RuntimeHandler(e.g. runc) from k8s request at first and run $runtime features command to fetch the potentiallyUnsafeConfigAnnotations value? What if RuntimeHandler is user-defined(may be runc with some special options named runc-1) ? As far as know, kata and gvisor do not have features subcommand for now.

AkihiroSuda commented 1 month ago

Yes, cri-dockerd may run runc features and check the potentiallyUnsafeConfigAnnotations field to filter unsafe annotations that are passed via Kubernetes annotations.

Sorry, I don't get it. You mean we get the RuntimeHandler(e.g. runc) from k8s request at first and run $runtime features command to fetch the potentiallyUnsafeConfigAnnotations value?

Right. $runtime features can be invoked before handling Kubernetes requests though.

What if RuntimeHandler is user-defined(may be runc with some special options named runc-1) ? As far as know, kata and gvisor do not have features subcommand for now.

Every annotation should be assumed unsafe, and should not be propagated in this case.

cncal commented 1 month ago

Yes, cri-dockerd may run runc features and check the potentiallyUnsafeConfigAnnotations field to filter unsafe annotations that are passed via Kubernetes annotations.

Sorry, I don't get it. You mean we get the RuntimeHandler(e.g. runc) from k8s request at first and run $runtime features command to fetch the potentiallyUnsafeConfigAnnotations value?

Right. $runtime features can be invoked before handling Kubernetes requests though.

What if RuntimeHandler is user-defined(may be runc with some special options named runc-1) ? As far as know, kata and gvisor do not have features subcommand for now.

Every annotation should be assumed unsafe, and should not be propagated in this case.

@corhere @nwneisen Any suggestions?

corhere commented 3 weeks ago

Runtime features are surfaced in the Info API, keyed by the runtime name. https://github.com/moby/moby/pull/46647

cncal commented 3 weeks ago

Runtime features are surfaced in the Info API, keyed by the runtime name. moby/moby#46647

Thanks @corhere , it sounds good.