-
Notifications
You must be signed in to change notification settings - Fork 260
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
Dont call sudo from the scripts. #18
base: master
Are you sure you want to change the base?
Conversation
ping @danielnorberg @dflemstr |
👍 |
Maybe it could be "check if sudo available, if so, then use it, if not - must be root"? |
Thanks @mbruggmann for the PR. Sorry, for keeping you hanging for a few weeks. @ktoso that's what I would prefer as well. AFAICS there's no way to allow non-root users to run @mbruggmann do you know of a best-practice how to solve such a situation? We could either add a flag whether to sudo or not, or make the SUDO command itself configurable through an environment variable. |
Another solution could be to document how to add NOPASSWD mode to sudoers for |
@ktoso @jrudolph We are running the scripts inside a Docker container where:
Installing Of course checking if |
@ktoso is this really related to this change? The question is how to handle the situation that sudo is not available at all. @dflemstr yes, we understood that your use case is different from ours but as running as root is mandatory for |
@jrudolph right, I mistook it to my use case. The proposal you just explained sounds good for all cases I guess. |
For reference, I don't have a strong opinion on this one (nor best-practice), so I'm happy to go with whatever as long as it enables our use-case. |
I've created #76 which hopefully addresses some of the concerns above. |
The reason we'd like to avoid using
sudo
is that we are executing this inside a docker container that doesn't have it installed.