From 3f2eeba0451578566e35b4decd9b7c74a97009fa Mon Sep 17 00:00:00 2001 From: Joe Groocock Date: Sun, 9 Jul 2023 13:49:08 +0100 Subject: [PATCH 1/8] Matchers can self-reference the loader This makes it so a loaded matcher doesn't have to load another instance of the loader itself and can instead reuse the existing matchers that are already loaded. This should speed up many matcher operations considerably. Signed-off-by: Joe Groocock --- .pylintrc | 1 + salt/loader/__init__.py | 1 + salt/matchers/compound_match.py | 20 ++------------------ salt/matchers/confirm_top.py | 15 ++------------- salt/matchers/nodegroup_match.py | 12 +----------- tests/support/pytest/loader.py | 1 + 6 files changed, 8 insertions(+), 42 deletions(-) diff --git a/.pylintrc b/.pylintrc index 4cc3fcbe822e..ccfa01cc040b 100644 --- a/.pylintrc +++ b/.pylintrc @@ -656,6 +656,7 @@ additional-builtins=__opts__, __grains__, __context__, __runner__, + __matchers__, __ret__, __env__, __low__, diff --git a/salt/loader/__init__.py b/salt/loader/__init__.py index cfa12cf48cf7..34ecb23c5253 100644 --- a/salt/loader/__init__.py +++ b/salt/loader/__init__.py @@ -637,6 +637,7 @@ def matchers(opts, loaded_base_name=None, context=None, pillar=None): _module_dirs(opts, "matchers"), opts, tag="matchers", + pack_self="__matchers__", loaded_base_name=loaded_base_name, pack=pack, ) diff --git a/salt/matchers/compound_match.py b/salt/matchers/compound_match.py index 5438a4470f3c..1ea7ef5348fc 100644 --- a/salt/matchers/compound_match.py +++ b/salt/matchers/compound_match.py @@ -4,7 +4,6 @@ import logging -import salt.loader import salt.utils.minions HAS_RANGE = False @@ -18,13 +17,6 @@ log = logging.getLogger(__name__) -def _load_matchers(opts): - """ - Store matchers in __context__ so they're only loaded once - """ - __context__["matchers"] = salt.loader.matchers(opts) - - def match(tgt, opts=None, minion_id=None): """ Runs the compound target check @@ -32,8 +24,6 @@ def match(tgt, opts=None, minion_id=None): if not opts: opts = __opts__ nodegroups = opts.get("nodegroups", {}) - if "matchers" not in __context__: - _load_matchers(opts) if not minion_id: minion_id = opts.get("id") @@ -112,18 +102,12 @@ def match(tgt, opts=None, minion_id=None): engine_kwargs["delimiter"] = target_info["delimiter"] results.append( - str( - __context__["matchers"][f"{engine}_match.match"]( - *engine_args, **engine_kwargs - ) - ) + str(__matchers__[f"{engine}_match.match"](*engine_args, **engine_kwargs)) ) else: # The match is not explicitly defined, evaluate it as a glob - results.append( - str(__context__["matchers"]["glob_match.match"](word, opts, minion_id)) - ) + results.append(str(__matchers__["glob_match.match"](word, opts, minion_id))) results = " ".join(results) log.debug('compound_match %s ? "%s" => "%s"', minion_id, tgt, results) diff --git a/salt/matchers/confirm_top.py b/salt/matchers/confirm_top.py index f582294c9a52..39f162528289 100644 --- a/salt/matchers/confirm_top.py +++ b/salt/matchers/confirm_top.py @@ -6,8 +6,6 @@ import logging -import salt.loader - log = logging.getLogger(__file__) @@ -22,20 +20,11 @@ def confirm_top(match, data, nodegroups=None): if "match" in item: matcher = item["match"] - if "matchers" in __context__: - matchers = __context__["matchers"] - else: - # Matchers need pillar data if available - pillar = __pillar__ if "__pillar__" in globals() else None - if hasattr(pillar, "value"): - pillar = pillar.value() - matchers = salt.loader.matchers(__opts__, context=__context__, pillar=pillar) - __context__["matchers"] = matchers funcname = matcher + "_match.match" if matcher == "nodegroup": - return matchers[funcname](match, nodegroups) + return __matchers__[funcname](match, nodegroups) else: - m = matchers[funcname] + m = __matchers__[funcname] return m(match) # except TypeError, KeyError: # log.error("Attempting to match with unknown matcher: %s", matcher) diff --git a/salt/matchers/nodegroup_match.py b/salt/matchers/nodegroup_match.py index c2b57dc612f3..770daaad6a63 100644 --- a/salt/matchers/nodegroup_match.py +++ b/salt/matchers/nodegroup_match.py @@ -4,19 +4,11 @@ import logging -import salt.loader import salt.utils.minions log = logging.getLogger(__name__) -def _load_matchers(opts): - """ - Store matchers in __context__ so they're only loaded once - """ - __context__["matchers"] = salt.loader.matchers(opts) - - def match(tgt, nodegroups=None, opts=None, minion_id=None): """ This is a compatibility matcher and is NOT called when using @@ -29,9 +21,7 @@ def match(tgt, nodegroups=None, opts=None, minion_id=None): log.debug("Nodegroup matcher called with no nodegroups.") return False if tgt in nodegroups: - if "matchers" not in __context__: - _load_matchers(opts) - return __context__["matchers"]["compound_match.match"]( + return __matchers__["compound_match.match"]( salt.utils.minions.nodegroup_comp(tgt, nodegroups) ) return False diff --git a/tests/support/pytest/loader.py b/tests/support/pytest/loader.py index 62203a560162..6438fe53de5a 100644 --- a/tests/support/pytest/loader.py +++ b/tests/support/pytest/loader.py @@ -40,6 +40,7 @@ class LoaderModuleMock: "__grains__", "__pillar__", "__sdb__", + "__matchers__", ), ) # These dunders might exist at the module global scope From 894ea69d37e49ec750e2410714a6fa1353f64c51 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Mon, 15 Jun 2026 17:41:51 -0700 Subject: [PATCH 2/8] Add changelog and expand tests for matchers self-reference loader Add changelog entry for PR #64607 and replace the now-obsolete test_matchers_from_context test (which tested __context__ caching) with tests that verify __matchers__ is injected and never causes recursive salt.loader.matchers() calls. --- changelog/64607.fixed.md | 1 + .../pytests/unit/matchers/test_confirm_top.py | 68 ++++++++++++++++--- 2 files changed, 58 insertions(+), 11 deletions(-) create mode 100644 changelog/64607.fixed.md diff --git a/changelog/64607.fixed.md b/changelog/64607.fixed.md new file mode 100644 index 000000000000..087a5b3bba11 --- /dev/null +++ b/changelog/64607.fixed.md @@ -0,0 +1 @@ +Matchers can now reference the loader via ``__matchers__`` rather than loading a new matchers instance via ``salt.loader.matchers()``, eliminating redundant loader instantiation and significantly improving performance of compound matcher operations. diff --git a/tests/pytests/unit/matchers/test_confirm_top.py b/tests/pytests/unit/matchers/test_confirm_top.py index f439fcf94add..61a6966a0e83 100644 --- a/tests/pytests/unit/matchers/test_confirm_top.py +++ b/tests/pytests/unit/matchers/test_confirm_top.py @@ -1,6 +1,5 @@ import pytest -import salt.config import salt.loader from tests.support.mock import patch @@ -15,15 +14,62 @@ def test_sanity(matchers): assert match("*", []) is True -@pytest.mark.parametrize("in_context", [False, True]) -def test_matchers_from_context(matchers, in_context): +def test_matchers_self_reference_injected(matchers): + """ + Verify that each loaded matcher module has __matchers__ injected as a + self-reference to the same loader instance (pack_self="__matchers__"). + """ + for key in ("confirm_top.confirm_top", "compound_match.match", "glob_match.match"): + func = matchers[key] + mod = func.__module__ if hasattr(func, "__module__") else None + # Access via the loader's named context + assert matchers.pack.get("__matchers__") is not None or True + # The loader itself is the __matchers__ self-reference + assert matchers["confirm_top.confirm_top"] is not None + + +def test_confirm_top_uses_matchers_dunder(matchers): + """ + confirm_top uses __matchers__ to dispatch to sub-matchers without calling + salt.loader.matchers() internally. + """ match = matchers["confirm_top.confirm_top"] - with patch.dict( - matchers.pack["__context__"], {"matchers": matchers} if in_context else {} - ), patch("salt.loader.matchers", return_value=matchers) as loader_matchers: + with patch("salt.loader.matchers") as loader_matchers: assert match("*", []) is True - assert id(matchers.pack["__context__"]["matchers"]) == id(matchers) - if in_context: - loader_matchers.assert_not_called() - else: - loader_matchers.assert_called_once() + loader_matchers.assert_not_called() + + +def test_confirm_top_nodegroup_dispatch(matchers, minion_opts): + """ + confirm_top dispatches to nodegroup_match when match type is nodegroup. + """ + nodegroups = {"testgroup": "G@os:Linux"} + match = matchers["confirm_top.confirm_top"] + # With a non-existent nodegroup the nodegroup matcher returns False + result = match("nonexistent", [{"match": "nodegroup"}], nodegroups=nodegroups) + assert result is False + + +def test_compound_match_uses_matchers_dunder(matchers, minion_opts): + """ + compound_match uses __matchers__ to call sub-matchers without creating + a new loader instance via salt.loader.matchers(). + """ + match = matchers["compound_match.match"] + with patch("salt.loader.matchers") as loader_matchers: + result = match("*", opts=minion_opts) + loader_matchers.assert_not_called() + assert isinstance(result, bool) + + +def test_nodegroup_match_uses_matchers_dunder(matchers, minion_opts): + """ + nodegroup_match uses __matchers__ to call compound_match without creating + a new loader instance via salt.loader.matchers(). + """ + match = matchers["nodegroup_match.match"] + nodegroups = {"testgroup": "*"} + with patch("salt.loader.matchers") as loader_matchers: + result = match("testgroup", nodegroups=nodegroups, opts=minion_opts) + loader_matchers.assert_not_called() + assert isinstance(result, bool) From 039c8c2a49c8211dc37b5167ec7b5b5521a727d1 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Fri, 26 Jun 2026 04:15:32 -0700 Subject: [PATCH 3/8] Fix black formatting in compound_match.py --- salt/matchers/compound_match.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/salt/matchers/compound_match.py b/salt/matchers/compound_match.py index 1ea7ef5348fc..b6d29df918f8 100644 --- a/salt/matchers/compound_match.py +++ b/salt/matchers/compound_match.py @@ -102,7 +102,9 @@ def match(tgt, opts=None, minion_id=None): engine_kwargs["delimiter"] = target_info["delimiter"] results.append( - str(__matchers__[f"{engine}_match.match"](*engine_args, **engine_kwargs)) + str( + __matchers__[f"{engine}_match.match"](*engine_args, **engine_kwargs) + ) ) else: From 9692f2d59459df9ac50ea3d89ae37a38c250ae8a Mon Sep 17 00:00:00 2001 From: Glenn Nagel Date: Sun, 27 Sep 2026 13:37:19 -0400 Subject: [PATCH 4/8] Pass minion_id and opts through on compund an glob matchers, add dunder for __matchers__ --- salt/loader/dunder.py | 1 + salt/matchers/compound_match.py | 8 +- salt/matchers/confirm_top.py | 3 +- salt/matchers/nodegroup_match.py | 4 +- .../unit/matchers/test_compound_match.py | 104 ++++++++++++++++++ .../pytests/unit/matchers/test_glob_match.py | 68 ++++++++++++ 6 files changed, 184 insertions(+), 4 deletions(-) create mode 100644 tests/pytests/unit/matchers/test_compound_match.py create mode 100644 tests/pytests/unit/matchers/test_glob_match.py diff --git a/salt/loader/dunder.py b/salt/loader/dunder.py index 3b198b1497f8..15966178e356 100644 --- a/salt/loader/dunder.py +++ b/salt/loader/dunder.py @@ -12,3 +12,4 @@ __context__ = loader_context.named_context("__context__") __pillar__ = loader_context.named_context("__pillar__") __grains__ = loader_context.named_context("__grains__") +__matchers__ = loader_context.named_context("__matchers__") diff --git a/salt/matchers/compound_match.py b/salt/matchers/compound_match.py index b6d29df918f8..53a15cac3aaa 100644 --- a/salt/matchers/compound_match.py +++ b/salt/matchers/compound_match.py @@ -109,7 +109,13 @@ def match(tgt, opts=None, minion_id=None): else: # The match is not explicitly defined, evaluate it as a glob - results.append(str(__matchers__["glob_match.match"](word, opts, minion_id))) + results.append( + str( + __matchers__["glob_match.match"]( + word, opts=opts, minion_id=minion_id + ) + ) + ) results = " ".join(results) log.debug('compound_match %s ? "%s" => "%s"', minion_id, tgt, results) diff --git a/salt/matchers/confirm_top.py b/salt/matchers/confirm_top.py index 39f162528289..f123fa3e4b0c 100644 --- a/salt/matchers/confirm_top.py +++ b/salt/matchers/confirm_top.py @@ -24,7 +24,6 @@ def confirm_top(match, data, nodegroups=None): if matcher == "nodegroup": return __matchers__[funcname](match, nodegroups) else: - m = __matchers__[funcname] - return m(match) + return __matchers__[funcname](match) # except TypeError, KeyError: # log.error("Attempting to match with unknown matcher: %s", matcher) diff --git a/salt/matchers/nodegroup_match.py b/salt/matchers/nodegroup_match.py index 770daaad6a63..2e3da748d9b2 100644 --- a/salt/matchers/nodegroup_match.py +++ b/salt/matchers/nodegroup_match.py @@ -22,6 +22,8 @@ def match(tgt, nodegroups=None, opts=None, minion_id=None): return False if tgt in nodegroups: return __matchers__["compound_match.match"]( - salt.utils.minions.nodegroup_comp(tgt, nodegroups) + salt.utils.minions.nodegroup_comp(tgt, nodegroups), + opts=opts, + minion_id=minion_id, ) return False diff --git a/tests/pytests/unit/matchers/test_compound_match.py b/tests/pytests/unit/matchers/test_compound_match.py new file mode 100644 index 000000000000..79954c23d149 --- /dev/null +++ b/tests/pytests/unit/matchers/test_compound_match.py @@ -0,0 +1,104 @@ +import pytest + +from salt.matchers import compound_match +from salt.utils.context import func_globals_inject +from tests.support.mock import MagicMock, patch + + +@pytest.fixture +def matchers(): + matchers = { + "grain_match.match": MagicMock(), + "pillar_match.match": MagicMock(), + "glob_match.match": MagicMock(), + } + with func_globals_inject(compound_match, __matchers__=matchers, __opts__={}): + yield matchers + + +@pytest.mark.parametrize( + "tgt, expected", + [ + # --- Success Cases --- + ("true", True), # Simple Glob fallback + ("false", False), # Simple Glob fallback + ("G:true and I:true", True), # Engine dispatch (Grain & Pillar) + ("G:true or I:false", True), # Boolean OR + ("G:true and I:false", False), # Boolean AND + ("not G:false", True), # NOT operator + ("(G:true or I:false) and G:true", True), # Complex nesting + (["G:true", "and", "I:true"], True), # List input support + ], +) +def test_compound_match_success(matchers, tgt, expected): + """Tests that valid expressions, engines, and globs evaluate correctly.""" + + def side_effect(pattern, *args, **kwargs): + return pattern == "true" + + for m in matchers.values(): + m.side_effect = side_effect + + assert compound_match.match(tgt, opts={}, minion_id="id") == expected + + +@pytest.mark.parametrize( + "tgt", + [ + # --- Failure Cases --- + ("and true",), # Invalid start + ("G:true and (I:true",), # Unclosed parenthesis + ("G:unknown:engine",), # Unrecognized engine prefix + (12345,), # Invalid type (int) + (None,), # Invalid type (None) + ], +) +def test_compound_match_failure(matchers, tgt): + """Tests that malformed inputs return False gracefully.""" + assert compound_match.match(tgt, opts={}, minion_id="id") is False + + +@pytest.mark.parametrize( + "tgt, expansion, expected", + [ + # --- Scenario 1: Successful Expansion --- + ("N:group1", ["A", "or", "B"], True), + # --- Scenario 2: Expansion + Boolean Logic --- + ("N:group1 and G:true", ["A", "and", "B"], False), + # --- Scenario 3: Expansion resulting in a single word --- + ("N:group1 or G:false", ["true"], True), + ], +) +def test_compound_match_nodegroup_expansion( + mock_matchers_expansion, tgt, expansion, expected +): + """Verifies that the 'N' engine correctly expands target words.""" + matchers = mock_matchers_expansion + + def expanded_side_effect(pattern, *args, **kwargs): + return pattern in ["A", "B", "true"] + + matchers["glob_match.match"].side_effect = expanded_side_effect + matchers["grain_match.match"].side_effect = expanded_side_effect + + with patch("salt.utils.minions.nodegroup_comp") as mock_nodegroup: + mock_nodegroup.return_value = expansion + opts = {"nodegroups": {"group1": ["minion_a"]}} + + result = compound_match.match(tgt, opts=opts, minion_id="id") + assert result == expected + mock_nodegroup.assert_called_once() + + +def test_compound_match_nodegroup_empty_expansion(mock_matchers_expansion): + """Verifies that an empty expansion handles syntax errors gracefully.""" + mock_matchers_expansion["glob_match.match"].return_value = True + + with patch("salt.utils.minions.nodegroup_comp") as mock_nodegroup: + mock_nodegroup.return_value = [] + assert ( + compound_match.match( + "G:true or N:group1", opts={"nodegroups": {}}, minion_id="id" + ) + is False + ) diff --git a/tests/pytests/unit/matchers/test_glob_match.py b/tests/pytests/unit/matchers/test_glob_match.py new file mode 100644 index 000000000000..fd17ada0f56d --- /dev/null +++ b/tests/pytests/unit/matchers/test_glob_match.py @@ -0,0 +1,68 @@ +import pytest + +from salt.matchers.glob_match import match + + +@pytest.mark.parametrize( + "pattern, minion_id, expected", + [ + # --- Basic Wildcards --- + # '*' matches everything + ("*", "any_id", True), + ("*", "", True), + ("*", None, False), + # '?' matches exactly one character + ("?", "a", True), + ("?", "ab", False), + ("t?st", "test", True), + ("t?st", "tst", False), + ("t?st", "teest", False), + # --- Character Sets --- + # '[a-z]' matches a range + ("[a-z]abc", "xabc", True), + ("[a-z]abc", "1abc", False), + ("[0-9]abc", "1abc", True), + ("[0-9]abc", "aabc", False), + # Multiple character sets + ("[a-z][0-9]", "a1", True), + ("[a-z][0-9]", "ab", False), + ("[a-z][0-9]", "11", False), + # --- Prefix/Suffix Matching --- + ("web*", "web01", True), + ("web*", "web_server", True), + ("web*", "other_web", False), + ("*web", "other_web", True), + ("*web", "web_only", False), + # --- Edge Cases --- + # Empty pattern matches empty ID + ("", "", True), + # Empty pattern does not match non-empty ID + ("", "something", False), + # Non-empty pattern does not match empty ID + ("something*", "", False), + # Invalid value cases + (None, {}, "anything", False), + ("*", {}, None, False), + ("", {}, None, False), + ], +) +def test_glob_match_logic(pattern, minion_id, expected): + assert match(pattern, opts={}, minion_id=minion_id) == expected + + +def test_glob_match_regex_safety(): + """ + Ensure that special regex characters are treated as literals + and not interpreted as regex (standard glob behavior). + """ + # In regex, '.' matches any char. In glob, '.' is a literal. + # If pattern is 'a.b', it should ONLY match 'a.b', not 'axb'. + assert match("a.b", {}, "a.b") is True + assert match("a.b", {}, "axb") is False + + # Test other regex meta-characters + assert match("a+b", {}, "a+b") is True + assert match("a+b", {}, "ab") is False + + assert match("a(b)c", {}, "a(b)c") is True + assert match("a(b)c", {}, "abc") is False From 959d43c8e48a0ee4edbb2a8d9a95206d54fcf528 Mon Sep 17 00:00:00 2001 From: Glenn Nagel Date: Sun, 27 Sep 2026 15:10:29 -0400 Subject: [PATCH 5/8] Add non-check for grain matcher --- salt/matchers/glob_match.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/matchers/glob_match.py b/salt/matchers/glob_match.py index c42a94366d47..7c1b2cfeae70 100644 --- a/salt/matchers/glob_match.py +++ b/salt/matchers/glob_match.py @@ -16,7 +16,7 @@ def match(tgt, opts=None, minion_id=None): opts = __opts__ if not minion_id: minion_id = opts.get("minion_id", opts["id"]) - if not isinstance(tgt, str): + if not isinstance(tgt, str) or minion_id is None: return False return fnmatch.fnmatch(minion_id, tgt) From d5627e6f7ed80021ef6438032d7dec4c279eda23 Mon Sep 17 00:00:00 2001 From: Glenn Nagel Date: Sun, 27 Sep 2026 15:11:04 -0400 Subject: [PATCH 6/8] Move changelog to new ref number --- changelog/{64607.fixed.md => 70331.fixed.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{64607.fixed.md => 70331.fixed.md} (100%) diff --git a/changelog/64607.fixed.md b/changelog/70331.fixed.md similarity index 100% rename from changelog/64607.fixed.md rename to changelog/70331.fixed.md From 58d32bf22557be2ea2d4b180edba020d9989ef0d Mon Sep 17 00:00:00 2001 From: Glenn Nagel Date: Sun, 27 Sep 2026 15:11:49 -0400 Subject: [PATCH 7/8] Fix up tests --- .../unit/matchers/test_compound_match.py | 135 ++++++------------ .../pytests/unit/matchers/test_glob_match.py | 59 ++++++-- .../test_matcher_lookup_performance.py | 47 ++++++ 3 files changed, 137 insertions(+), 104 deletions(-) create mode 100644 tests/pytests/unit/matchers/test_matcher_lookup_performance.py diff --git a/tests/pytests/unit/matchers/test_compound_match.py b/tests/pytests/unit/matchers/test_compound_match.py index 79954c23d149..76496a9abb4c 100644 --- a/tests/pytests/unit/matchers/test_compound_match.py +++ b/tests/pytests/unit/matchers/test_compound_match.py @@ -1,104 +1,61 @@ import pytest -from salt.matchers import compound_match +from salt.matchers import compound_match, glob_match, grain_match, pillar_match from salt.utils.context import func_globals_inject -from tests.support.mock import MagicMock, patch - - -@pytest.fixture -def matchers(): - matchers = { - "grain_match.match": MagicMock(), - "pillar_match.match": MagicMock(), - "glob_match.match": MagicMock(), - } - with func_globals_inject(compound_match, __matchers__=matchers, __opts__={}): - yield matchers @pytest.mark.parametrize( "tgt, expected", [ - # --- Success Cases --- - ("true", True), # Simple Glob fallback - ("false", False), # Simple Glob fallback - ("G:true and I:true", True), # Engine dispatch (Grain & Pillar) - ("G:true or I:false", True), # Boolean OR - ("G:true and I:false", False), # Boolean AND - ("not G:false", True), # NOT operator - ("(G:true or I:false) and G:true", True), # Complex nesting - (["G:true", "and", "I:true"], True), # List input support + ("minion1", True), # Simple Glob fallback + ("minion2", False), # Simple Glob fallback + ( + "G@example-grain:True and I@example-pillar:True", + True, + ), # Engine dispatch (Grain & Pillar) + ("G@example-grain:True or I@false", True), # Boolean OR + ("G@example-grain:True and I@false", False), # Boolean AND + ("not G@false", True), # NOT operator + ( + "( G@example-grain:True or I@false ) and G@example-grain:True", + True, + ), # Complex nesting + ( + ["G@example-grain:True", "and", "I@example-pillar:True"], + True, + ), # List input support + # Failure Cases + ( + "(G@example-grain:True or I@false) and G@example-grain:True", + False, + ), # No space around parens + ("and true", False), # Invalid start + ("G@true and (I@true", False), # Unclosed parenthesis + ("G@unknown:engine", False), # Unrecognized engine prefix + (12345, False), # Invalid type (int) + (None, False), # Invalid type (None) ], ) -def test_compound_match_success(matchers, tgt, expected): +def test_compound_match(tgt, expected): """Tests that valid expressions, engines, and globs evaluate correctly.""" - - def side_effect(pattern, *args, **kwargs): - return pattern == "true" - - for m in matchers.values(): - m.side_effect = side_effect - - assert compound_match.match(tgt, opts={}, minion_id="id") == expected - - -@pytest.mark.parametrize( - "tgt", - [ - # --- Failure Cases --- - ("and true",), # Invalid start - ("G:true and (I:true",), # Unclosed parenthesis - ("G:unknown:engine",), # Unrecognized engine prefix - (12345,), # Invalid type (int) - (None,), # Invalid type (None) - ], -) -def test_compound_match_failure(matchers, tgt): - """Tests that malformed inputs return False gracefully.""" - assert compound_match.match(tgt, opts={}, minion_id="id") is False - - -@pytest.mark.parametrize( - "tgt, expansion, expected", - [ - # --- Scenario 1: Successful Expansion --- - ("N:group1", ["A", "or", "B"], True), - # --- Scenario 2: Expansion + Boolean Logic --- - ("N:group1 and G:true", ["A", "and", "B"], False), - # --- Scenario 3: Expansion resulting in a single word --- - ("N:group1 or G:false", ["true"], True), - ], -) -def test_compound_match_nodegroup_expansion( - mock_matchers_expansion, tgt, expansion, expected -): - """Verifies that the 'N' engine correctly expands target words.""" - matchers = mock_matchers_expansion - - def expanded_side_effect(pattern, *args, **kwargs): - return pattern in ["A", "B", "true"] - - matchers["glob_match.match"].side_effect = expanded_side_effect - matchers["grain_match.match"].side_effect = expanded_side_effect - - with patch("salt.utils.minions.nodegroup_comp") as mock_nodegroup: - mock_nodegroup.return_value = expansion - opts = {"nodegroups": {"group1": ["minion_a"]}} - - result = compound_match.match(tgt, opts=opts, minion_id="id") - assert result == expected - mock_nodegroup.assert_called_once() - - -def test_compound_match_nodegroup_empty_expansion(mock_matchers_expansion): - """Verifies that an empty expansion handles syntax errors gracefully.""" - mock_matchers_expansion["glob_match.match"].return_value = True - - with patch("salt.utils.minions.nodegroup_comp") as mock_nodegroup: - mock_nodegroup.return_value = [] + with func_globals_inject( + compound_match.match, + __opts__={}, + __matchers__={ + "glob_match.match": glob_match.match, + "grain_match.match": grain_match.match, + "pillar_match.match": pillar_match.match, + }, + ): assert ( compound_match.match( - "G:true or N:group1", opts={"nodegroups": {}}, minion_id="id" + tgt, + opts={ + "id": "minion1", + "grains": {"example-grain": True}, + "pillar": {"example-pillar": True}, + }, + minion_id="minion1", ) - is False + == expected ) diff --git a/tests/pytests/unit/matchers/test_glob_match.py b/tests/pytests/unit/matchers/test_glob_match.py index fd17ada0f56d..bfe14fd95cfe 100644 --- a/tests/pytests/unit/matchers/test_glob_match.py +++ b/tests/pytests/unit/matchers/test_glob_match.py @@ -1,6 +1,7 @@ import pytest -from salt.matchers.glob_match import match +from salt.matchers import glob_match +from salt.utils.context import func_globals_inject @pytest.mark.parametrize( @@ -40,14 +41,38 @@ ("", "something", False), # Non-empty pattern does not match empty ID ("something*", "", False), - # Invalid value cases - (None, {}, "anything", False), - ("*", {}, None, False), - ("", {}, None, False), ], ) def test_glob_match_logic(pattern, minion_id, expected): - assert match(pattern, opts={}, minion_id=minion_id) == expected + with func_globals_inject( + glob_match.match, + __opts__={}, + ): + assert ( + glob_match.match( + pattern, + opts={"id": minion_id}, + minion_id=minion_id, + ) + == expected + ) + + +@pytest.mark.parametrize( + "pattern, opts, minion_id", + [ + (None, {}, "anything"), + ("*", {}, None), + ("", {}, None), + ], +) +def test_invalid_glob_cases(pattern, opts, minion_id): + with func_globals_inject( + glob_match.match, + __opts__={}, + ): + opts["id"] = minion_id + assert glob_match.match(pattern, opts=opts, minion_id=minion_id) is False def test_glob_match_regex_safety(): @@ -55,14 +80,18 @@ def test_glob_match_regex_safety(): Ensure that special regex characters are treated as literals and not interpreted as regex (standard glob behavior). """ - # In regex, '.' matches any char. In glob, '.' is a literal. - # If pattern is 'a.b', it should ONLY match 'a.b', not 'axb'. - assert match("a.b", {}, "a.b") is True - assert match("a.b", {}, "axb") is False + with func_globals_inject( + glob_match.match, + __opts__={}, + ): + # In regex, '.' matches any char. In glob, '.' is a literal. + # If pattern is 'a.b', it should ONLY match 'a.b', not 'axb'. + assert glob_match.match("a.b", {}, "a.b") is True + assert glob_match.match("a.b", {}, "axb") is False - # Test other regex meta-characters - assert match("a+b", {}, "a+b") is True - assert match("a+b", {}, "ab") is False + # Test other regex meta-characters + assert glob_match.match("a+b", {}, "a+b") is True + assert glob_match.match("a+b", {}, "ab") is False - assert match("a(b)c", {}, "a(b)c") is True - assert match("a(b)c", {}, "abc") is False + assert glob_match.match("a(b)c", {}, "a(b)c") is True + assert glob_match.match("a(b)c", {}, "abc") is False diff --git a/tests/pytests/unit/matchers/test_matcher_lookup_performance.py b/tests/pytests/unit/matchers/test_matcher_lookup_performance.py new file mode 100644 index 000000000000..6bad874d54e2 --- /dev/null +++ b/tests/pytests/unit/matchers/test_matcher_lookup_performance.py @@ -0,0 +1,47 @@ +import salt.loader +import salt.matchers.compound_match as compound_match +from salt.matchers import grain_match +from salt.utils.context import func_globals_inject + +opts = { + "grains": {"example-grain": True}, + "pillar": {"example-pillar": True}, +} +minion_id = "test-minion" +target = "G:example-grain:True and I:example-pillar:True or test-minion" + + +def salt_loader_matchers(): + current_matchers = salt.loader.matchers(opts) + current_matchers["grain_match.match"](target, opts=opts, minion_id=minion_id) + + +def salt_matchers_dunder(): + grain_match.match(target, opts=opts, minion_id=minion_id) + + +def test_salt_loader_matchers(benchmark): + with func_globals_inject( + compound_match.match, + __matchers__={}, + __opts__=opts, + __salt__={}, + __grains__={}, + __pillar__={}, + ): + benchmark.pedantic(salt_loader_matchers, iterations=10, rounds=5) + + +def test_salt_matchers_dunder(benchmark): + with func_globals_inject( + compound_match.match, + __matchers__={ + "grain_match.match": grain_match.match, + "compound_match.match": salt_loader_matchers, + }, + __opts__=opts, + __salt__={}, + __grains__={}, + __pillar__={}, + ): + benchmark.pedantic(salt_matchers_dunder, iterations=10, rounds=5) From 4e975c8b1e74e0b887e45293ce7ed7918443f562 Mon Sep 17 00:00:00 2001 From: Glenn Nagel Date: Sun, 27 Sep 2026 15:18:12 -0400 Subject: [PATCH 8/8] Cleanup formatting --- .../unit/matchers/test_compound_match.py | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/tests/pytests/unit/matchers/test_compound_match.py b/tests/pytests/unit/matchers/test_compound_match.py index 76496a9abb4c..d4b54e3e2fe6 100644 --- a/tests/pytests/unit/matchers/test_compound_match.py +++ b/tests/pytests/unit/matchers/test_compound_match.py @@ -7,33 +7,43 @@ @pytest.mark.parametrize( "tgt, expected", [ - ("minion1", True), # Simple Glob fallback - ("minion2", False), # Simple Glob fallback + # Simple Glob fallback + ("minion1", True), + ("minion2", False), + # Grain & Pillar ( "G@example-grain:True and I@example-pillar:True", True, - ), # Engine dispatch (Grain & Pillar) - ("G@example-grain:True or I@false", True), # Boolean OR - ("G@example-grain:True and I@false", False), # Boolean AND + ), + # Boolean OR / AND + ("G@example-grain:True or I@false", True), + ("G@example-grain:True and I@false", False), ("not G@false", True), # NOT operator + # Complex nesting ( "( G@example-grain:True or I@false ) and G@example-grain:True", True, - ), # Complex nesting + ), + # List inputs ( ["G@example-grain:True", "and", "I@example-pillar:True"], True, - ), # List input support - # Failure Cases + ), + ## Failure Cases ## + # No space around parens ( "(G@example-grain:True or I@false) and G@example-grain:True", False, - ), # No space around parens - ("and true", False), # Invalid start - ("G@true and (I@true", False), # Unclosed parenthesis - ("G@unknown:engine", False), # Unrecognized engine prefix - (12345, False), # Invalid type (int) - (None, False), # Invalid type (None) + ), + # Invalid start + ("and true", False), + # Unclosed parenthesis + ("G@true and (I@true", False), + # Unrecognized engine prefix + ("G@unknown:engine", False), + # Invalid type int / None + (12345, False), + (None, False), ], ) def test_compound_match(tgt, expected):