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

fix macOS running as user. #47212

Merged
merged 4 commits into from May 16, 2018

Conversation

Projects
None yet
5 participants
@weswhet
Contributor

weswhet commented Apr 20, 2018

What does this PR do?

Fixes an issue on macOS where running certain commands as a user would fail. This also fixes the service state on macOS for starting/stoping LaunchAgents.

What issues does this PR fix or reference?

#43185

Previous Behavior

On macOS cmdmod would try to load the user environment with something like 'sudo', '-i', '-u', runas, '--', sys.executable. This method of running commands as user fails for some commands like launchctl.

New Behavior

When attempting to run a command as a user cmdmod will now inject su -l {user} -c into the command. This method works for properly simulating commands as a user when run from root for every scenario I could think of.

Tests written?

No

Commits signed with GPG?

Yes

@weswhet weswhet changed the title from adding in a fix for running commands as a user on macos to fix macOS running as user. Apr 20, 2018

@cachedout cachedout requested review from terminalmage and gtmanfred Apr 23, 2018

@gtmanfred

Has this been tested on any other BSD platforms?

I am wondering if it needs to be more restrictive and only be applied on mac, and not BSD OSs

@cedwards would you mind taking a look at this?

@weswhet

This comment has been minimized.

Contributor

weswhet commented Apr 23, 2018

This should only be on macOS unless salt.utils.platform.is_darwin() returns True on something other than macOS.

@gtmanfred

After reading about darwin more closely, it is only Apple stuff, for some reason I thought that apple still used the freebsd kernel, but they have done their own stuff with it and renamed it darwin.

# just run it from the environment on macOS as that
# method doesn't work properly when run as root for certain commands.
if isinstance(cmd, (list, tuple)):
cmd = ' '.join(cmd)

This comment has been minimized.

@terminalmage

terminalmage May 3, 2018

Member

This will break if any arguments contain spaces. You'll need to use shlex.quote from six which we've already imported into cmdmod.py as _cmd_quote. For example:

cmd = ' '.join(map(_cmd_quote, cmd))

Note that you'll also need to add map to the from salt.ext.six.moves import in cmdmod.py to ensure that the itertools version is used in Python 2:

from salt.ext.six.moves import range, zip, map
if isinstance(cmd, (list, tuple)):
cmd = ' '.join(cmd)
cmd = 'su -l {0} -c \'{1}\''.format(runas, cmd)

This comment has been minimized.

@terminalmage

terminalmage May 3, 2018

Member

Since shlex.quote uses single quotes, you'll want to double-quote {1} here.

weswhet added some commits May 7, 2018

@weswhet

This comment has been minimized.

Contributor

weswhet commented May 7, 2018

@terminalmage Thanks for taking a look at this PR. I made the requested changes. Let me know if there is anything else.

@rallytime rallytime requested a review from terminalmage May 15, 2018

@rallytime

This comment has been minimized.

Contributor

rallytime commented May 15, 2018

re-run lint

@rallytime rallytime merged commit 4fe78bb into saltstack:2018.3 May 16, 2018

5 of 8 checks passed

jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #4847 — ABORTED
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #18903 — ABORTED
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #9816 — FAILURE
Details
WIP ready for review
Details
default Build finished.
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #25100 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #22779 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #21830 — SUCCESS
Details

@Ch3LL Ch3LL referenced this pull request Jul 18, 2018

Closed

Brew tests on mac failing #1033

@weswhet weswhet deleted the weswhet:fix-macos-runas branch Sep 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment