File improve-broken-events-catching-and-reporting.patch of Package salt

Overview Repositories Revisions Requests Users Attributes Meta

File improve-broken-events-catching-and-reporting.patch of Package salt

From 88bd54971d39b34d9728f3fe5fcb493cec3ff2fd Mon Sep 17 00:00:00 2001
From: Victor Zhestkov <vzhestkov@suse.com>
Date: Wed, 15 May 2024 09:22:11 +0200
Subject: [PATCH] Improve broken events catching and reporting

* Improve broken events catching and reporting

* Add test of catching SaltDeserializationError on reading event

* Add test for fire_ret_load
---
 salt/utils/event.py                          |  23 +++-
 tests/pytests/unit/utils/event/test_event.py | 107 +++++++++++++++++++
 2 files changed, 128 insertions(+), 2 deletions(-)

diff --git a/salt/utils/event.py b/salt/utils/event.py
index e6d7b00520..ef048335ae 100644
--- a/salt/utils/event.py
+++ b/salt/utils/event.py
@@ -75,6 +75,7 @@ import salt.utils.platform
 import salt.utils.process
 import salt.utils.stringutils
 import salt.utils.zeromq
+from salt.exceptions import SaltDeserializationError
 
 log = logging.getLogger(__name__)
 
@@ -461,7 +462,13 @@ class SaltEvent:
             salt.utils.stringutils.to_bytes(TAGEND)
         )  # split tag from data
         mtag = salt.utils.stringutils.to_str(mtag)
-        data = salt.payload.loads(mdata, encoding="utf-8")
+        try:
+            data = salt.payload.loads(mdata, encoding="utf-8")
+        except SaltDeserializationError:
+            log.warning(
+                "SaltDeserializationError on unpacking data, the payload could be incomplete"
+            )
+            raise
         return mtag, data
 
     def _get_match_func(self, match_type=None):
@@ -583,6 +590,9 @@ class SaltEvent:
                     raise
                 else:
                     return None
+            except SaltDeserializationError:
+                log.error("Unable to deserialize received event")
+                return None
             except RuntimeError:
                 return None
 
@@ -889,6 +899,14 @@ class SaltEvent:
             ret = load.get("return", {})
             retcode = load["retcode"]
 
+        if not isinstance(ret, dict):
+            log.error(
+                "Event with bad payload received from '%s': %s",
+                load.get("id", "UNKNOWN"),
+                "".join(ret) if isinstance(ret, list) else ret,
+            )
+            return
+
         try:
             for tag, data in ret.items():
                 data["retcode"] = retcode
@@ -910,7 +928,8 @@ class SaltEvent:
                     )
         except Exception as exc:  # pylint: disable=broad-except
             log.error(
-                "Event iteration failed with exception: %s",
+                "Event from '%s' iteration failed with exception: %s",
+                load.get("id", "UNKNOWN"),
                 exc,
                 exc_info_on_loglevel=logging.DEBUG,
             )
diff --git a/tests/pytests/unit/utils/event/test_event.py b/tests/pytests/unit/utils/event/test_event.py
index f4b6c15999..3eadfaf6ba 100644
--- a/tests/pytests/unit/utils/event/test_event.py
+++ b/tests/pytests/unit/utils/event/test_event.py
@@ -12,6 +12,7 @@ import salt.ext.tornado.ioloop
 import salt.ext.tornado.iostream
 import salt.utils.event
 import salt.utils.stringutils
+from salt.exceptions import SaltDeserializationError
 from salt.utils.event import SaltEvent
 from tests.support.events import eventpublisher_process, eventsender_process
 from tests.support.mock import patch
@@ -340,3 +341,109 @@ def test_master_pub_permissions(sock_dir):
         assert bool(os.lstat(p).st_mode & stat.S_IRUSR)
         assert not bool(os.lstat(p).st_mode & stat.S_IRGRP)
         assert not bool(os.lstat(p).st_mode & stat.S_IROTH)
+
+
+def test_event_unpack_with_SaltDeserializationError(sock_dir):
+    with eventpublisher_process(str(sock_dir)), salt.utils.event.MasterEvent(
+        str(sock_dir), listen=True
+    ) as me, patch.object(
+        salt.utils.event.log, "warning", autospec=True
+    ) as mock_log_warning, patch.object(
+        salt.utils.event.log, "error", autospec=True
+    ) as mock_log_error:
+        me.fire_event({"data": "foo1"}, "evt1")
+        me.fire_event({"data": "foo2"}, "evt2")
+        evt2 = me.get_event(tag="")
+        with patch("salt.payload.loads", side_effect=SaltDeserializationError):
+            evt1 = me.get_event(tag="")
+        _assert_got_event(evt2, {"data": "foo2"}, expected_failure=True)
+        assert evt1 is None
+        assert (
+            mock_log_warning.mock_calls[0].args[0]
+            == "SaltDeserializationError on unpacking data, the payload could be incomplete"
+        )
+        assert (
+            mock_log_error.mock_calls[0].args[0]
+            == "Unable to deserialize received event"
+        )
+
+
+def test_event_fire_ret_load():
+    event = SaltEvent(node=None)
+    test_load = {
+        "id": "minion_id.example.org",
+        "jid": "20240212095247760376",
+        "fun": "state.highstate",
+        "retcode": 254,
+        "return": {
+            "saltutil_|-sync_states_|-sync_states_|-sync_states": {
+                "result": True,
+            },
+            "saltutil_|-sync_modules_|-sync_modules_|-sync_modules": {
+                "result": False,
+            },
+        },
+    }
+    test_fire_event_data = {
+        "result": False,
+        "retcode": 254,
+        "jid": "20240212095247760376",
+        "id": "minion_id.example.org",
+        "success": False,
+        "return": "Error: saltutil.sync_modules",
+        "fun": "state.highstate",
+    }
+    test_unhandled_exc = "Unhandled exception running state.highstate"
+    test_traceback = [
+        "Traceback (most recent call last):\n",
+        "    Just an example of possible return as a list\n",
+    ]
+    with patch.object(
+        event, "fire_event", side_effect=[None, None, Exception]
+    ) as mock_fire_event, patch.object(
+        salt.utils.event.log, "error", autospec=True
+    ) as mock_log_error:
+        event.fire_ret_load(test_load)
+        assert len(mock_fire_event.mock_calls) == 2
+        assert mock_fire_event.mock_calls[0].args[0] == test_fire_event_data
+        assert mock_fire_event.mock_calls[0].args[1] == "saltutil.sync_modules"
+        assert mock_fire_event.mock_calls[1].args[0] == test_fire_event_data
+        assert (
+            mock_fire_event.mock_calls[1].args[1]
+            == "salt/job/20240212095247760376/sub/minion_id.example.org/error/state.highstate"
+        )
+        assert not mock_log_error.mock_calls
+
+        mock_log_error.reset_mock()
+
+        event.fire_ret_load(test_load)
+        assert (
+            mock_log_error.mock_calls[0].args[0]
+            == "Event from '%s' iteration failed with exception: %s"
+        )
+        assert mock_log_error.mock_calls[0].args[1] == "minion_id.example.org"
+
+        mock_log_error.reset_mock()
+        test_load["return"] = test_unhandled_exc
+
+        event.fire_ret_load(test_load)
+        assert (
+            mock_log_error.mock_calls[0].args[0]
+            == "Event with bad payload received from '%s': %s"
+        )
+        assert mock_log_error.mock_calls[0].args[1] == "minion_id.example.org"
+        assert (
+            mock_log_error.mock_calls[0].args[2]
+            == "Unhandled exception running state.highstate"
+        )
+
+        mock_log_error.reset_mock()
+        test_load["return"] = test_traceback
+
+        event.fire_ret_load(test_load)
+        assert (
+            mock_log_error.mock_calls[0].args[0]
+            == "Event with bad payload received from '%s': %s"
+        )
+        assert mock_log_error.mock_calls[0].args[1] == "minion_id.example.org"
+        assert mock_log_error.mock_calls[0].args[2] == "".join(test_traceback)
-- 
2.45.0

Places

File improve-broken-events-catching-and-reporting.patch of Package salt

Places