kubereboot / kured

Kubernetes Reboot Daemon
https://kured.dev
Apache License 2.0
2.12k stars 201 forks source link

Make nsenter in reboot command optional #896

Closed bh185140 closed 3 weeks ago

bh185140 commented 5 months ago

Following https://github.com/kubereboot/kured/pull/814 which added signal based reboot we could reduce the privilege of the kured pods. With this we could also look into reducing the privileges for the reboot command method which currently is limited by the need to enter the host namespace.

If nsenter was made optional or configurable, this allows a lot more flexibility in setting up reboot methods that are more secure. We can imitate signal reboot by sending signals as a command through /bin/kill. This can also indirectly solve https://github.com/kubereboot/kured/issues/868 as we could also make the reboot command touch a reboot file.

ckotzbauer commented 5 months ago

But this only would work, if you can use binaries which are already inside the kured-image right? So maybe you use a custom-image?

bh185140 commented 5 months ago

But this only would work, if you can use binaries which are already inside the kured-image right? So maybe you use a custom-image?

Yes, for the use cases I've listed

I think it should be fairly small change that won't break any thing. Mainly need it for the reduced privileges as I can't use the new reboot signal methods since I need to run a small shell script in the command before hand.

bh185140 commented 5 months ago

Main driver for this is mostly I needed a mechanism to trigger shutdowns as well as reboot. I could open up a separate issue for it I suppose, not sure if there would be enough interest in supporting shutdown on a reboot daemon. Though this could be a smaller change to go in.

ckotzbauer commented 5 months ago

Thanks for the explanations. Yes, both binaries are already present. For the path-based method we could just create a file with go-code at a configurable, mounted location. Shutdowns are not directly in scope of kured, I think.

bh185140 commented 5 months ago

Happy to open up a PR for this if you think this change could go into Kured. I see currently nsenter is done with /usr/bin/nsenter", fmt.Sprintf("-m/proc/%d/ns/mnt", pid), "-- where the pid is a constant 1. Can possibly replace it with a default argument --reboot-command-prefix="/usr/bin/nsenter -m/proc/1/ns/mnt --". Alternatively just make it boolean parameter on where or not we add nsenter. What do you think?

bh185140 commented 5 months ago

Raised a PR here if interested https://github.com/kubereboot/kured/pull/899/files

// PID set to 1, until we have a better discovery mechanism.

Saw this comment while making this. Could be another use case to support changing the PID as a configuration for rancher.

bh185140 commented 3 months ago

@ckotzbauer Gentle bump for this thread

ckotzbauer commented 3 months ago

Thanks for the hint, I did a review.

github-actions[bot] commented 1 month ago

This issue was automatically considered stale due to lack of activity. Please update it and/or join our slack channels to promote it, before it automatically closes (in 7 days).