From ed841647465afcf6bfaeb3fcf63d915aa8e54623 Mon Sep 17 00:00:00 2001 From: SAY-5 Date: Tue, 21 Apr 2026 00:17:03 -0700 Subject: [PATCH] runners/jobs: don't iterate non-iterable Target values in list_jobs Failed or aborted jobs can leave Target as None or some other non-iterable value in the job cache. list_jobs used to iterate it unconditionally, raising TypeError and aborting the whole listing, which made it impossible to even see the malformed entries in order to fix them. Replace the explicit (list, tuple, set) instance check with a single collections.abc.Iterable check per @bdrx312's review feedback - that covers any future iterable container the cache may grow without re-listing every type. Strings are pulled out beforehand so a single target string still gets wrapped in a list rather than being treated as a sequence of characters. When a non-iterable Target is hit, log it at DEBUG with the offending job id so operators have a breadcrumb back to the malformed cache row, rather than the previous silent skip. Add a regression test that mixes a normal job entry with one carrying Target=None, asserts the normal one is still matched, the bad one is filtered out, and the debug log carries the bad jid. The test fails on master with a missing log entry (because the old code silently skipped), confirming both behaviour changes. Closes #68780 Signed-off-by: SAY-5 --- changelog/68780.fixed.md | 4 ++ salt/runners/jobs.py | 12 ++++++ tests/pytests/unit/runners/test_jobs.py | 54 +++++++++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 changelog/68780.fixed.md diff --git a/changelog/68780.fixed.md b/changelog/68780.fixed.md new file mode 100644 index 000000000000..4955db48a7f6 --- /dev/null +++ b/changelog/68780.fixed.md @@ -0,0 +1,4 @@ +Fix `jobs.list_jobs` crashing with a `TypeError` when the job cache has a +`Target` value that is neither a string nor an iterable (e.g. `None` from a +failed or aborted job). Non-iterable targets are now treated as no target +so the remaining `search_target` filtering keeps working. diff --git a/salt/runners/jobs.py b/salt/runners/jobs.py index f1ff0f8b4921..3911388338d0 100644 --- a/salt/runners/jobs.py +++ b/salt/runners/jobs.py @@ -2,6 +2,7 @@ A convenience system to manage jobs, both active and already run """ +import collections.abc import fnmatch import logging import os @@ -334,6 +335,17 @@ def list_jobs( targets = ret[item]["Target"] if isinstance(targets, str): targets = [targets] + elif not isinstance(targets, collections.abc.Iterable): + # Failed/aborted jobs can leave Target as None or some + # other non-iterable value in the cache; iterating would + # raise TypeError. Log once at debug level and treat as + # no target so the rest of list_jobs keeps working. + log.debug( + "list_jobs: ignoring non-iterable Target %r on jid %s", + targets, + item, + ) + targets = [] for target in targets: for key in salt.utils.args.split_input(search_target): if fnmatch.fnmatch(target, key): diff --git a/tests/pytests/unit/runners/test_jobs.py b/tests/pytests/unit/runners/test_jobs.py index 8d9fe3853a77..70fb980adeaa 100644 --- a/tests/pytests/unit/runners/test_jobs.py +++ b/tests/pytests/unit/runners/test_jobs.py @@ -70,3 +70,57 @@ def __init__(self, *args, **kwargs): assert jobs.list_jobs(search_target="node-1-2.com") == returns["node-1-2.com"] assert jobs.list_jobs(search_target="non-existant") == returns["non-existant"] + + +def test_list_jobs_with_non_iterable_target(caplog): + """ + Regression for #68780: failed/aborted jobs can leave Target as None or + some other non-iterable value in the job cache. list_jobs used to + iterate blindly and crash with TypeError; it must now skip those + entries and continue filtering the rest of the cache. + """ + mock_jobs_cache = { + # Valid entry with a real target - must still be matched. + "20260421000000000001": { + "Arguments": [], + "Function": "test.ping", + "StartTime": "2026, Apr 21 00:00:00.000001", + "Target": "node-1-1.com", + "Target-type": "glob", + "User": "root", + }, + # Non-iterable Target (None is the real-world case from the bug + # report). Must not trip list_jobs, and must be logged at debug. + "20260421000000000002": { + "Arguments": [], + "Function": "test.ping", + "StartTime": "2026, Apr 21 00:00:00.000002", + "Target": None, + "Target-type": "glob", + "User": "root", + }, + } + + def return_mock_jobs(): + return mock_jobs_cache + + class MockMasterMinion: + returners = {"local_cache.get_jids": return_mock_jobs} + + def __init__(self, *args, **kwargs): + pass + + with patch.object(salt.minion, "MasterMinion", MockMasterMinion): + import logging + + with caplog.at_level(logging.DEBUG, logger="salt.runners.jobs"): + # Must return the entry that has a valid target without raising + # or masking the match quietly. + result = jobs.list_jobs(search_target="node-1-1.com") + + assert "20260421000000000001" in result + assert "20260421000000000002" not in result + # The skipped job id should appear in the debug log so operators + # can find and fix the malformed cache row instead of discovering + # it via a crash. + assert "20260421000000000002" in caplog.text