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

Allow to adjust LinuxNamespaces #135

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

champtar
Copy link
Contributor

2 use cases I have in mind:

One line change in api.proto and regenerate the pb.go files.

Signed-off-by: Etienne Champetier <[email protected]>
@champtar champtar force-pushed the adjustement-namespaces branch from 8e194a2 to 91fba35 Compare January 23, 2025 02:45
@klihub klihub self-requested a review January 23, 2025 06:35
@klihub
Copy link
Member

klihub commented Jan 23, 2025

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

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

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

@champtar
Copy link
Contributor Author

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

@champtar
Copy link
Contributor Author

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

@klihub
Copy link
Member

klihub commented Jan 23, 2025

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
Copy link
Member

klihub commented Jan 23, 2025

@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 ?

@champtar champtar force-pushed the adjustement-namespaces branch from e4ab673 to 7088e70 Compare January 23, 2025 19:40
@champtar champtar force-pushed the adjustement-namespaces branch 2 times, most recently from 79f4f24 to 808dd0b Compare January 23, 2025 20:02
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

Successfully merging this pull request may close these issues.

2 participants