Merge pull request #54769 from cdalvaro/2019.2.1

Call bash only when necessary on macOS

salt/modules/cmdmod.py

@@ -418,7 +418,13 @@ def _get_stripped(cmd):
 
         # Ensure environment is correct for a newly logged-in user by running
         # the command under bash as a login shell
-        cmd = '/bin/bash -l -c {cmd}'.format(cmd=_cmd_quote(cmd))
+        try:
+            user_shell = __salt__['user.info'](runas)['shell']
+            if re.search('bash$', user_shell):
+                cmd = '{shell} -l -c {cmd}'.format(shell=user_shell,
+                                                   cmd=_cmd_quote(cmd))
+        except KeyError:
+            pass
 
         # Ensure the login is simulated correctly (note: su runs sh, not bash,
         # which causes the environment to be initialised incorrectly, which is

tests/unit/modules/test_cmdmod.py

@@ -334,6 +334,59 @@ def test_os_environment_remains_intact(self):
                     if not salt.utils.platform.is_darwin():
                         getpwnam_mock.assert_called_with('foobar')
 
+    @skipIf(not salt.utils.platform.is_darwin(), 'applicable to macOS only')
+    def test_shell_properly_handled_on_macOS(self):
+        '''
+        cmd.run should invoke a new bash login only
+        when bash is the default shell for the selected user
+        '''
+        class _CommandHandler(object):
+            '''
+            Class for capturing cmd
+            '''
+            def __init__(self):
+                self.cmd = None
+
+            def clear(self):
+                self.cmd = None
+
+        cmd_handler = _CommandHandler()
+
+        def mock_proc(__cmd__, **kwargs):
+            cmd_handler.cmd = ' '.join(__cmd__)
+            return MagicMock(return_value=MockTimedProc(stdout=None, stderr=None))
+
+        with patch('pwd.getpwnam') as getpwnam_mock:
+            with patch('salt.utils.timed_subprocess.TimedProc', mock_proc):
+
+                # User default shell is '/usr/local/bin/bash'
+                user_default_shell = '/usr/local/bin/bash'
+                with patch.dict(cmdmod.__salt__,
+                                {'user.info': MagicMock(return_value={'shell': user_default_shell})}):
+
+                    cmd_handler.clear()
+                    cmdmod._run('ls',
+                                cwd=tempfile.gettempdir(),
+                                runas='foobar',
+                                use_vt=False)
+
+                    self.assertRegex(cmd_handler.cmd, "{} -l -c".format(user_default_shell),
+                                        "cmd invokes right bash session on macOS")
+
+                # User default shell is '/bin/zsh'
+                user_default_shell = '/bin/zsh'
+                with patch.dict(cmdmod.__salt__,
+                                {'user.info': MagicMock(return_value={'shell': user_default_shell})}):
+
+                    cmd_handler.clear()
+                    cmdmod._run('ls',
+                                cwd=tempfile.gettempdir(),
+                                runas='foobar',
+                                use_vt=False)
+
+                    self.assertNotRegex(cmd_handler.cmd, "bash -l -c",
+                                        "cmd does not invoke user shell on macOS")
+
     def test_run_cwd_doesnt_exist_issue_7154(self):
         '''
         cmd.run should fail and raise