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

cwd option not working with cmd.run and runas #51008

Closed
cdalvaro opened this issue Dec 29, 2018 · 10 comments
Closed

cwd option not working with cmd.run and runas #51008

cdalvaro opened this issue Dec 29, 2018 · 10 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists
Milestone

Comments

@cdalvaro
Copy link
Contributor

cdalvaro commented Dec 29, 2018

Description of Issue

cwd option from cmd.run state is not working in combination with runas option (on macOS at least)

I have arrived to this problem through the git.latest and git.cloned states.

These states are failing with the following comment:

fatal: not a git repository (or any of the parent directories): .git

(Possible related issues #43185 and #586)

Setup

SLS recipe

running as root:
  cmd.run:
    - name: pwd
    - cwd: /Users/Carlos/Desktop

running as user:
  cmd.run:
    - name: pwd
    - cwd: /Users/Carlos/Desktop
    - runas: Carlos

powerlevel9k present:
  git.latest:
    - name: https://github.com/bhilburn/powerlevel9k.git
    - target: /Users/Carlos/.oh-my-zsh/custom/themes/powerlevel9k
    - user: Carlos
    - require:
      - oh-my-zsh present

Output

----------
          ID: running as root
    Function: cmd.run
        Name: pwd
      Result: True
     Comment: Command "pwd" run
     Started: 16:12:18.764387
    Duration: 9.628 ms
     Changes:
              ----------
              pid:
                  73618
              retcode:
                  0
              stderr:
              stdout:
                  /Users/Carlos/Desktop
----------
          ID: running as user
    Function: cmd.run
        Name: pwd
      Result: True
     Comment: Command "pwd" run
     Started: 16:12:18.774467
    Duration: 55.428 ms
     Changes:
              ----------
              pid:
                  73619
              retcode:
                  0
              stderr:
              stdout:
                  /Users/Carlos
----------
          ID: powerlevel9k present
    Function: git.latest
        Name: https://github.com/bhilburn/powerlevel9k.git
      Result: False
     Comment: fatal: not a git repository (or any of the parent directories): .git
     Started: 16:12:18.831065
    Duration: 1908.322 ms
     Changes:

Versions Report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

Salt Version:
           Salt: 2018.3.3

Dependency Versions:
           cffi: 1.11.5
       cherrypy: unknown
       dateutil: 2.4.2
      docker-py: 1.10.6
          gitdb: 2.0.5
      gitpython: 2.1.11
          ioflo: 1.7.5
         Jinja2: 2.8
        libgit2: 0.27.7
        libnacl: 1.6.1
       M2Crypto: 0.31.0
           Mako: 1.0.7
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: 2.19
       pycrypto: 3.7.2
   pycryptodome: Not Installed
         pygit2: 0.27.2
         Python: 3.5.2 (default, Nov 12 2018, 13:43:14)
   python-gnupg: 0.3.8
         PyYAML: 3.11
          PyZMQ: 15.2.0
           RAET: 0.6.8
          smmap: 2.0.5
        timelib: 0.2.4
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: Ubuntu 16.04 xenial
         locale: ANSI_X3.4-1968
        machine: x86_64
        release: 4.4.59+
         system: Linux
        version: Ubuntu 16.04 xenial

Update

The git.latest task was working before upgrading my saltstack minion from 2018.3.2 to 2018.3.3 (with salt-master in 2018.3.3)

I have installed salt in my minion workstation with @Homebrew

brew install salt
@cdalvaro
Copy link
Contributor Author

Maybe a possible solution is the following change:

cmd = 'su -l {0} -c "{1}"'.format(runas, cmd)

by:

cmd = 'su -l {0} -c "cd {1}; {2}"'.format(runas, cwd, cmd)

cdalvaro added a commit to cdalvaro/salt that referenced this issue Dec 31, 2018
When calling cmd.run with cwd in combination
with runas the working directory was not set on macOS

This commit also adds a test to check the expected behaviour
@dwoz
Copy link
Contributor

dwoz commented Dec 31, 2018

@cdalvaro Thanks for reporting and fixing this issue! :)

@dwoz dwoz added Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists P2 Priority 2 labels Dec 31, 2018
@dwoz dwoz added this to the Approved milestone Dec 31, 2018
@garethgreenaway
Copy link
Contributor

@cdalvaro Thanks for the report. I am able to reproduce this, looking at your PR I'm not sure chaining multiple commands is the right fix. If it was working as expected in 2018.3.2, we should figure out what changed between releases. @saltstack/team-core Thoughts?

@garethgreenaway
Copy link
Contributor

FYI:

diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py
index 9bb9a42..e590357 100644
--- a/salt/modules/cmdmod.py
+++ b/salt/modules/cmdmod.py
@@ -413,7 +413,7 @@ def _run(cmd,
         if isinstance(cmd, (list, tuple)):
             cmd = ' '.join(map(_cmd_quote, cmd))
 
-        cmd = 'su -l {0} -c "{1}"'.format(runas, cmd)
+        cmd = 'su {0} -c "{1}"'.format(runas, cmd)
         # set runas to None, because if you try to run `su -l` as well as
         # simulate the environment macOS will prompt for the password of the
         # user and will cause salt to hang.

@cdalvaro
Copy link
Contributor Author

cdalvaro commented Jan 1, 2019

man su
DESCRIPTION
...
     -l      Simulate a full login.  The environment is discarded except for HOME, SHELL, PATH, TERM, and USER.  HOME and SHELL are modified as above.  USER is set to the target
             login.  PATH is set to ``/bin:/usr/bin''.  TERM is imported from your current environment.  The invoked shell is the target login's, and su will change directory to
             the target login's home directory.
...

With option -l the working directory is changed to the target login's home directory, this is the new behavior between 2018.3.2 and 2018.3.3.

But, since -l option simulates a full login, it can be necessary for some commands to work properly when executing as a different user because dependencies with its .profile

I don't know a different solution rather than chaining the cd command, but I would like to know if it exists 😃

gitebra pushed a commit to gitebra/salt that referenced this issue Jan 2, 2019
* upstream/develop: (156 commits)
  Set os and os_family in test_run_cwd_in_combination_with_runas
  Fix test_run_cwd_in_combination_with_runas
  Add multiple retries and timeout for getting vm resources in proxmox cloud. Fixes saltstack#49485
  Fix issue saltstack#51008
  Adding in Windows related core grain code that got removed in merge.
  Ensure the refresh_pillar is run against both the minion and sub_minion, otherwise minion_blackout pillar value is left behind for the sub_minion.
  Update codeauthor email
  Optionally include line number in caller_name
  Allow clone_from setting in proxmox salt-cloud to be an integer as per documentation
  Prevent valid return from virt-what creating unhandled exceptions
  Fix mocking
  Lint fix
  Stop using the deprecated `salt.transport.Channel.factory`
  Bugfix: setting empty 'webhook' option in test_no_webhook
  Bugfix: checking webhook is empty or None
  Bugfix: typo custom_grains
  Fix: __grains__ contents in test_generate_payload
  Fix PyLint: remove unused imports
  Add Visual Studio Code IDE to .gitignore
  Fix: test_generate_payload
  ...
@xenophonf
Copy link
Contributor

xenophonf commented Jan 17, 2019

I'm having the same problem on Windows with Salt 2018.3.3. salt windows-minion cmd.run cd cwd='C:\' returns C:\ like one would expect, but salt windows-minion cmd.run cd cwd='C:\' runas='.\local-user' password='password' returns c:\salt\bin. The same goes for cmd.run states that use runas and runas_password. As a workaround I've prefixed the commands I run with cd wherever &&, which works in the Windows command interpreter like on Unix.

@twangboy twangboy self-assigned this Jan 17, 2019
@a-wildman
Copy link

Still seeing this behavior in 2019.2 (we just attempted to upgrade from 2017.7)

Since the git execution module piggybacks on cmd.run for execution, this effectively means that the git module is completely unusable on macOS since the 2018.3.2 release. While a workaround exists for users of the cmd.run module itself, no such workaround exists for the git module.

Seems like this should be 'High Severity' per https://docs.saltstack.com/en/latest/topics/development/labels.html

@jheiselman
Copy link

Same issue noticed on Windows 2019 with Salt minion 2019.2.0. cmd.run does not "do the right thing" when combining runas and cwd options. Using a 'cmd /C cd /D directory && run action' works, but does not permit the git module to function.

@cdalvaro
Copy link
Contributor Author

At least on macOS this issue has been fixed in #51012 and improved in #54136 and #54769

@sagetherage sagetherage removed the P2 Priority 2 label Jun 3, 2020
@cdalvaro
Copy link
Contributor Author

cdalvaro commented Oct 3, 2020

I will close this issue in a couple of days as it has been already fixed, unless someone has an objection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists
Projects
None yet
Development

No branches or pull requests

8 participants