mirror of
https://github.com/valitydev/salt.git
synced 2024-11-07 17:09:03 +00:00
git.latest: Fix regression with identity file usage
Unit test included
This commit is contained in:
Erik Johnson
rallytime
parent
1a50abb3b4
commit
3fac72431f
@ -186,6 +186,28 @@ def _format_git_opts(opts):
|
||||
return _format_opts(opts)
|
||||
|
||||
|
||||
def _find_ssh_exe():
|
||||
'''
|
||||
Windows only: search for git.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):
|
||||
@ -245,23 +267,13 @@ def _git_run(command, cwd=None, user=None, password=None, identity=None,
|
||||
'git/ssh-id-wrapper'
|
||||
)
|
||||
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:
|
||||
if salt.utils.is_windows():
|
||||
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
|
||||
@ -302,25 +314,33 @@ def _git_run(command, cwd=None, user=None, password=None, identity=None,
|
||||
redirect_stderr=redirect_stderr,
|
||||
**kwargs)
|
||||
finally:
|
||||
# Cleanup the temporary ssh wrapper file
|
||||
try:
|
||||
__salt__['file.remove'](tmp_ssh_wrapper)
|
||||
log.debug('Removed ssh wrapper file %s', tmp_ssh_wrapper)
|
||||
except AttributeError:
|
||||
# No wrapper was used
|
||||
pass
|
||||
except (SaltInvocationError, CommandExecutionError) as exc:
|
||||
log.warning('Failed to remove ssh wrapper file %s: %s', tmp_ssh_wrapper, exc)
|
||||
if tmp_ssh_wrapper:
|
||||
# Cleanup the temporary ssh wrapper file
|
||||
try:
|
||||
__salt__['file.remove'](tmp_ssh_wrapper)
|
||||
log.debug('Removed ssh wrapper file %s', tmp_ssh_wrapper)
|
||||
except AttributeError:
|
||||
# No wrapper was used
|
||||
pass
|
||||
except (SaltInvocationError, CommandExecutionError) as exc:
|
||||
log.warning(
|
||||
'Failed to remove ssh wrapper file %s: %s',
|
||||
tmp_ssh_wrapper, exc
|
||||
)
|
||||
|
||||
# Cleanup the temporary identity file
|
||||
try:
|
||||
__salt__['file.remove'](tmp_identity_file)
|
||||
log.debug('Removed identity file %s', tmp_identity_file)
|
||||
except AttributeError:
|
||||
# No identify file was used
|
||||
pass
|
||||
except (SaltInvocationError, CommandExecutionError) as exc:
|
||||
log.warning('Failed to remove identity file %s: %s', tmp_identity_file, exc)
|
||||
if tmp_identity_file:
|
||||
# Cleanup the temporary identity file
|
||||
try:
|
||||
__salt__['file.remove'](tmp_identity_file)
|
||||
log.debug('Removed identity file %s', tmp_identity_file)
|
||||
except AttributeError:
|
||||
# No identify file was used
|
||||
pass
|
||||
except (SaltInvocationError, CommandExecutionError) as 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:
|
||||
|
||||
@ -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,
|
||||
@ -164,3 +165,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.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.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()
|
||||
|
||||
Loading…
Reference in New Issue
Block a user