Backport a fix for salt-ssh exit code

Fixes QubesOS/qubes-issues#7993

0006-Fix-salt-ssh-state.-commands-retcode-for-render-fail.patch

@@ -0,0 +1,369 @@
+From b58190a56e1986dfca357472c113afb26ce5535a Mon Sep 17 00:00:00 2001
+From: jeanluc <lkubb@protonmail.com>
+Date: Wed, 21 Jun 2023 15:38:32 +0200
+Subject: [PATCH] Fix salt-ssh state.* commands retcode for render fail
+
+---
+ changelog/64514.fixed.md         |  1 +
+ salt/client/ssh/__init__.py      | 40 ++++++++++--------
+ salt/client/ssh/wrapper/state.py | 70 ++++++++++++++++++++++++++++----
+ 3 files changed, 84 insertions(+), 27 deletions(-)
+ create mode 100644 changelog/64514.fixed.md
+
+diff --git a/changelog/64514.fixed.md b/changelog/64514.fixed.md
+new file mode 100644
+index 0000000000..b84fb366bf
+--- /dev/null
++++ b/changelog/64514.fixed.md
+@@ -0,0 +1 @@
++Fixed salt-ssh state.* commands returning retcode 0 when state/pillar rendering fails
+diff --git a/salt/client/ssh/__init__.py b/salt/client/ssh/__init__.py
+index 88365a6099..4a1e785e6b 100644
+--- a/salt/client/ssh/__init__.py
++++ b/salt/client/ssh/__init__.py
+@@ -552,6 +552,11 @@ class SSH(MultiprocessingStateMixin):
+             data = salt.utils.json.find_json(stdout)
+             if len(data) < 2 and "local" in data:
+                 ret["ret"] = data["local"]
++                try:
++                    # Ensure a reported local retcode is kept
++                    retcode = data["local"]["retcode"]
++                except (KeyError, TypeError):
++                    pass
+             else:
+                 ret["ret"] = {
+                     "stdout": stdout,
+@@ -564,7 +569,7 @@ class SSH(MultiprocessingStateMixin):
+                 "stderr": stderr,
+                 "retcode": retcode,
+             }
+-        que.put(ret)
++        que.put((ret, retcode))
+
+     def handle_ssh(self, mine=False):
+         """
+@@ -608,7 +613,7 @@ class SSH(MultiprocessingStateMixin):
+                         "fun": "",
+                         "id": host,
+                     }
+-                    yield {host: no_ret}
++                    yield {host: no_ret}, 1
+                     continue
+                 args = (
+                     que,
+@@ -622,11 +627,12 @@ class SSH(MultiprocessingStateMixin):
+                 running[host] = {"thread": routine}
+                 continue
+             ret = {}
++            retcode = 0
+             try:
+-                ret = que.get(False)
++                ret, retcode = que.get(False)
+                 if "id" in ret:
+                     returned.add(ret["id"])
+-                    yield {ret["id"]: ret["ret"]}
++                    yield {ret["id"]: ret["ret"]}, retcode
+             except queue.Empty:
+                 pass
+             for host in running:
+@@ -636,10 +642,10 @@ class SSH(MultiprocessingStateMixin):
+                         # last checked
+                         try:
+                             while True:
+-                                ret = que.get(False)
++                                ret, retcode = que.get(False)
+                                 if "id" in ret:
+                                     returned.add(ret["id"])
+-                                    yield {ret["id"]: ret["ret"]}
++                                    yield {ret["id"]: ret["ret"]}, retcode
+                         except queue.Empty:
+                             pass
+
+@@ -650,7 +656,7 @@ class SSH(MultiprocessingStateMixin):
+                             )
+                             ret = {"id": host, "ret": error}
+                             log.error(error)
+-                            yield {ret["id"]: ret["ret"]}
++                            yield {ret["id"]: ret["ret"]}, 1
+                     running[host]["thread"].join()
+                     rets.add(host)
+             for host in rets:
+@@ -705,8 +711,8 @@ class SSH(MultiprocessingStateMixin):
+                 jid, job_load
+             )
+
+-        for ret in self.handle_ssh(mine=mine):
+-            host = next(iter(ret.keys()))
++        for ret, _ in self.handle_ssh(mine=mine):
++            host = next(iter(ret))
+             self.cache_job(jid, host, ret[host], fun)
+             if self.event:
+                 id_, data = next(iter(ret.items()))
+@@ -799,15 +805,9 @@ class SSH(MultiprocessingStateMixin):
+         sret = {}
+         outputter = self.opts.get("output", "nested")
+         final_exit = 0
+-        for ret in self.handle_ssh():
+-            host = next(iter(ret.keys()))
+-            if isinstance(ret[host], dict):
+-                host_ret = ret[host].get("retcode", 0)
+-                if host_ret != 0:
+-                    final_exit = 1
+-            else:
+-                # Error on host
+-                final_exit = 1
++        for ret, retcode in self.handle_ssh():
++            host = next(iter(ret))
++            final_exit = max(final_exit, retcode)
+
+             self.cache_job(jid, host, ret[host], fun)
+             ret = self.key_deploy(host, ret)
+@@ -1274,6 +1274,10 @@ class Single:
+             )
+             log.error(result, exc_info_on_loglevel=logging.DEBUG)
+             retcode = 1
++
++        # Ensure retcode from wrappers is respected, especially state render exceptions
++        retcode = max(retcode, self.context.get("retcode", 0))
++
+         # Mimic the json data-structure that "salt-call --local" will
+         # emit (as seen in ssh_py_shim.py)
+         if isinstance(result, dict) and "local" in result:
+diff --git a/salt/client/ssh/wrapper/state.py b/salt/client/ssh/wrapper/state.py
+index 002853972a..0a1d5bdf5f 100644
+--- a/salt/client/ssh/wrapper/state.py
++++ b/salt/client/ssh/wrapper/state.py
+@@ -8,6 +8,7 @@ import time
+
+ import salt.client.ssh.shell
+ import salt.client.ssh.state
++import salt.defaults.exitcodes
+ import salt.loader
+ import salt.minion
+ import salt.roster
+@@ -84,14 +85,14 @@ def _set_retcode(ret, highstate=None):
+     """
+
+     # Set default retcode to 0
+-    __context__["retcode"] = 0
++    __context__["retcode"] = salt.defaults.exitcodes.EX_OK
+
+     if isinstance(ret, list):
+-        __context__["retcode"] = 1
++        __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+         return
+     if not salt.utils.state.check_result(ret, highstate=highstate):
+
+-        __context__["retcode"] = 2
++        __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_FAILURE
+
+
+ def _check_pillar(kwargs, pillar=None):
+@@ -182,6 +183,11 @@ def sls(mods, saltenv="base", test=None, exclude=None, **kwargs):
+         __context__["fileclient"],
+         context=__context__.value(),
+     ) as st_:
++        if not _check_pillar(kwargs, st_.opts["pillar"]):
++            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
++            err = ["Pillar failed to render with the following messages:"]
++            err += st_.opts["pillar"]["_errors"]
++            return err
+         st_.push_active()
+         mods = _parse_mods(mods)
+         high_data, errors = st_.render_highstate(
+@@ -198,12 +204,14 @@ def sls(mods, saltenv="base", test=None, exclude=None, **kwargs):
+         errors += ext_errors
+         errors += st_.state.verify_high(high_data)
+         if errors:
++            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+             return errors
+         high_data, req_in_errors = st_.state.requisite_in(high_data)
+         errors += req_in_errors
+         high_data = st_.state.apply_exclude(high_data)
+         # Verify that the high data is structurally sound
+         if errors:
++            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+             return errors
+         # Compile and verify the raw chunks
+         chunks = st_.state.compile_high_data(high_data)
+@@ -316,7 +324,7 @@ def _check_queue(queue, kwargs):
+     else:
+         conflict = running(concurrent=kwargs.get("concurrent", False))
+         if conflict:
+-            __context__["retcode"] = 1
++            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+             return conflict
+
+
+@@ -681,6 +689,11 @@ def highstate(test=None, **kwargs):
+         __context__["fileclient"],
+         context=__context__.value(),
+     ) as st_:
++        if not _check_pillar(kwargs, st_.opts["pillar"]):
++            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
++            err = ["Pillar failed to render with the following messages:"]
++            err += st_.opts["pillar"]["_errors"]
++            return err
+         st_.push_active()
+         chunks = st_.compile_low_chunks(context=__context__.value())
+         file_refs = salt.client.ssh.state.lowstate_file_refs(
+@@ -692,7 +705,7 @@ def highstate(test=None, **kwargs):
+         # Check for errors
+         for chunk in chunks:
+             if not isinstance(chunk, dict):
+-                __context__["retcode"] = 1
++                __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+                 return chunks
+
+         roster = salt.roster.Roster(opts, opts.get("roster", "flat"))
+@@ -766,9 +779,19 @@ def top(topfn, test=None, **kwargs):
+         __context__["fileclient"],
+         context=__context__.value(),
+     ) as st_:
++        if not _check_pillar(kwargs, st_.opts["pillar"]):
++            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
++            err = ["Pillar failed to render with the following messages:"]
++            err += st_.opts["pillar"]["_errors"]
++            return err
+         st_.opts["state_top"] = os.path.join("salt://", topfn)
+         st_.push_active()
+         chunks = st_.compile_low_chunks(context=__context__.value())
++        # Check for errors
++        for chunk in chunks:
++            if not isinstance(chunk, dict):
++                __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
++                return chunks
+         file_refs = salt.client.ssh.state.lowstate_file_refs(
+             chunks,
+             _merge_extra_filerefs(
+@@ -839,8 +862,17 @@ def show_highstate(**kwargs):
+         __context__["fileclient"],
+         context=__context__.value(),
+     ) as st_:
++        if not _check_pillar(kwargs, st_.opts["pillar"]):
++            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
++            err = ["Pillar failed to render with the following messages:"]
++            err += st_.opts["pillar"]["_errors"]
++            return err
+         st_.push_active()
+         chunks = st_.compile_highstate(context=__context__.value())
++        # Check for errors
++        if not isinstance(chunks, dict):
++            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
++            return chunks
+         _cleanup_slsmod_high_data(chunks)
+         return chunks
+
+@@ -864,6 +896,11 @@ def show_lowstate(**kwargs):
+         __context__["fileclient"],
+         context=__context__.value(),
+     ) as st_:
++        if not _check_pillar(kwargs, st_.opts["pillar"]):
++            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
++            err = ["Pillar failed to render with the following messages:"]
++            err += st_.opts["pillar"]["_errors"]
++            return err
+         st_.push_active()
+         chunks = st_.compile_low_chunks(context=__context__.value())
+         _cleanup_slsmod_low_data(chunks)
+@@ -925,7 +962,7 @@ def sls_id(id_, mods, test=None, queue=False, **kwargs):
+     ) as st_:
+
+         if not _check_pillar(kwargs, st_.opts["pillar"]):
+-            __context__["retcode"] = 5
++            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
+             err = ["Pillar failed to render with the following messages:"]
+             err += __pillar__["_errors"]
+             return err
+@@ -943,7 +980,7 @@ def sls_id(id_, mods, test=None, queue=False, **kwargs):
+             # but it is required to get the unit tests to pass.
+             errors.extend(req_in_errors)
+         if errors:
+-            __context__["retcode"] = 1
++            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+             return errors
+         chunks = st_.state.compile_high_data(high_)
+         chunk = [x for x in chunks if x.get("__id__", "") == id_]
+@@ -988,6 +1025,11 @@ def show_sls(mods, saltenv="base", test=None, **kwargs):
+         __context__["fileclient"],
+         context=__context__.value(),
+     ) as st_:
++        if not _check_pillar(kwargs, st_.opts["pillar"]):
++            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
++            err = ["Pillar failed to render with the following messages:"]
++            err += st_.opts["pillar"]["_errors"]
++            return err
+         st_.push_active()
+         mods = _parse_mods(mods)
+         high_data, errors = st_.render_highstate(
+@@ -997,12 +1039,14 @@ def show_sls(mods, saltenv="base", test=None, **kwargs):
+         errors += ext_errors
+         errors += st_.state.verify_high(high_data)
+         if errors:
++            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+             return errors
+         high_data, req_in_errors = st_.state.requisite_in(high_data)
+         errors += req_in_errors
+         high_data = st_.state.apply_exclude(high_data)
+         # Verify that the high data is structurally sound
+         if errors:
++            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+             return errors
+         _cleanup_slsmod_high_data(high_data)
+         return high_data
+@@ -1036,6 +1080,11 @@ def show_low_sls(mods, saltenv="base", test=None, **kwargs):
+         __context__["fileclient"],
+         context=__context__.value(),
+     ) as st_:
++        if not _check_pillar(kwargs, st_.opts["pillar"]):
++            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
++            err = ["Pillar failed to render with the following messages:"]
++            err += st_.opts["pillar"]["_errors"]
++            return err
+         st_.push_active()
+         mods = _parse_mods(mods)
+         high_data, errors = st_.render_highstate(
+@@ -1045,12 +1094,14 @@ def show_low_sls(mods, saltenv="base", test=None, **kwargs):
+         errors += ext_errors
+         errors += st_.state.verify_high(high_data)
+         if errors:
++            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+             return errors
+         high_data, req_in_errors = st_.state.requisite_in(high_data)
+         errors += req_in_errors
+         high_data = st_.state.apply_exclude(high_data)
+         # Verify that the high data is structurally sound
+         if errors:
++            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+             return errors
+         ret = st_.state.compile_high_data(high_data)
+         _cleanup_slsmod_low_data(ret)
+@@ -1080,6 +1131,7 @@ def show_top(**kwargs):
+         errors = []
+         errors += st_.verify_tops(top_data)
+         if errors:
++            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+             return errors
+         matches = st_.top_matches(top_data)
+         return matches
+@@ -1110,7 +1162,7 @@ def single(fun, name, test=None, **kwargs):
+     # state.fun -> [state, fun]
+     comps = fun.split(".")
+     if len(comps) < 2:
+-        __context__["retcode"] = 1
++        __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+         return "Invalid function passed"
+
+     # Create the low chunk, using kwargs as a base
+@@ -1133,7 +1185,7 @@ def single(fun, name, test=None, **kwargs):
+     # Verify the low chunk
+     err = st_.verify_data(kwargs)
+     if err:
+-        __context__["retcode"] = 1
++        __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+         return err
+
+     # Must be a list of low-chunks
+-- 
+2.39.2
+

salt.spec.in

@@ -45,6 +45,7 @@ Patch2: 0002-Hide-known-DeprecationWarning-related-to-PEP-594.patch
 Patch3: 0003-Stop-using-the-deprecated-cgi-module.patch
 Patch4: 0004-Fixed-committed-reviewer-suggestion.patch
 Patch5: 0005-Stop-using-the-deprecated-pipes-module.patch
+Patch6: 0006-Fix-salt-ssh-state.-commands-retcode-for-render-fail.patch
 BuildArch: noarch
 
 %ifarch %{ix86} x86_64