saltstack/salt
1
0
Fork 0
mirror of https://github.com/saltstack/salt.git synced 2025-04-17 10:10:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Update key deploy routine for new parsing workflow

Browse source
This commit is contained in:
jeanluc 2023-06-26 10:44:12 +02:00 • committed by Daniel Wozniak
parent 01a56caac6
commit 2224a82ded
3 changed files with 101 additions and 51 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

74
salt/client/ssh/__init__.py
View file

@ -464,19 +464,15 @@ class SSH(MultiprocessingStateMixin):
"""
Deploy the SSH key if the minions don't auth
"""
# `or` initially was `and`, but was changed to be able to "deploy
# to multiple hosts" in #22661. Why? On each command error, this checks
# if a key deploy can be attempted without it being requested.
if not isinstance(ret[host], dict) or self.opts.get("ssh_key_deploy"):
target = self.targets[host]
if target.get("passwd", False) or self.opts["ssh_passwd"]:
self._key_deploy_run(host, target, False)
return ret
stderr = ret[host].get("stderr", "")
# -failed to upload file- is detecting scp errors
# Errors to ignore when Permission denied is in the stderr. For example
# scp can get a permission denied on the target host, but they where
# able to accurate authenticate against the box
ignore_err = ["failed to upload file"]
check_err = [x for x in ignore_err if stderr.count(x)]
if "Permission denied" in stderr and not check_err:
return ret, None
if "_error" in ret[host] and ret[host]["_error"] == "Permission denied":
target = self.targets[host]
# permission denied, attempt to auto deploy ssh key
print(
@ -490,7 +486,7 @@ class SSH(MultiprocessingStateMixin):
"Password for {}@{}: ".format(target["user"], host)
)
return self._key_deploy_run(host, target, True)
return ret
return ret, None
def _key_deploy_run(self, host, target, re_run=True):
"""
@ -529,15 +525,46 @@ class SSH(MultiprocessingStateMixin):
)
stdout, stderr, retcode = single.cmd_block()
try:
data = salt.utils.json.find_json(stdout)
return {host: data.get("local", data)}
except Exception: # pylint: disable=broad-except
if stderr:
return {host: stderr}
return {host: "Bad Return"}
retcode = int(retcode)
except (TypeError, ValueError):
log.warning(f"Got an invalid retcode for host '{host}': '{retcode}'")
retcode = 1
try:
ret = (
salt.client.ssh.wrapper.parse_ret(stdout, stderr, retcode),
salt.defaults.exitcodes.EX_OK,
)
except (
salt.client.ssh.wrapper.SSHPermissionDeniedError,
salt.client.ssh.wrapper.SSHCommandExecutionError,
) as err:
ret = err.to_ret()
retcode = max(retcode, err.retcode, 1)
except salt.client.ssh.wrapper.SSHException as err:
ret = err.to_ret()
if not self.opts.get("raw_shell"):
# We only expect valid JSON output from Salt
retcode = max(retcode, err.retcode, 1)
else:
ret.pop("_error", None)
except Exception as err: # pylint: disable=broad-except
log.error(
f"Error while parsing the command output: {err}",
exc_info_on_loglevel=logging.DEBUG,
)
ret = {
"_error": f"Internal error while parsing the command output: {err}",
"stdout": stdout,
"stderr": stderr,
"retcode": retcode,
"data": None,
}
retcode = max(retcode, 1)
return {host: ret}, retcode
if salt.defaults.exitcodes.EX_OK != retcode:
return {host: stderr}
return {host: stdout}
return {host: stderr}, retcode
return {host: stdout}, retcode
def handle_routine(self, que, opts, host, target, mine=False):
"""
@ -559,6 +586,11 @@ class SSH(MultiprocessingStateMixin):
retcode = 0
try:
stdout, stderr, retcode = single.run()
try:
retcode = int(retcode)
except (TypeError, ValueError):
log.warning(f"Got an invalid retcode for host '{host}': '{retcode}'")
retcode = 1
ret["ret"] = salt.client.ssh.wrapper.parse_ret(stdout, stderr, retcode)
except (
salt.client.ssh.wrapper.SSHPermissionDeniedError,
@ -831,7 +863,11 @@ class SSH(MultiprocessingStateMixin):
final_exit = max(final_exit, retcode)
self.cache_job(jid, host, ret[host], fun)
ret = self.key_deploy(host, ret)
ret, deploy_retcode = self.key_deploy(host, ret)
if deploy_retcode is not None:
retcode = deploy_retcode
final_exit = max(final_exit, retcode)
if isinstance(ret[host], dict) and (
ret[host].get("stderr") or ""

63
salt/client/ssh/wrapper/__init__.py
View file

@ -272,38 +272,55 @@ def parse_ret(stdout, stderr, retcode, result_only=False):
log.warning(f"Got an invalid retcode for host: '{retcode}'")
retcode = 1
if retcode and stderr.count("Permission denied"):
raise SSHPermissionDeniedError(stdout=stdout, stderr=stderr, retcode=retcode)
if "Permission denied" in stderr:
# -failed to upload file- is detecting scp errors
# Errors to ignore when Permission denied is in the stderr. For example
# scp can get a permission denied on the target host, but they where
# able to accurate authenticate against the box
ignore_err = ["failed to upload file"]
check_err = [x for x in ignore_err if stderr.count(x)]
if not check_err:
raise SSHPermissionDeniedError(
stdout=stdout, stderr=stderr, retcode=retcode
)
result = NOT_SET
error = None
data = None
try:
data = salt.utils.json.loads(stdout)
if len(data) < 2 and "local" in data:
try:
result = data["local"]
try:
# Ensure a reported local retcode is kept (at least)
retcode = max(retcode, result["retcode"])
except (KeyError, TypeError):
pass
if not isinstance(data["local"], dict):
# When a command has failed, the return is dumped as-is
# without declaring it as a result, usually a string or list.
error = SSHCommandExecutionError
elif result_only:
result = result["return"]
except KeyError:
error = SSHMalformedReturnError
result = NOT_SET
else:
error = SSHMalformedReturnError
data = salt.utils.json.find_json(stdout)
except ValueError:
# No valid JSON output was found
error = SSHReturnDecodeError
else:
if isinstance(data, dict) and len(data) < 2 and "local" in data:
result = data["local"]
try:
remote_retcode = result["retcode"]
except (KeyError, TypeError):
pass
else:
try:
# Ensure a reported local retcode is kept (at least)
retcode = max(retcode, remote_retcode)
except (TypeError, ValueError):
log.warning(f"Host reported an invalid retcode: '{remote_retcode}'")
retcode = max(retcode, 1)
if not isinstance(result, dict):
# When a command has failed, the return is dumped as-is
# without declaring it as a result, usually a string or list.
error = SSHCommandExecutionError
elif result_only:
try:
result = result["return"]
except KeyError:
error = SSHMalformedReturnError
result = NOT_SET
else:
error = SSHMalformedReturnError
if retcode:
error = SSHCommandExecutionError
if error is not None:

15
tests/pytests/unit/client/ssh/test_ssh.py
View file

@ -356,6 +356,7 @@ def test_key_deploy_permission_denied_scp(tmp_path, opts):
ssh_ret = {
host: {
"_error": "Permission denied",
"stdout": "\rroot@192.168.1.187's password: \n\rroot@192.168.1.187's password: \n\rroot@192.168.1.187's password: \n",
"stderr": "Permission denied, please try again.\nPermission denied, please try again.\nroot@192.168.1.187: Permission denied (publickey,gssapi-keyex,gssapi-with-micimport pudb; pu.dbassword).\nscp: Connection closed\n",
"retcode": 255,
@ -370,7 +371,7 @@ def test_key_deploy_permission_denied_scp(tmp_path, opts):
"fun": "cmd.run",
"fun_args": ["echo test"],
}
}
}, 0
patch_roster_file = patch("salt.roster.get_roster_file", MagicMock(return_value=""))
with patch_roster_file:
client = ssh.SSH(opts)
@ -415,7 +416,7 @@ def test_key_deploy_permission_denied_file_scp(tmp_path, opts):
patch_roster_file = patch("salt.roster.get_roster_file", MagicMock(return_value=""))
with patch_roster_file:
client = ssh.SSH(opts)
ret = client.key_deploy(host, ssh_ret)
ret, retcode = client.key_deploy(host, ssh_ret)
assert ret == ssh_ret
assert mock_key_run.call_count == 0
@ -446,7 +447,7 @@ def test_key_deploy_no_permission_denied(tmp_path, opts):
patch_roster_file = patch("salt.roster.get_roster_file", MagicMock(return_value=""))
with patch_roster_file:
client = ssh.SSH(opts)
ret = client.key_deploy(host, ssh_ret)
ret, retcode = client.key_deploy(host, ssh_ret)
assert ret == ssh_ret
assert mock_key_run.call_count == 0
@ -472,7 +473,7 @@ def test_handle_routine_remote_invalid_retcode(opts, target, retcode, expected,
que.put.assert_called_once_with(
({"id": "localhost", "ret": {"retcode": expected, "return": "foo"}}, 1)
)
assert f"Host 'localhost' reported an invalid retcode: '{expected}'" in caplog.text
assert f"Host reported an invalid retcode: '{expected}'" in caplog.text
def test_handle_routine_single_run_invalid_retcode(opts, target, caplog):
@ -496,11 +497,7 @@ def test_handle_routine_single_run_invalid_retcode(opts, target, caplog):
(
{
"id": "localhost",
"ret": {
"stdout": "",
"stderr": "Something went seriously wrong",
"retcode": 1,
},
"ret": "Something went seriously wrong",
},
1,
)