diff --git a/changelog/58551.fixed.md b/changelog/58551.fixed.md new file mode 100644 index 00000000000..3529ab1b5d2 --- /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 dd580963ec8..c4720a7f4f3 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 ec096214eea..b9b152a65ae 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