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

Do not use exec for cinc-solo. Run with sudo #2

Open
wants to merge 1 commit into
base: latest
Choose a base branch
from

Conversation

j-rivero
Copy link
Contributor

@j-rivero j-rivero commented Feb 2, 2021

I see no reason to use exec in a command line call inside the script. Added sudo since we want admin permissions to provision *NIX machines

@nuclearsandwich
Copy link
Contributor

I see no reason to use exec in a command line call inside the script.

If the script is designed to set-up execution of another process (chef-solo, in this case) I usually exec that process so the script process is not hanging around while the target process runs. But that's a personal style thing and not a rule with justifiable position so I'm flexible on it.

Added sudo since we want admin permissions to provision *NIX machines

I do have an objection to inserting sudo calls into scripts. I would expect the entire configure script to be run either as root or via sudo in order to "advertise" the fact that there is root level stuff happening. A slightly more material objection to sudo in scripts is that it prevents the script from being run on systems with sudo unavailable with other means of escalating privileges (shout out to my fellow doas(1) hipsters).

What's the motivation for having sudo in the script rather than running the script itself with sudo when needed?

@j-rivero
Copy link
Contributor Author

What's the motivation for having sudo in the script rather than running the script itself with sudo when needed?

In this case, I think that the goal is to run all the checkout without root permissions under ubuntu user and run the provision with root permissions since it needs them. So the script has two parts with different privileges desired.

@nuclearsandwich
Copy link
Contributor

In this case, I think that the goal is to run all the checkout without root permissions under ubuntu user and run the provision with root permissions since it needs them. So the script has two parts with different privileges desired.

Ah I see. The deployment workflow I've been using involves an interactive root shell created with sudo -Es since I want the clone of the privileged chef repository to be root owned, thus preventing it from being read by unprivileged users on the system such as jenkins-agent.

Do you think we should update the docs to make using the scripts via root the official method or add more work to try and harden the checkouts so they're safe to run as a sudo-able user?

@j-rivero
Copy link
Contributor Author

Ah I see. The deployment workflow I've been using involves an interactive root shell created with sudo -Es since I want the clone of the privileged chef repository to be root owned, thus preventing it from being read by unprivileged users on the system such as jenkins-agent.

Oh that is a great thing. Yes let's change the workflow to make the deployment to stay always under root privileges. It would be nice to avoid completely to have the checkout in the machines since it can help to mitigate the risk.

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