Fix salt-ssh stacktrace when retcode is not an integer

changelog/64575.fixed.md

@@ -0,0 +1 @@
+Fixed salt-ssh stacktrace when retcode is not an integer

salt/client/ssh/__init__.py

@@ -554,7 +554,14 @@ def handle_routine(self, que, opts, host, target, mine=False):
                 ret["ret"] = data["local"]
                 try:
                     # Ensure a reported local retcode is kept
-                    retcode = data["local"]["retcode"]
+                    remote_retcode = data["local"]["retcode"]
+                    try:
+                        retcode = int(remote_retcode)
+                    except (TypeError, ValueError):
+                        log.warning(
+                            f"Host '{host}' reported an invalid retcode: '{remote_retcode}'"
+                        )
+                        retcode = max(retcode, 1)
                 except (KeyError, TypeError):
                     pass
             else:
@@ -807,6 +814,9 @@ def run(self, jid=None):
         final_exit = 0
         for ret, retcode in self.handle_ssh():
             host = next(iter(ret))
+            if not isinstance(retcode, int):
+                log.warning(f"Host '{host}' returned an invalid retcode: {retcode}")
+                retcode = 1
             final_exit = max(final_exit, retcode)
 
             self.cache_job(jid, host, ret[host], fun)

tests/pytests/unit/client/ssh/test_ssh.py

@@ -3,7 +3,7 @@
 import salt.client.ssh.client
 import salt.utils.msgpack
 from salt.client import ssh
-from tests.support.mock import MagicMock, patch
+from tests.support.mock import MagicMock, Mock, patch
 
 pytestmark = [
     pytest.mark.skip_if_binaries_missing("ssh", "ssh-keygen", check_all=True),
@@ -339,3 +339,26 @@ def test_extra_filerefs(tmp_path, opts):
     with patch("salt.roster.get_roster_file", MagicMock(return_value=roster)):
         ssh_obj = client._prep_ssh(**ssh_opts)
         assert ssh_obj.opts.get("extra_filerefs", None) == "salt://foobar"
+
+
+def test_handle_routine_remote_invalid_retcode(opts, target, caplog):
+    """
+    Ensure that if a remote returns an invalid retcode as part of the return dict,
+    the final exit code is still an integer and set to 1 at least.
+    """
+    single_ret = ('{"local": {"retcode": null, "return": "foo"}}', "", 0)
+    opts["tgt"] = "localhost"
+    single = MagicMock(spec=ssh.Single)
+    single.id = "localhost"
+    single.run.return_value = single_ret
+    que = Mock()
+
+    with patch("salt.roster.get_roster_file", MagicMock(return_value="")), patch(
+        "salt.client.ssh.Single", autospec=True, return_value=single
+    ):
+        client = ssh.SSH(opts)
+        client.handle_routine(que, opts, "localhost", target)
+    que.put.assert_called_once_with(
+        ({"id": "localhost", "ret": {"retcode": None, "return": "foo"}}, 1)
+    )
+    assert "Host 'localhost' reported an invalid retcode: 'None'" in caplog.text