Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/69730.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed the master logging ``Event iteration failed with exception: 'list' object has no attribute 'items'`` for every failing state compilation: the return of a failed compile is a list of error strings, not a mapping of state results, and the event tagger assumed a dict.
12 changes: 12 additions & 0 deletions salt/utils/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,18 @@ def _fire_ret_load_specific_fun(self, load, fun_index=0):
ret = load.get("return", {})
retcode = load["retcode"]

if not isinstance(ret, dict):
# A failing state compilation returns a list of error strings (or
# a plain string from some renderers) instead of a mapping of
# per-state results, so there are no state tags to fire sub
# events for.
log.debug(
"Skipping sub event for job %s: return is a %s, not a dict",
load.get("jid"),
type(ret).__name__,
)
return

try:
for tag, data in ret.items():
data["retcode"] = retcode
Expand Down
69 changes: 69 additions & 0 deletions tests/pytests/unit/utils/event/test_event.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import hashlib
import logging
import os
import stat
import time
Expand Down Expand Up @@ -340,3 +341,71 @@ 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)


@pytest.fixture
def ret_load_event(sock_dir):
with salt.utils.event.SaltEvent(
"master", str(sock_dir), opts={"transport": "zeromq"}, listen=False
) as event:
with patch.object(event, "fire_event") as fire_event:
yield event, fire_event


def test_fire_ret_load_list_return_skips_quietly_69730(ret_load_event, caplog):
"""
A failing state compilation returns a list of error strings rather than
a mapping of per-state results. fire_ret_load used to hand that list to
_fire_ret_load_specific_fun, which crashed on ret.items() and logged
"Event iteration failed with exception: 'list' object has no attribute
'items'" at ERROR for every failed compile. There are no state tags in
such a return, so it must be skipped without logging an error and
without firing sub events.
"""
event, fire_event = ret_load_event
# The exact shape the master receives for a failed state.apply compile:
# fun in SUB_EVENT, a non-zero retcode, and a list-of-errors return.
load = {
"id": "minion",
"jid": "20260706000000000000",
"fun": "state.sls",
"retcode": 1,
"return": ["Rendering SLS 'base:broken' failed: Jinja error"],
}
with caplog.at_level(logging.ERROR, logger="salt.utils.event"):
event.fire_ret_load(load)
assert "Event iteration failed" not in caplog.text
fire_event.assert_not_called()


def test_fire_ret_load_dict_return_still_fires_sub_events_69730(ret_load_event, caplog):
"""
Guard against overcorrection: a dict-shaped failing state return (the
normal case) must keep firing the per-tag failure events exactly as
before the non-dict guard was added. This passes with and without the
fix.
"""
event, fire_event = ret_load_event
tag = "file_|-broken_|-/etc/broken_|-managed"
load = {
"id": "minion",
"jid": "20260706000000000000",
"fun": "state.sls",
"retcode": 2,
"return": {tag: {"result": False, "comment": "no such file"}},
}
with caplog.at_level(logging.ERROR, logger="salt.utils.event"):
event.fire_ret_load(load)
assert "Event iteration failed" not in caplog.text
assert fire_event.call_count == 2
# old-style duplicate event: <state>.<func> tag
first_data, first_tag = fire_event.call_args_list[0][0]
assert first_tag == "file.managed"
assert first_data["retcode"] == 2
# namespaced job sub event, enriched with job metadata
second_data, second_tag = fire_event.call_args_list[1][0]
assert second_tag == "salt/job/20260706000000000000/sub/minion/error/state.sls"
assert second_data["jid"] == "20260706000000000000"
assert second_data["id"] == "minion"
assert second_data["success"] is False
assert second_data["fun"] == "state.sls"
Loading