Skip to content
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

[Spec extension] Allow for IPC, NET, and CAP modifications to be specified in the CDI spec #55

Open
elezar opened this issue Mar 16, 2022 · 12 comments

Comments

@elezar
Copy link
Contributor

elezar commented Mar 16, 2022

We have a use case that would require the following (docker) arguments to work out the box:

--ipc host
--net host
--cap-add=IPC_LOCK

Does it make sense to extend the CDI spec to allow for IPC, NET, and CAP modifications to the OCI spec?

Copy link

This issue is stale because it has been open 90 days with no activity. This issue will be closed in 30 days unless new comments are made or the stale label is removed.

Copy link

This issue was automatically closed due to inactivity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 29, 2024
@mythi
Copy link

mythi commented Oct 28, 2024

Does it make sense to extend the CDI spec to allow for IPC, NET, and CAP modifications to the OCI spec?

we would have a use-case with --cap-add=IPC_LOCK too so +1

@elezar
Copy link
Contributor Author

elezar commented Oct 28, 2024

@mythi let's reopen this then. Would you have time to expand on this / create a PR with the additions?

Note that one concern is that these changes would happen without the user knowing about them, so it may be good to limit the scope to use cases that don't provide too many additional privileges.

@elezar elezar reopened this Oct 28, 2024
@zvonkok
Copy link
Contributor

zvonkok commented Oct 28, 2024

@elezar @mythi Just for understanding, CDI would be able to override what the Pod spec is configuring?
What would happen in this case when run with docker,podman --net=host and CDI says something else?

@mythi
Copy link

mythi commented Oct 28, 2024

I did not think of the details yet. My use case is with QAT devices that use VFIO framework. Pods requesting resources (DRA/device plugins) would also need to know to add IPC_LOCK to get the workload to work. The idea is that users would not need to know those details but the HW dependent settings would come through CDI.

@zvonkok
Copy link
Contributor

zvonkok commented Oct 28, 2024

I will throw in rlimit settings, since k8s is not yet able to set them properly, where one would need CAP_SYS_RESOURCE as well so it kinda fits in here as well. kubernetes/kubernetes#3595

@elezar
Copy link
Contributor Author

elezar commented Oct 28, 2024

@elezar @mythi Just for understanding, CDI would be able to override what the Pod spec is configuring?
What would happen in this case when run with docker,podman --net=host and CDI says something else?

That is what we would need to discuss / define for this issue. We would need to define the modifications that are to be made to the OCI runtime spec as well as determine whether there could be contradicting configurations.

@elezar
Copy link
Contributor Author

elezar commented Oct 28, 2024

Maybe splitting this into adding CAP_ADD support and the "host"-specifics would be useful. One option woudl be to add a REQUIRES_CAP check that would alert the user if the required caps are not specified?

@klihub
Copy link
Contributor

klihub commented Oct 29, 2024

Hmm... changing the IPC and network namespace seems a bit extreme. A failure to inject with an appropriate error message in a case where a device requires host IPC or network namespace but runs in a different one sounds more appropriate... as a last resort. Now I have not followed the latest directions DRA has taken, but when a CDI device reference is added by a DRA driver, it would sound more appropriate to already fail as early as possible, in the DRA driver.

@kad
Copy link
Contributor

kad commented Oct 29, 2024

I will throw in rlimit settings, since k8s is not yet able to set them properly, where one would need CAP_SYS_RESOURCE as well so it kinda fits in here as well. kubernetes/kubernetes#3595

@zvonkok for rlimit, CDI probably not the best way to do it. you can use NRI plugin to inject it based on the need. See containerd/nri#48 for details.

@bachp
Copy link

bachp commented Dec 5, 2024

I have a different use case where a local resource I am exposing requires NET_RAW and NET_ADMIN capabilities to function correctly.

I think from a security point of view it might be better to just produce an error if the container doesn't have the capabilities required, as just adding them behind the scenes via cdi might be surprising to the user and expose them to security risks without realizing the container is running with added capabilies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants