Add CAP_SYS_ADMIN the bazel container

[Alice Frosi]

Hi everyone,

I have a request if it would be possible to add the capability CAP_SYS_ADMIN to the kubevirt-bazel-container. This capability is necessary in order to run unshare with the pid namespace. This option is a requirement for testing a new component added by [1] which configure the namespace and cgroup for virtiofs

[1] https://github.com/kubevirt/kubevirt/pull/13442

Many thanks,

Alice

[Alex Kalenyuk]

Hey Alice

I don't think it's a problem to add some capabilities to the builder (development only container), it just seems weird to me that we would need to do that.
Could you elaborate a little on how this functionality will help with testing the new component? 
Will you be relying on the unshare in the unit tests? In that case, I strongly suggest that we inject some dependency to avoid the real calls.

--
You received this message because you are subscribed to the Google Groups "kubevirt-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubevirt-dev...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/kubevirt-dev/CABBoX7MEk-hs2yE6mZvuKJ6UEsi7vjsDG%3D6JB3Ajkun%2BQq-8uA%40mail.gmail.com.

[Alice Frosi]

Hi Alex,

On Mon, Dec 9, 2024 at 12:26 PM Alex Kalenyuk <akal...@redhat.com> wrote:

Hey Alice

I don't think it's a problem to add some capabilities to the builder (development only container), it just seems weird to me that we would need to do that.
Could you elaborate a little on how this functionality will help with testing the new component? 
Will you be relying on the unshare in the unit tests? In that case, I strongly suggest that we inject some dependency to avoid the real calls.

The new component (the dispatcher) is a small binary which launches virtiofs in the unprivileged container. In order to test the component we need to create at least a pid namespace. We can do this using the unshare utility. However, it requires CAP_SYS_ADMIN, otherwise it fails with the permission denied error.

One additional thing, the new component is a C program, so it isn't so easy to mock eventual function.

You can see what is required to test the component in the PR [1].

[Alex Kalenyuk]

add the capability CAP_SYS_ADMIN to the kubevirt-bazel-container

In that case, I am not sure how CAP_SYS_ADMIN on anything other than virt-handler will help? 

[Alice Frosi]

On Mon, Dec 9, 2024 at 12:44 PM Alex Kalenyuk <akal...@redhat.com> wrote:

add the capability CAP_SYS_ADMIN to the kubevirt-bazel-container

In that case, I am not sure how CAP_SYS_ADMIN on anything other than virt-handler will help? 

Well, the component will be used by virt-handler, but we want to test it standalone.

[Alex Kalenyuk]

So, you will be ~unit-testing this new component, and since the unit tests run in the builder container, this capability is needed?

[Alice Frosi]

On Mon, Dec 9, 2024 at 1:29 PM Alex Kalenyuk <akal...@redhat.com> wrote:

So, you will be ~unit-testing this new component, and since the unit tests run in the builder container, this capability is needed?

Exactly! 

[Alex Kalenyuk]

So I don't see an issue with adding a capability into a development container,
but the testing setup does sound like it's increasing the complexity of the things we maintain.

Maybe https://github.com/opencontainers/runc exports something similar in golang, which is already tested?

[Dan Kenigsberg]

Excuse me for being a bit of a novice, Alice, but what are the downsides of enabling CAP_SYS_ADMIN like you suggest?

E.g would it make it easier for a buggy/malicious kubevirt code to harm my dev laptop?

To view this discussion visit https://groups.google.com/d/msgid/kubevirt-dev/CAHahevcgu%2BFMLK71rm7un-_trMP84aRHfa8f0TEuSRoB6x%3DiHg%40mail.gmail.com.

[Edward Haas]

On Tue, Dec 10, 2024 at 5:53 PM Dan Kenigsberg <dan...@redhat.com> wrote:

Excuse me for being a bit of a novice, Alice, but what are the downsides of enabling CAP_SYS_ADMIN like you suggest?

E.g would it make it easier for a buggy/malicious kubevirt code to harm my dev laptop?

On Mon, Dec 9, 2024 at 8:17 PM Alex Kalenyuk <akal...@redhat.com> wrote:

So I don't see an issue with adding a capability into a development container,
but the testing setup does sound like it's increasing the complexity of the things we maintain.

Maybe https://github.com/opencontainers/runc exports something similar in golang, which is already tested?

On Mon, Dec 9, 2024 at 6:13 PM Alice Frosi <afr...@redhat.com> wrote:

On Mon, Dec 9, 2024 at 1:29 PM Alex Kalenyuk <akal...@redhat.com> wrote:

So, you will be ~unit-testing this new component, and since the unit tests run in the builder container, this capability is needed?

Exactly! 

Then I would not classify it as a unit test.

Network code has integration tests [1][2] that are touching the kernel but in an isolated manner.

(isolated by having a network namespace, so side effects are mostly cleaned)

The fact it is not classified as unit tests helps in not requiring all to run it, focusing them for only interested developers and CI.

If your tests can run there without any further elevated capabilities, it will be best.

[1] https://github.com/kubevirt/kubevirt/blob/f916aca46d706e2811642704903e7df6289d8f56/Makefile#L95
[2] https://github.com/kubevirt/kubevirt/blob/f916aca46d706e2811642704903e7df6289d8f56/hack/integration-test.sh#L57

To view this discussion visit https://groups.google.com/d/msgid/kubevirt-dev/CAHOEP56-NH4f95aSLZ98RTgquzb-rRXjcTqvdivcEtqXjQ%3DMhA%40mail.gmail.com.

[Alex Kalenyuk]

I wasn't aware of this setup, it looks like a great fit here.

E.g would it make it easier for a buggy/malicious kubevirt code to harm my dev laptop?

I think the answer is yes, but this setup is already spinning up a `--privileged` builder container, which implies this capability and many more.

[Alice Frosi]

Sorry, I was on PTO last week, and I will take a look at the suggestions! Thanks!