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 changing program to be used by pmm as sudo #294

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

netkv
Copy link

@netkv netkv commented Aug 4, 2023

This pr adds config value [pmm] sudo which can be used to change sudo program to for example doas.

Not sure if the flags -u should part of the config value. Doing so would perhaps allow more flexibility.

I don't know awk very well so it might be completelly wrong but it seems to work on my machine.

@netkv
Copy link
Author

netkv commented Aug 4, 2023

Also should be doas.conf added [global] etc by default? It works but doas has to be configured to have keepenv setenv { PATH } to work properly.

@paradigm
Copy link
Member

paradigm commented Aug 4, 2023

I agree we should make this configurable. In retrospect I'm surprised I didn't do so when first writing this subsystem.

I'd rather not make the configuration item called sudo - I think that'd be confusing for exactly the target audience of this change that doesn't want sudo. How about this:

  • Add a new configuration item under [pmm] called drop-privileges-command and have it default to sudo -u $SUDO_USER.
  • Remove the old unprivileged-user item from the default bedrock.conf, as the new one effectively replaces it.
  • Ensure we're backwards compatible with older bedrock.confs by having pmm first check for and use drop-privileges-command. If it's unavailable, check for and use unprivileged-user pairing it with sudo.
  • In /bedrock/share/pmm/package_managers/* replace ${unprivileged_user} with ${drop_privileges_command}.

Also should be doas.conf added [global] etc by default?

Ideally yes. However, a quirk in how 0.7 works means adding it will make the global instance shadow over every stratum's individual instance. Essentially, it'll hide preexisting doas.conf instances, which will be very confusing for some users.

The proper way to handle this is to move the existing doas.conf elsewhere before making it global, then move it back. You're welcome to do this to your own system now. However, many users may not know to plan ahead for this and lock themselves out of root permissions needed to fix it if we make it a proposed default on preexisting installs.

I'm reworking the configuration system in 0.8 to make the behavior around adding new global items less surprising.

It works but doas has to be configured to have keepenv setenv { PATH } to work properly.

0.7 doesn't have good automation to automatically configure this, and trying to add some now is a bit risky as if it doesn't work it could lock people out of root permissions needed to undo the change. Improving the auto-configuration subsystem is another goal for 0.8.

@netkv
Copy link
Author

netkv commented Aug 5, 2023

Sounds like good plan, i'll try implement that

@netkv
Copy link
Author

netkv commented Aug 5, 2023

This seems to work, any thoughts?

@paradigm paradigm force-pushed the master branch 2 times, most recently from 12e1061 to 524beae Compare March 18, 2024 01:27
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