Backport more patches to fix regression

Fixes QubesOS/qubes-issues#7993 (one more time)

0007-Fix-salt-ssh-stacktrace-when-retcode-is-not-an-integ.patch

@@ -0,0 +1,138 @@
+From 5ded7f2dc5a11878be362aa20cc7979fedd8e65c Mon Sep 17 00:00:00 2001
+From: jeanluc <[email protected]>
+Date: Thu, 29 Jun 2023 14:24:52 +0200
+Subject: [PATCH 1/3] Fix salt-ssh stacktrace when retcode is not an integer
+
+---
+ changelog/64575.fixed.md                  |  1 +
+ salt/client/ssh/__init__.py               | 17 ++++++-
+ tests/pytests/unit/client/ssh/test_ssh.py | 59 ++++++++++++++++++++++-
+ 3 files changed, 75 insertions(+), 2 deletions(-)
+ create mode 100644 changelog/64575.fixed.md
+
+diff --git a/changelog/64575.fixed.md b/changelog/64575.fixed.md
+new file mode 100644
+index 0000000000..71ff76ea9d
+--- /dev/null
++++ b/changelog/64575.fixed.md
+@@ -0,0 +1 @@
++Fixed salt-ssh stacktrace when retcode is not an integer
+diff --git a/salt/client/ssh/__init__.py b/salt/client/ssh/__init__.py
+index 0d9ac9509b..78dbecc989 100644
+--- a/salt/client/ssh/__init__.py
++++ b/salt/client/ssh/__init__.py
+@@ -547,6 +547,11 @@ class SSH(MultiprocessingStateMixin):
+         )
+         ret = {"id": single.id}
+         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
+         # This job is done, yield
+         try:
+             data = salt.utils.json.find_json(stdout)
+@@ -554,7 +559,14 @@ class SSH(MultiprocessingStateMixin):
+                 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 +819,9 @@ class SSH(MultiprocessingStateMixin):
+         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)
+diff --git a/tests/pytests/unit/client/ssh/test_ssh.py b/tests/pytests/unit/client/ssh/test_ssh.py
+index 2be96ab195..9d9d0c094c 100644
+--- a/tests/pytests/unit/client/ssh/test_ssh.py
++++ b/tests/pytests/unit/client/ssh/test_ssh.py
+@@ -3,7 +3,7 @@ import pytest
+ 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,60 @@ 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"
++
++
++@pytest.mark.parametrize("retcode,expected", [("null", None), ('"foo"', "foo")])
++def test_handle_routine_remote_invalid_retcode(opts, target, retcode, expected, 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 = (f'{{"local": {{"retcode": {retcode}, "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": expected, "return": "foo"}}, 1)
++    )
++    assert f"Host 'localhost' reported an invalid retcode: '{expected}'" in caplog.text
++
++
++def test_handle_routine_single_run_invalid_retcode(opts, target, caplog):
++    """
++    Ensure that if Single.run() call returns an invalid retcode,
++    the final exit code is still an integer and set to 1 at least.
++    """
++    single_ret = ("", "Something went seriously wrong", None)
++    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": {
++                    "stdout": "",
++                    "stderr": "Something went seriously wrong",
++                    "retcode": 1,
++                },
++            },
++            1,
++        )
++    )
++    assert "Got an invalid retcode for host 'localhost': 'None'" in caplog.text
+-- 
+2.41.0
+

0008-Add-tests-for-issue-64588.patch

@@ -0,0 +1,75 @@
+From c4bd0dcc61f79e71a9e139c305c7742c3508bf05 Mon Sep 17 00:00:00 2001
+From: jeanluc <[email protected]>
+Date: Fri, 30 Jun 2023 20:35:12 +0200
+Subject: [PATCH 2/3] Add tests for issue 64588
+
+---
+ tests/pytests/unit/client/ssh/test_shell.py | 48 ++++++++++++++++++++-
+ 1 file changed, 47 insertions(+), 1 deletion(-)
+
+diff --git a/tests/pytests/unit/client/ssh/test_shell.py b/tests/pytests/unit/client/ssh/test_shell.py
+index 37065c4c18..96bc776106 100644
+--- a/tests/pytests/unit/client/ssh/test_shell.py
++++ b/tests/pytests/unit/client/ssh/test_shell.py
+@@ -4,7 +4,7 @@ import types
+ import pytest
+
+ import salt.client.ssh.shell as shell
+-from tests.support.mock import patch
++from tests.support.mock import MagicMock, PropertyMock, patch
+
+
+ @pytest.fixture
+@@ -52,3 +52,49 @@ def test_ssh_shell_exec_cmd(caplog):
+         ret = _shell.exec_cmd("ls {}".format(passwd))
+         assert not any([x for x in ret if passwd in str(x)])
+         assert passwd not in caplog.text
++
++
++def test_ssh_shell_exec_cmd_waits_for_term_close_before_reading_exit_status():
++    """
++    Ensure that the terminal is always closed before accessing its exitstatus.
++    """
++    term = MagicMock()
++    has_unread_data = PropertyMock(side_effect=(True, True, False))
++    exitstatus = PropertyMock(
++        side_effect=lambda *args: 0 if term._closed is True else None
++    )
++    term.close.side_effect = lambda *args, **kwargs: setattr(term, "_closed", True)
++    type(term).has_unread_data = has_unread_data
++    type(term).exitstatus = exitstatus
++    term.recv.side_effect = (("hi ", ""), ("there", ""), (None, None), (None, None))
++    shl = shell.Shell({}, "localhost")
++    with patch("salt.utils.vt.Terminal", autospec=True, return_value=term):
++        stdout, stderr, retcode = shl.exec_cmd("do something")
++    assert stdout == "hi there"
++    assert stderr == ""
++    assert retcode == 0
++
++
++def test_ssh_shell_exec_cmd_returns_status_code_with_highest_bit_set_if_process_dies():
++    """
++    Ensure that if a child process dies as the result of a signal instead of exiting
++    regularly, the shell returns the signal code encoded in the lowest seven bits with
++    the highest one set, not None.
++    """
++    term = MagicMock()
++    term.exitstatus = None
++    term.signalstatus = 9
++    has_unread_data = PropertyMock(side_effect=(True, True, False))
++    type(term).has_unread_data = has_unread_data
++    term.recv.side_effect = (
++        ("", "leave me alone"),
++        ("", " please"),
++        (None, None),
++        (None, None),
++    )
++    shl = shell.Shell({}, "localhost")
++    with patch("salt.utils.vt.Terminal", autospec=True, return_value=term):
++        stdout, stderr, retcode = shl.exec_cmd("do something")
++    assert stdout == ""
++    assert stderr == "leave me alone please"
++    assert retcode == 137
+-- 
+2.41.0
+

0009-Make-SSH-shell-report-exitcode-to-the-best-of-its-ab.patch

@@ -0,0 +1,46 @@
+From 30cc2b7bff38e99025ef467dae5006bd06e24359 Mon Sep 17 00:00:00 2001
+From: jeanluc <[email protected]>
+Date: Fri, 30 Jun 2023 20:39:56 +0200
+Subject: [PATCH 3/3] Make SSH shell report exitcode to the best of its ability
+
+---
+ changelog/64588.fixed.md |  1 +
+ salt/client/ssh/shell.py | 15 ++++++++++++++-
+ 2 files changed, 15 insertions(+), 1 deletion(-)
+ create mode 100644 changelog/64588.fixed.md
+
+diff --git a/changelog/64588.fixed.md b/changelog/64588.fixed.md
+new file mode 100644
+index 0000000000..bf9def4eb4
+--- /dev/null
++++ b/changelog/64588.fixed.md
+@@ -0,0 +1 @@
++Fixed SSH shell seldomly fails to report any exit code
+diff --git a/salt/client/ssh/shell.py b/salt/client/ssh/shell.py
+index cfa82d13c2..2df328ed1f 100644
+--- a/salt/client/ssh/shell.py
++++ b/salt/client/ssh/shell.py
+@@ -464,6 +464,19 @@ class Shell:
+                 if stdout:
+                     old_stdout = stdout
+                 time.sleep(0.01)
+-            return ret_stdout, ret_stderr, term.exitstatus
+         finally:
+             term.close(terminate=True, kill=True)
++        # Ensure term.close is called before querying the exitstatus, otherwise
++        # it might still be None.
++        ret_status = term.exitstatus
++        if ret_status is None:
++            if term.signalstatus is not None:
++                # The process died because of an unhandled signal, report
++                # a non-zero exitcode bash-style.
++                ret_status = 128 + term.signalstatus
++            else:
++                log.warning(
++                    "VT reported both exitstatus and signalstatus as None. "
++                    "This is likely a bug."
++                )
++        return ret_stdout, ret_stderr, ret_status
+-- 
+2.41.0
+