Allow to adjust LinuxNamespaces #135

champtar · 2025-01-23T02:42:25Z

2 use cases I have in mind:

add cgroup namespace for k8s privileged pods
enable time namespace (depends at least on time namespace missing opencontainers/runtime-tools#783)

klihub · 2025-01-23T06:37:30Z

@champtar AFAICT, this is an alternative implementation of #124.

champtar · 2025-01-23T15:07:02Z

@klihub indeed ...
Now that I've written it, mine is more fine grained, ie you can add / modify / remove individual namespaces

champtar · 2025-01-23T15:36:43Z

Ci is broken: /bin/sh: 2: protoc: not found

champtar · 2025-01-23T15:42:48Z

@klihub as there is some interest in having namespaces adjustment, can you ping the right persons to move this PR or #124 forward ?

klihub · 2025-01-23T16:19:38Z

Ci is broken: /bin/sh: 2: protoc: not found

It's not CI per se IMO, rather I think some github-related PITA that hits us with regular irregularity.

What happens is that every once in a while for some reason the protobuf source is cloned with a newer timestamp than the compiled bindings. When this happens our build system tries to rebuild them, but can't since we deliberately leave protoc uninstalled... because commits touching the protocol should commit both the protobuf and the generated sources.

Maybe the right thing to do would be to always install protoc and the plugins we need, then always force a recompilation, and finally check that there are no changes. Why I'm reluctant to do that is that it would also implicitly/indirectly force everyone to use the same protoc{-gen-go,} versions, since those are emitted to the generated sources...

Anyway, I retriggered the workflow for you, and it looks like the coin flipped the other way this time, because it succeeded.

klihub · 2025-01-23T16:45:24Z

@klihub indeed ... Now that I've written it, mine is more fine grained, ie you can add / modify / remove individual namespaces

Currently there is a pair of PRs (#123, #124) with security implications, and it's actively being discussed what additional infra bits we'd need (mostly the ability to lock down selected adjustment capabilities in NRI administratively) so that everybody would feel confident enough about taking those changes in. Your PR is an alternative implementation of #124, so if you have valid uses cases for all of these additional capabilities, could you chime in #124 and point to this PR of yours as an alternative ?

klihub · 2025-06-12T08:14:37Z

@champtar We should rebase this on latest main/HEAD and allow namespace adjustment to be locked via validation. For instance, like this: https://github.com/klihub/nri/tree/devel/champtar/adjustment-namespaces

champtar · 2025-06-12T08:49:10Z

@klihub if you've already done the work, do not hesitate to update this PR https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/committing-changes-to-a-pull-request-branch-created-from-a-fork

klihub · 2025-06-12T09:47:26Z

@klihub if you've already done the work, do not hesitate to update this PR https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/committing-changes-to-a-pull-request-branch-created-from-a-fork

Ah, ok. Thanks for the tip. I did not realize that was possible. I pushed the updates. Is it okay with you if I leave you as the author of the updated original commits ?

champtar · 2025-06-12T10:07:29Z

Ah, ok. Thanks for the tip. I did not realize that was possible. I pushed the updates. Is it okay with you if I leave you as the author of the updated original commits ?

What I usually do in this situation is keep the same commit author and add what was modified in [] + my signed-off-by

Signed-off-by: original author
[rework x y z / fix typo / ...]
Signed-off-by: me

pkg/adaptation/result.go

plugins/default-validator/default-validator.go

mikebrow · 2025-07-14T14:44:47Z

let's wait to rebase this on #123

mikebrow · 2025-07-14T17:19:59Z

pls rebase...

One line change in api.proto and regenerate the pb.go files. Signed-off-by: Etienne Champetier <[email protected]> Signed-off-by: Krisztian Litkey <[email protected]>

Signed-off-by: Etienne Champetier <[email protected]> Signed-off-by: Krisztian Litkey <[email protected]>

klihub · 2025-07-15T07:42:16Z

pls rebase...

@mikebrow Rebased and pushed.

mikebrow

just one question around the TODO..

mikebrow · 2025-07-15T15:15:27Z

pkg/api/adjustment.go

+// AddOrReplaceNamespace records the addition or replacement of the given namespace to a container.
+func (a *ContainerAdjustment) AddOrReplaceNamespace(n *LinuxNamespace) {
+	a.initLinuxNamespaces()
+	a.Linux.Namespaces = append(a.Linux.Namespaces, n) // TODO: should we dup n here ?


What do you mean by this TODO? oci only allows one namespace per type and the func we are calling AddOrReplaceLinuxNamespace ensures that..

@mikebrow It's related to the similar comments about the corresponding helpers for mounts and [CDI] devices (as this is done consistently with those).

Basically we append here the user-supplied n *LinuxNamespace as such to the set of adjustments, as opposed to appending a copy of it. That (being a pointer) allows the user, in principle, to modify it once it has been recorded as an adjustment.

Since the plugin can only shoot itself in the foot with that, I think no too big harm is done. But I think it's good to have the comments in place, so that if we ever decide to change one, we update all the others consistently.

Signed-off-by: Etienne Champetier <[email protected]> Update adjustment plugin ownership tracking and conflict detection using the revised pkg/api-based infra. Signed-off-by: Krisztian Litkey <[email protected]>

Signed-off-by: Etienne Champetier <[email protected]> Signed-off-by: Krisztian Litkey <[email protected]>

Add LinuxNamespace plugin/owner lookup for validation. Signed-off-by: Krisztian Litkey <[email protected]>

Implement configurable restriction of namespace adjustment in the default validator. Signed-off-by: Krisztian Litkey <[email protected]>

Signed-off-by: Krisztian Litkey <[email protected]>

mikebrow

LGTM

champtar force-pushed the adjustement-namespaces branch from 8e194a2 to 91fba35 Compare January 23, 2025 02:45

klihub self-requested a review January 23, 2025 06:35

champtar force-pushed the adjustement-namespaces branch from 91fba35 to e4ab673 Compare January 23, 2025 13:53

champtar force-pushed the adjustement-namespaces branch 4 times, most recently from 808dd0b to 29bb2a8 Compare January 23, 2025 20:09

champtar mentioned this pull request Jan 23, 2025

api: add namespace adjustment #124

Closed

klihub mentioned this pull request Jan 28, 2025

Configurable Restrictions of Selected NRI Features #137

Open

samuelkarp mentioned this pull request May 28, 2025

[pluggable-validation / 3]: implement default validator. #171

Merged

klihub force-pushed the adjustement-namespaces branch from 29bb2a8 to 1e461b4 Compare June 12, 2025 09:42

klihub requested review from djdongjin, fuweid and mikebrow June 12, 2025 19:00

klihub approved these changes Jun 12, 2025

View reviewed changes

djdongjin approved these changes Jun 13, 2025

View reviewed changes

pkg/adaptation/result.go Outdated Show resolved Hide resolved

plugins/default-validator/default-validator.go Outdated Show resolved Hide resolved

klihub force-pushed the adjustement-namespaces branch from 1e461b4 to 3d34356 Compare June 13, 2025 07:18

klihub force-pushed the adjustement-namespaces branch from 3d34356 to 93c5abe Compare July 14, 2025 13:40

mikebrow added the v.next to be merged into the next release label Jul 14, 2025

champtar added 2 commits July 14, 2025 21:27

api: add LinuxNamespace to LinuxContainerAdjustment

c6e1e28

One line change in api.proto and regenerate the pb.go files. Signed-off-by: Etienne Champetier <[email protected]> Signed-off-by: Krisztian Litkey <[email protected]>

api: add helpers for LinuxNamespace adjustement

8538927

Signed-off-by: Etienne Champetier <[email protected]> Signed-off-by: Krisztian Litkey <[email protected]>

klihub force-pushed the adjustement-namespaces branch from 93c5abe to ef933bb Compare July 14, 2025 18:52

mikebrow reviewed Jul 15, 2025

View reviewed changes

klihub requested a review from mikebrow July 16, 2025 14:09

champtar and others added 6 commits July 16, 2025 17:19

adaptation,api: adjust LinuxNamespaces

a584b9b

Signed-off-by: Etienne Champetier <[email protected]> Update adjustment plugin ownership tracking and conflict detection using the revised pkg/api-based infra. Signed-off-by: Krisztian Litkey <[email protected]>

adaptation: add LinuxNamespaces adjustement unit test

1d4f3e1

Signed-off-by: Etienne Champetier <[email protected]> Signed-off-by: Krisztian Litkey <[email protected]>

runtime-tools/generate: adjust LinuxNamespaces

4ab735f

Signed-off-by: Etienne Champetier <[email protected]> Signed-off-by: Krisztian Litkey <[email protected]>

api: add LinuxNamespace owner lookup.

8bcbefb

Add LinuxNamespace plugin/owner lookup for validation. Signed-off-by: Krisztian Litkey <[email protected]>

default-validator: allow restricting namespace adjustment.

6347e7e

Implement configurable restriction of namespace adjustment in the default validator. Signed-off-by: Krisztian Litkey <[email protected]>

docs: update README.md with Linux namespace adjustment.

7d06097

Signed-off-by: Krisztian Litkey <[email protected]>

klihub force-pushed the adjustement-namespaces branch from ef933bb to 7d06097 Compare July 16, 2025 14:19

mikebrow approved these changes Jul 16, 2025

View reviewed changes

mikebrow merged commit c753be1 into containerd:main Jul 16, 2025
7 checks passed

mikebrow mentioned this pull request Jul 16, 2025

api,adaptation,generate: allow setting kernel scheduling policy attributes. #160

Open

champtar deleted the adjustement-namespaces branch July 16, 2025 15:44

Allow to adjust LinuxNamespaces #135

Allow to adjust LinuxNamespaces #135

Uh oh!

Conversation

champtar commented Jan 23, 2025

Uh oh!

klihub commented Jan 23, 2025

Uh oh!

champtar commented Jan 23, 2025

Uh oh!

champtar commented Jan 23, 2025

Uh oh!

champtar commented Jan 23, 2025

Uh oh!

klihub commented Jan 23, 2025

Uh oh!

klihub commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

klihub commented Jun 12, 2025

Uh oh!

champtar commented Jun 12, 2025

Uh oh!

klihub commented Jun 12, 2025

Uh oh!

champtar commented Jun 12, 2025

Uh oh!

Uh oh!

Uh oh!

mikebrow commented Jul 14, 2025

Uh oh!

mikebrow commented Jul 14, 2025

Uh oh!

klihub commented Jul 15, 2025

Uh oh!

mikebrow left a comment

Choose a reason for hiding this comment

Uh oh!

mikebrow Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

klihub Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikebrow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

klihub commented Jan 23, 2025 •

edited

Loading

klihub Jul 15, 2025 •

edited

Loading