zypperpkg: stop logging a spurious ERROR on zypper exit code 104 (nothing found)

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.

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")
     ):

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):
         </solvable-list></search-result></stream>
                 """
         _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):
         </solvable-list></search-result></stream>
         """
         _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):
         </solvable-list></search-result></stream>
         """
         _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):
         </solvable-list></search-result></stream>
         """
         _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):
         </solvable-list></search-result></stream>
         """
         _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):
         </solvable-list></search-result></stream>
         """
         _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": "<?xml version='1.0'?><stream></stream>",
+            "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": "<?xml version='1.0'?><stream></stream>",
+            "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