From 01a56caac643ea9868c69cb24f95e8aea583961e Mon Sep 17 00:00:00 2001 From: jeanluc Date: Mon, 26 Jun 2023 08:30:29 +0200 Subject: [PATCH] Also test exact expected command parsing failure output --- salt/client/ssh/__init__.py | 1 + salt/client/ssh/wrapper/__init__.py | 27 ++++++++----------- tests/pytests/integration/ssh/test_deploy.py | 28 +++++++++++++++++--- 3 files changed, 37 insertions(+), 19 deletions(-) diff --git a/salt/client/ssh/__init__.py b/salt/client/ssh/__init__.py index 863227a8c4b..4d8ad6423a8 100644 --- a/salt/client/ssh/__init__.py +++ b/salt/client/ssh/__init__.py @@ -572,6 +572,7 @@ class SSH(MultiprocessingStateMixin): if not self.opts.get("raw_shell"): # We only expect valid JSON output from Salt retcode = max(retcode, err.retcode, 1) + else: ret["ret"].pop("_error", None) except Exception as err: # pylint: disable=broad-except log.error( diff --git a/salt/client/ssh/wrapper/__init__.py b/salt/client/ssh/wrapper/__init__.py index d4c47c2195b..c898f564fec 100644 --- a/salt/client/ssh/wrapper/__init__.py +++ b/salt/client/ssh/wrapper/__init__.py @@ -27,16 +27,17 @@ class SSHException(SaltException): _error = "" def __init__( - self, stdout, stderr, retcode, result=NOT_SET, data=None, *args, **kwargs + self, stdout, stderr, retcode, result=NOT_SET, parsed=None, *args, **kwargs ): super().__init__(stderr, *args, **kwargs) self.stdout = stdout self.stderr = self._filter_stderr(stderr) self.result = result - self.data = data + self.parsed = parsed self.retcode = retcode if args: self._error = args.pop(0) + super().__init__(self._error) def _filter_stderr(self, stderr): stderr_lines = [] @@ -61,7 +62,7 @@ class SSHException(SaltException): "stdout": self.stdout, "stderr": self.stderr, "retcode": self.retcode, - "data": self.data, + "parsed": self.parsed, } if self._error: ret["_error"] = self._error @@ -81,9 +82,9 @@ class SSHCommandExecutionError(SSHException, CommandExecutionError): _error = "The command resulted in a non-zero exit code" def to_ret(self): - if self.data and "local" in self.data: + if self.parsed and "local" in self.parsed: # Wrapped commands that indicate a non-zero retcode - return self.data["local"] + return self.parsed["local"] elif self.stderr: # Remote executions that indicate a non-zero retcode return self.stderr @@ -95,7 +96,7 @@ class SSHPermissionDeniedError(SSHException): Thrown when "Permission denied" is found in stderr """ - _error = "Permission Denied" + _error = "Permission denied" class SSHReturnDecodeError(SSHException): @@ -271,13 +272,12 @@ 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"): + if retcode and stderr.count("Permission denied"): raise SSHPermissionDeniedError(stdout=stdout, stderr=stderr, retcode=retcode) result = NOT_SET error = None data = None - local_data = None try: data = salt.utils.json.loads(stdout) @@ -297,6 +297,7 @@ def parse_ret(stdout, stderr, retcode, result_only=False): result = result["return"] except KeyError: error = SSHMalformedReturnError + result = NOT_SET else: error = SSHMalformedReturnError @@ -304,19 +305,13 @@ def parse_ret(stdout, stderr, retcode, result_only=False): # No valid JSON output was found error = SSHReturnDecodeError if retcode: - raise SSHCommandExecutionError( - stdout=stdout, - stderr=stderr, - retcode=retcode, - result=result, - data=local_data if local_data is not None else data, - ) + error = SSHCommandExecutionError if error is not None: raise error( stdout=stdout, stderr=stderr, retcode=retcode, result=result, - data=local_data if local_data is not None else data, + parsed=data, ) return result diff --git a/tests/pytests/integration/ssh/test_deploy.py b/tests/pytests/integration/ssh/test_deploy.py index 169439d9c7e..396733d4b89 100644 --- a/tests/pytests/integration/ssh/test_deploy.py +++ b/tests/pytests/integration/ssh/test_deploy.py @@ -251,6 +251,7 @@ def test_retcode_json_decode_error(salt_ssh_cli): ret.data["stdout"] == '{ "local": { "whoops": "hrhrhr" }Warning: Chaos is not a letter\n}' ) + assert ret.data["_error"] == "Failed to return clean data" assert ret.data["retcode"] == 0 @@ -264,7 +265,9 @@ def test_retcode_invalid_return(salt_ssh_cli): assert ret.returncode == EX_AGGREGATE assert isinstance(ret.data, dict) assert ret.data["stdout"] == '{"no_local_key_present": "Chaos is a ladder though"}' + assert ret.data["_error"] == "Return dict was malformed" assert ret.data["retcode"] == 0 + assert ret.data["parsed"] == {"no_local_key_present": "Chaos is a ladder though"} @pytest.mark.usefixtures("remote_exception_wrap_mod") @@ -273,9 +276,11 @@ def test_wrapper_unwrapped_command_exception(salt_ssh_cli): Verify salt-ssh does not return unexpected exception output to wrapper modules. """ ret = salt_ssh_cli.run("check_exception.failure") + assert ret.returncode == EX_AGGREGATE + assert isinstance(ret.data, str) assert ret.data assert "Probably got garbage" not in ret.data - assert ret.returncode == EX_AGGREGATE + assert "Error running 'disk.usage': Invalid flag passed to disk.usage" in ret.data @pytest.mark.usefixtures("remote_parsing_failure_wrap_mod", "invalid_json_exe_mod") @@ -284,9 +289,16 @@ def test_wrapper_unwrapped_command_parsing_failure(salt_ssh_cli): Verify salt-ssh does not return unexpected unparsable output to wrapper modules. """ ret = salt_ssh_cli.run("check_parsing.failure", "whoops") + assert ret.returncode == EX_AGGREGATE assert ret.data assert "Probably got garbage" not in ret.data - assert ret.returncode == EX_AGGREGATE + assert isinstance(ret.data, dict) + assert ret.data["_error"] == "Failed to return clean data" + assert ret.data["retcode"] == 0 + assert ( + ret.data["stdout"] + == '{ "local": { "whoops": "hrhrhr" }Warning: Chaos is not a letter\n}' + ) @pytest.mark.usefixtures("remote_parsing_failure_wrap_mod", "invalid_return_exe_mod") @@ -295,6 +307,16 @@ def test_wrapper_unwrapped_command_invalid_return(salt_ssh_cli): Verify salt-ssh does not return unexpected unparsable output to wrapper modules. """ ret = salt_ssh_cli.run("check_parsing.failure", "whoopsiedoodle") + assert ret.returncode == EX_AGGREGATE assert ret.data assert "Probably got garbage" not in ret.data - assert ret.returncode == EX_AGGREGATE + assert isinstance(ret.data, dict) + assert ret.data["_error"] == "Return dict was malformed" + assert ret.data["retcode"] == 0 + assert ( + ret.data["stdout"] + == '{"local": {"no_return_key_present": "Chaos is a ladder though"}}' + ) + assert ret.data["parsed"] == { + "local": {"no_return_key_present": "Chaos is a ladder though"} + }