Merge pull request #46069 from rallytime/bp-46036

Back-port #46036 to oxygen.rc1

salt/modules/git.py

@@ -186,6 +186,28 @@ def _format_git_opts(opts):
     return _format_opts(opts)
 
 
+def _find_ssh_exe():
+    '''
+    Windows only: search for Git's bundled ssh.exe in known locations
+    '''
+    # Known locations for Git's ssh.exe in Windows
+    globmasks = [os.path.join(os.getenv('SystemDrive'), os.sep,
+                              'Program Files*', 'Git', 'usr', 'bin',
+                              'ssh.exe'),
+                 os.path.join(os.getenv('SystemDrive'), os.sep,
+                              'Program Files*', 'Git', 'bin',
+                              'ssh.exe')]
+    for globmask in globmasks:
+        ssh_exe = glob.glob(globmask)
+        if ssh_exe and os.path.isfile(ssh_exe[0]):
+            ret = ssh_exe[0]
+            break
+    else:
+        ret = None
+
+    return ret
+
+
 def _git_run(command, cwd=None, user=None, password=None, identity=None,
              ignore_retcode=False, failhard=True, redirect_stderr=False,
              saltenv='base', **kwargs):
@@ -246,22 +268,12 @@ def _git_run(command, cwd=None, user=None, password=None, identity=None,
             )
             tmp_ssh_wrapper = None
             if salt.utils.platform.is_windows():
-                # Known locations for Git's ssh.exe in Windows
-                globmasks = [os.path.join(os.getenv('SystemDrive'), os.sep,
-                                          'Program Files*', 'Git', 'usr', 'bin',
-                                          'ssh.exe'),
-                             os.path.join(os.getenv('SystemDrive'), os.sep,
-                                          'Program Files*', 'Git', 'bin',
-                                          'ssh.exe')]
-                for globmask in globmasks:
-                    ssh_exe = glob.glob(globmask)
-                    if ssh_exe and os.path.isfile(ssh_exe[0]):
-                        env['GIT_SSH_EXE'] = ssh_exe[0]
-                        break
-                else:
+                ssh_exe = _find_ssh_exe()
+                if ssh_exe is None:
                     raise CommandExecutionError(
                         'Failed to find ssh.exe, unable to use identity file'
                     )
+                env['GIT_SSH_EXE'] = ssh_exe
                 # Use the windows batch file instead of the bourne shell script
                 ssh_id_wrapper += '.bat'
                 env['GIT_SSH'] = ssh_id_wrapper
@@ -275,7 +287,7 @@ def _git_run(command, cwd=None, user=None, password=None, identity=None,
                 env['GIT_SSH'] = tmp_ssh_wrapper
 
             if 'salt-call' not in _salt_cli \
-                    and __salt__['ssh.key_is_encrypted'](id_file):
+                    and __utils__['ssh.key_is_encrypted'](id_file):
                 errors.append(
                     'Identity file {0} is passphrase-protected and cannot be '
                     'used in a non-interactive command. Using salt-call from '
@@ -302,6 +314,7 @@ def _git_run(command, cwd=None, user=None, password=None, identity=None,
                     redirect_stderr=redirect_stderr,
                     **kwargs)
             finally:
+                if tmp_ssh_wrapper:
                     # Cleanup the temporary ssh wrapper file
                     try:
                         __salt__['file.remove'](tmp_ssh_wrapper)
@@ -310,8 +323,12 @@ def _git_run(command, cwd=None, user=None, password=None, identity=None,
                         # No wrapper was used
                         pass
                     except (SaltInvocationError, CommandExecutionError) as exc:
-                    log.warning('Failed to remove ssh wrapper file %s: %s', tmp_ssh_wrapper, exc)
+                        log.warning(
+                            'Failed to remove ssh wrapper file %s: %s',
+                            tmp_ssh_wrapper, exc
+                        )
 
+                if tmp_identity_file:
                     # Cleanup the temporary identity file
                     try:
                         __salt__['file.remove'](tmp_identity_file)
@@ -320,7 +337,10 @@ def _git_run(command, cwd=None, user=None, password=None, identity=None,
                         # No identify file was used
                         pass
                     except (SaltInvocationError, CommandExecutionError) as exc:
-                    log.warning('Failed to remove identity file %s: %s', tmp_identity_file, exc)
+                        log.warning(
+                            'Failed to remove identity file %s: %s',
+                            tmp_identity_file, exc
+                        )
 
             # If the command was successful, no need to try additional IDs
             if result['retcode'] == 0:

salt/modules/ssh.py

@@ -1472,18 +1472,4 @@ def key_is_encrypted(key):
 
         salt '*' ssh.key_is_encrypted /root/id_rsa
     '''
-    try:
-        with salt.utils.files.fopen(key, 'r') as fp_:
-            key_data = salt.utils.stringutils.to_unicode(fp_.read())
-    except (IOError, OSError) as exc:
-        # Raise a CommandExecutionError
-        salt.utils.files.process_read_exception(exc, key)
-
-    is_private_key = re.search(r'BEGIN (?:\w+\s)*PRIVATE KEY', key_data)
-    is_encrypted = 'ENCRYPTED' in key_data
-    del key_data
-
-    if not is_private_key:
-        raise CommandExecutionError('{0} is not a private key'.format(key))
-
-    return is_encrypted
+    return __utils__['ssh.key_is_encrypted'](key)

salt/utils/ssh.py

@@ -0,0 +1,30 @@
+# -*- coding: utf-8 -*-
+
+# Import Python libs
+from __future__ import absolute_import, print_function, unicode_literals
+import re
+
+# Import salt libs
+import salt.utils.files
+import salt.utils.stringutils
+from salt.exceptions import CommandExecutionError
+
+
+def key_is_encrypted(key):
+    # NOTE: this is a temporary workaround until we can get salt/modules/ssh.py
+    # working on Windows.
+    try:
+        with salt.utils.files.fopen(key, 'r') as fp_:
+            key_data = salt.utils.stringutils.to_unicode(fp_.read())
+    except (IOError, OSError) as exc:
+        # Raise a CommandExecutionError
+        salt.utils.files.process_read_exception(exc, key)
+
+    is_private_key = re.search(r'BEGIN (?:\w+\s)*PRIVATE KEY', key_data)
+    is_encrypted = 'ENCRYPTED' in key_data
+    del key_data
+
+    if not is_private_key:
+        raise CommandExecutionError('{0} is not a private key'.format(key))
+
+    return is_encrypted

tests/unit/modules/test_git.py

@@ -14,6 +14,7 @@ import subprocess
 from tests.support.mixins import LoaderModuleMockMixin
 from tests.support.unit import TestCase, skipIf
 from tests.support.mock import (
+    Mock,
     MagicMock,
     patch,
     NO_MOCK,
@@ -77,7 +78,13 @@ class GitTestCase(TestCase, LoaderModuleMockMixin):
     Test cases for salt.modules.git
     '''
     def setup_loader_modules(self):
-        return {git_mod: {}}
+        return {
+            git_mod: {
+                '__utils__': {
+                    'ssh.key_is_encrypted': Mock(return_value=False)
+                },
+            }
+        }
 
     def test_list_worktrees(self):
         '''
@@ -164,3 +171,50 @@ class GitTestCase(TestCase, LoaderModuleMockMixin):
                     dict([(x, worktree_ret[x]) for x in WORKTREE_INFO
                           if WORKTREE_INFO[x].get('stale', False)])
                 )
+
+    def test__git_run_tmp_wrapper(self):
+        '''
+        When an identity file is specified, make sure we don't attempt to
+        remove a temp wrapper that wasn't created. Windows doesn't use temp
+        wrappers, and *NIX won't unless no username was specified and the path
+        is not executable.
+        '''
+        file_remove_mock = Mock()
+        mock_true = Mock(return_value=True)
+        mock_false = Mock(return_value=False)
+        cmd_mock = MagicMock(return_value={
+            'retcode': 0,
+            'stdout': '',
+            'stderr': '',
+        })
+        with patch.dict(git_mod.__salt__, {'file.file_exists': mock_true,
+                                           'file.remove': file_remove_mock,
+                                           'cmd.run_all': cmd_mock,
+                                           'ssh.key_is_encrypted': mock_false}):
+
+            # Non-windows
+            with patch('salt.utils.platform.is_windows', mock_false), \
+                    patch.object(git_mod, '_path_is_executable_others',
+                                 mock_true):
+
+                # Command doesn't really matter here since we're mocking
+                git_mod._git_run(
+                    ['git', 'rev-parse', 'HEAD'],
+                    cwd='/some/path',
+                    user=None,
+                    identity='/root/.ssh/id_rsa')
+
+                file_remove_mock.assert_not_called()
+
+            file_remove_mock.reset_mock()
+            with patch('salt.utils.platform.is_windows', mock_true), \
+                    patch.object(git_mod, '_find_ssh_exe',
+                                 MagicMock(return_value=r'C:\Git\ssh.exe')):
+                # Command doesn't really matter here since we're mocking
+                git_mod._git_run(
+                    ['git', 'rev-parse', 'HEAD'],
+                    cwd=r'C:\some\path',
+                    user=None,
+                    identity=r'C:\ssh\id_rsa')
+
+                file_remove_mock.assert_not_called()