From b1b919dc93b1ccfe34104491b74dc9e271729406 Mon Sep 17 00:00:00 2001 From: "Gary T. Giesen" Date: Fri, 10 Jul 2026 12:52:35 -0400 Subject: [PATCH] Silence spurious ERROR log from zypperpkg search on exit code 104 zypper returns exit code 104 when a search matches nothing. The search-style calls in zypperpkg (search, _get_visible_patterns, _get_patches, and Wildcard._get_available_versions) run through cmd.run_all, whose default success_retcodes is [0], so the 104 was logged as a command failure even though the search itself completed normally and correctly reported that nothing was found. Add an ignore_not_found option to the _Zypper wrapper that whitelists 104 via success_retcodes for those search calls, so the exit code is treated as success and no ERROR is emitted. The found case (0) and all genuine error codes are left untouched. Fixes #58551 --- changelog/58551.fixed.md | 1 + salt/modules/zypperpkg.py | 25 +++++-- tests/unit/modules/test_zypperpkg.py | 106 ++++++++++++++++++++++++--- 3 files changed, 116 insertions(+), 16 deletions(-) create mode 100644 changelog/58551.fixed.md diff --git a/changelog/58551.fixed.md b/changelog/58551.fixed.md new file mode 100644 index 000000000000..3529ab1b5d24 --- /dev/null +++ b/changelog/58551.fixed.md @@ -0,0 +1 @@ +Stopped zypperpkg search functions from logging a spurious ERROR when zypper exits with code 104 (nothing found); the 104 exit code is now whitelisted for search-style calls. diff --git a/salt/modules/zypperpkg.py b/salt/modules/zypperpkg.py index dd580963ec8f..c4720a7f4f32 100644 --- a/salt/modules/zypperpkg.py +++ b/salt/modules/zypperpkg.py @@ -102,6 +102,7 @@ class _Zypper: } LOCK_EXIT_CODE = 7 + NOT_FOUND_EXIT_CODE = 104 XML_DIRECTIVES = ["-x", "--xmlout"] # ZYPPER_LOCK is not affected by --root ZYPPER_LOCK = "/var/run/zypp.pid" @@ -133,6 +134,7 @@ def _reset(self): self.__no_raise = False self.__refresh = False self.__ignore_repo_failure = False + self.__ignore_not_found = False self.__systemd_scope = False self.__root = None @@ -152,6 +154,9 @@ def __call__(self, *args, **kwargs): # Ignore exit code for 106 (repo is not available) if "no_repo_failure" in kwargs: self.__ignore_repo_failure = kwargs["no_repo_failure"] + # Ignore exit code for 104 (package not found) + if "ignore_not_found" in kwargs: + self.__ignore_not_found = kwargs["ignore_not_found"] if "systemd_scope" in kwargs: self.__systemd_scope = kwargs["systemd_scope"] if "root" in kwargs: @@ -332,6 +337,10 @@ def __call(self, *args, **kwargs): if self.__root: self.__cmd.extend(["--root", self.__root]) + # Do not consider 104 (nothing found) as a retcode error + if self.__ignore_not_found: + kwargs["success_retcodes"] = [_Zypper.NOT_FOUND_EXIT_CODE] + self.__cmd.extend(args) kwargs["output_loglevel"] = "trace" kwargs["python_shell"] = False @@ -476,9 +485,11 @@ def _get_available_versions(self): Get available versions of the package. :return: """ - solvables = self.zypper.nolock.xml.call( - "se", "-xv", self.name - ).getElementsByTagName("solvable") + solvables = ( + self.zypper(ignore_not_found=True) + .nolock.xml.call("se", "-xv", self.name) + .getElementsByTagName("solvable") + ) if not solvables: raise CommandExecutionError(f"No packages found matching '{self.name}'") @@ -2535,7 +2546,9 @@ def owner(*paths, **kwargs): def _get_visible_patterns(root=None): """Get all available patterns in the repo that are visible.""" patterns = {} - search_patterns = __zypper__(root=root).nolock.xml.call("se", "-t", "pattern") + search_patterns = __zypper__(root=root, ignore_not_found=True).nolock.xml.call( + "se", "-t", "pattern" + ) for element in search_patterns.getElementsByTagName("solvable"): installed = element.getAttribute("status") == "installed" patterns[element.getAttribute("name")] = { @@ -2732,7 +2745,7 @@ def search(criteria, refresh=False, **kwargs): cmd.append(criteria) solvables = ( - __zypper__(root=root) + __zypper__(root=root, ignore_not_found=True) .nolock.noraise.xml.call(*cmd) .getElementsByTagName("solvable") ) @@ -2984,7 +2997,7 @@ def _get_patches(installed_only=False, root=None): """ patches = {} for element in ( - __zypper__(root=root) + __zypper__(root=root, ignore_not_found=True) .nolock.xml.call("se", "-t", "patch") .getElementsByTagName("solvable") ): diff --git a/tests/unit/modules/test_zypperpkg.py b/tests/unit/modules/test_zypperpkg.py index ec096214eeaf..b9b152a65ae8 100644 --- a/tests/unit/modules/test_zypperpkg.py +++ b/tests/unit/modules/test_zypperpkg.py @@ -27,7 +27,10 @@ def __getattr__(self, item): def __call__(self, *args, **kwargs): # If the call is for a configuration modifier, we return self - if any(i in kwargs for i in ("no_repo_failure", "systemd_scope", "root")): + if any( + i in kwargs + for i in ("no_repo_failure", "ignore_not_found", "systemd_scope", "root") + ): return self return MagicMock(return_value=self.__return_value)() @@ -1662,7 +1665,9 @@ def test_wildcard_to_query_match_all(self): """ _zpr = MagicMock() - _zpr.nolock.xml.call = MagicMock(return_value=minidom.parseString(xmldoc)) + # _get_available_versions now calls self.zypper(ignore_not_found=True) + # before .nolock.xml.call, so configure the return_value chain. + _zpr.return_value.nolock.xml.call.return_value = minidom.parseString(xmldoc) wcard = zypper.Wildcard(_zpr) wcard.name, wcard.version = "libzypp", "*" assert wcard._get_scope_versions(wcard._get_available_versions()) == [ @@ -1685,7 +1690,9 @@ def test_wildcard_to_query_multiple_asterisk(self): """ _zpr = MagicMock() - _zpr.nolock.xml.call = MagicMock(return_value=minidom.parseString(xmldoc)) + # _get_available_versions now calls self.zypper(ignore_not_found=True) + # before .nolock.xml.call, so configure the return_value chain. + _zpr.return_value.nolock.xml.call.return_value = minidom.parseString(xmldoc) wcard = zypper.Wildcard(_zpr) wcard.name, wcard.version = "libzypp", "16.2.*-2*" assert wcard._get_scope_versions(wcard._get_available_versions()) == [ @@ -1707,7 +1714,9 @@ def test_wildcard_to_query_exact_match_at_end(self): """ _zpr = MagicMock() - _zpr.nolock.xml.call = MagicMock(return_value=minidom.parseString(xmldoc)) + # _get_available_versions now calls self.zypper(ignore_not_found=True) + # before .nolock.xml.call, so configure the return_value chain. + _zpr.return_value.nolock.xml.call.return_value = minidom.parseString(xmldoc) wcard = zypper.Wildcard(_zpr) wcard.name, wcard.version = "libzypp", "16.2.5*" assert wcard._get_scope_versions(wcard._get_available_versions()) == [ @@ -1728,7 +1737,9 @@ def test_wildcard_to_query_exact_match_at_beginning(self): """ _zpr = MagicMock() - _zpr.nolock.xml.call = MagicMock(return_value=minidom.parseString(xmldoc)) + # _get_available_versions now calls self.zypper(ignore_not_found=True) + # before .nolock.xml.call, so configure the return_value chain. + _zpr.return_value.nolock.xml.call.return_value = minidom.parseString(xmldoc) wcard = zypper.Wildcard(_zpr) wcard.name, wcard.version = "libzypp", "*.1" assert wcard._get_scope_versions(wcard._get_available_versions()) == [ @@ -1750,7 +1761,9 @@ def test_wildcard_to_query_usage(self): """ _zpr = MagicMock() - _zpr.nolock.xml.call = MagicMock(return_value=minidom.parseString(xmldoc)) + # _get_available_versions now calls self.zypper(ignore_not_found=True) + # before .nolock.xml.call, so configure the return_value chain. + _zpr.return_value.nolock.xml.call.return_value = minidom.parseString(xmldoc) assert zypper.Wildcard(_zpr)("libzypp", "16.2.4*") == "16.2.4-19.5" assert zypper.Wildcard(_zpr)("libzypp", "16.2*") == "16.2.5-25.1" assert zypper.Wildcard(_zpr)("libzypp", "*6-*") == "17.2.6-27.9.1" @@ -1770,7 +1783,9 @@ def test_wildcard_to_query_noversion(self): """ _zpr = MagicMock() - _zpr.nolock.xml.call = MagicMock(return_value=minidom.parseString(xmldoc)) + # _get_available_versions now calls self.zypper(ignore_not_found=True) + # before .nolock.xml.call, so configure the return_value chain. + _zpr.return_value.nolock.xml.call.return_value = minidom.parseString(xmldoc) assert zypper.Wildcard(_zpr)("libzypp", None) is None def test_wildcard_to_query_typecheck(self): @@ -1787,7 +1802,9 @@ def test_wildcard_to_query_typecheck(self): """ _zpr = MagicMock() - _zpr.nolock.xml.call = MagicMock(return_value=minidom.parseString(xmldoc)) + # _get_available_versions now calls self.zypper(ignore_not_found=True) + # before .nolock.xml.call, so configure the return_value chain. + _zpr.return_value.nolock.xml.call.return_value = minidom.parseString(xmldoc) assert isinstance(zypper.Wildcard(_zpr)("libzypp", "*.1"), str) def test_wildcard_to_query_condition_preservation(self): @@ -1804,7 +1821,9 @@ def test_wildcard_to_query_condition_preservation(self): """ _zpr = MagicMock() - _zpr.nolock.xml.call = MagicMock(return_value=minidom.parseString(xmldoc)) + # _get_available_versions now calls self.zypper(ignore_not_found=True) + # before .nolock.xml.call, so configure the return_value chain. + _zpr.return_value.nolock.xml.call.return_value = minidom.parseString(xmldoc) for op in zypper.Wildcard.Z_OP: assert zypper.Wildcard(_zpr)("libzypp", f"{op}*.1") == f"{op}17.2.6-27.9.1" @@ -1827,7 +1846,9 @@ def test_wildcard_to_query_unsupported_operators(self): """ _zpr = MagicMock() - _zpr.nolock.xml.call = MagicMock(return_value=minidom.parseString(xmldoc)) + # _get_available_versions now calls self.zypper(ignore_not_found=True) + # before .nolock.xml.call, so configure the return_value chain. + _zpr.return_value.nolock.xml.call.return_value = minidom.parseString(xmldoc) with self.assertRaises(CommandExecutionError): for op in [">>", "==", "<<", "+"]: zypper.Wildcard(_zpr)("libzypp", f"{op}*.1") @@ -1953,3 +1974,68 @@ def test_rpm_lock_not_acquirable(self): self.assertFalse(zypper.__zypper__._is_rpm_lock()) self.assertEqual(lockf_mock.call_count, 2) zypper.__zypper__._reset() + + def test_search(self): + """ + search() must request ignore_not_found so that zypper's 104 + (nothing found) exit code is whitelisted and not logged as an error. + """ + xml_mock = MagicMock(return_value=[]) + zypp_mock = MagicMock(return_value=xml_mock) + with patch("salt.modules.zypperpkg.__zypper__", zypp_mock): + zypper.search("emacs") + zypp_mock.assert_called_with(root=None, ignore_not_found=True) + xml_mock.nolock.noraise.xml.call.assert_called_with("search", "emacs") + + def test_search_not_found_58551(self): + """ + Regression test for issue #58551. + + When a search matches nothing, zypper exits with code 104. search() + passes ignore_not_found=True, so _Zypper whitelists 104 through + cmd.run_all's success_retcodes and no spurious ERROR is logged. The + empty result still raises CommandExecutionError as before. + """ + ret = { + "stdout": "", + "stderr": None, + "retcode": 104, + } + run_all_mock = MagicMock(return_value=ret) + with patch.dict(zypper.__salt__, {"cmd.run_all": run_all_mock}): + self.assertRaises(CommandExecutionError, zypper.search, "vim") + # ignore_not_found=True -> success_retcodes=[104] on the cmd.run_all call + run_all_mock.assert_called_with( + [ + "zypper", + "--non-interactive", + "--xmlout", + "--no-refresh", + "search", + "vim", + ], + success_retcodes=[104], + output_loglevel="trace", + python_shell=False, + env={"ZYPP_READONLY_HACK": "1"}, + ) + + def test_search_without_ignore_not_found_still_errors(self): + """ + Inverse must-not-regress guard for issue #58551. + + A zypper call that does NOT request ignore_not_found must not + whitelist retcode 104: cmd.run_all is invoked without a + success_retcodes kwarg, so a genuine non-zero retcode still surfaces + as an error. This passes with and without the fix, ensuring the 104 + whitelist never leaks into unrelated zypper calls. + """ + ret = { + "stdout": "", + "stderr": None, + "retcode": 0, + } + run_all_mock = MagicMock(return_value=ret) + with patch.dict(zypper.__salt__, {"cmd.run_all": run_all_mock}): + zypper._Zypper().xml.call("se", "-t", "pattern") + assert "success_retcodes" not in run_all_mock.call_args.kwargs