From fa5de69a77c321c045a267e9b7b26d154fe54634 Mon Sep 17 00:00:00 2001 From: guillaume pernot Date: Fri, 17 Oct 2025 20:17:49 +0200 Subject: [PATCH 1/7] Uniquify requisite_in'd states checks. Fix #68408 --- salt/state.py | 75 ++--- .../pytests/unit/state/test_add_to_extend.py | 256 ++++++++++++++++++ 2 files changed, 299 insertions(+), 32 deletions(-) create mode 100644 tests/pytests/unit/state/test_add_to_extend.py diff --git a/salt/state.py b/salt/state.py index d1f897439ae5..09a6917ff581 100644 --- a/salt/state.py +++ b/salt/state.py @@ -1903,6 +1903,41 @@ def apply_exclude(self, high): high.pop(id_) return high + @staticmethod + def _add_to_extend( + extend, + id_to_extend, + state_mod_name, + req_type, + req_id, + req_state_mod_name, + ): + """ + Add requisiste `req_id` as a requisite for `id_to_extend` + """ + + if id_to_extend not in extend: + # The state does not exist yet: create it + extend[id_to_extend] = HashableOrderedDict() + + if (req_states := extend[id_to_extend].get(req_state_mod_name)) is None: + # The requisited state is not present yet, create it and initialize it + extend[id_to_extend][req_state_mod_name] = [ + {req_type: [{state_mod_name: req_id}]} + ] + return + + # Lookup req_type if req_states + for state_arg in req_states: + if (req_items := state_arg.get(req_type)) is not None: + for req_item in req_items: + # (state_mode_name, req_id) is already defined as a requisiste + if req_item.get(state_mod_name) == req_id: + break + else: + # Extending again + state_arg[req_type].append({state_mod_name: req_id}) + def requisite_in(self, high): """ Extend the data reference with requisite_in arguments @@ -1953,11 +1988,7 @@ def requisite_in(self, high): if isinstance(items, dict): # Formatted as a single req_in for _state, name in items.items(): - # Not a use requisite_in - found = False - if name not in extend: - extend[name] = HashableOrderedDict() if "." in _state: errors.append( "Invalid requisite in {}: {} for " @@ -1971,21 +2002,13 @@ def requisite_in(self, high): ) ) _state = _state.split(".")[0] - if _state not in extend[name]: - extend[name][_state] = [] + found = self._add_to_extend( + extend, name, _state, rkey, id_, state + ) extend[name]["__env__"] = body["__env__"] extend[name]["__sls__"] = body["__sls__"] - for ind in range(len(extend[name][_state])): - if next(iter(extend[name][_state][ind])) == rkey: - # Extending again - extend[name][_state][ind][rkey].append( - {state: id_} - ) - found = True if found: continue - # The rkey is not present yet, create it - extend[name][_state].append({rkey: [{state: id_}]}) if isinstance(items, list): # Formed as a list of requisite additions @@ -2123,27 +2146,15 @@ def requisite_in(self, high): continue extend[id_][state].append(arg) continue - found = False - if name not in extend: - extend[name] = HashableOrderedDict() - if _state not in extend[name]: - extend[name][_state] = [] + found = self._add_to_extend( + extend, name, _state, rkey, id_, state + ) + extend[name]["__env__"] = body["__env__"] extend[name]["__sls__"] = body["__sls__"] - for ind in range(len(extend[name][_state])): - if ( - next(iter(extend[name][_state][ind])) - == rkey - ): - # Extending again - extend[name][_state][ind][rkey].append( - {state: id_} - ) - found = True + if found: continue - # The rkey is not present yet, create it - extend[name][_state].append({rkey: [{state: id_}]}) high["__extend__"] = [] for key, val in extend.items(): high["__extend__"].append({key: val}) diff --git a/tests/pytests/unit/state/test_add_to_extend.py b/tests/pytests/unit/state/test_add_to_extend.py new file mode 100644 index 000000000000..0d4cb8053cbb --- /dev/null +++ b/tests/pytests/unit/state/test_add_to_extend.py @@ -0,0 +1,256 @@ +import logging +from copy import deepcopy + +import pytest + +import salt.config +import salt.state +from salt.utils.odict import HashableOrderedDict + +log = logging.getLogger(__name__) + + +pytestmark = [ + pytest.mark.core_test, +] + + +@pytest.fixture +def minion_config(minion_opts): + minion_opts["file_client"] = "local" + minion_opts["id"] = "foo01" + return minion_opts + + +single_extend_test_cases = [ + ( + {}, # extend + "bar", # id_to_extend + "file", # state_mod_name + "require", # req_type + "foo", # req_id + "file", # req_state_mod_name + { + "bar": HashableOrderedDict({"file": [{"require": [{"file": "foo"}]}]}) + }, # expected + ) +] + +simple_extend_test_cases = [ + ( + {}, # extend + "bar", # id_to_extend + "file", # state_mod_name + "require", # req_type + "foo", # req_id + "file", # req_state_mod_name + {"bar": {"file": [{"require": [{"file": "foo"}]}]}}, # expected + ), + ( + { # extend + "bar": { + "file": [{"require": [{"file": "foo"}]}], + "__env__": "base", + "__sls__": "test.foo", + }, + "baz": { + "file": [{"require": [{"file": "foo"}]}], + "__env__": "base", + "__sls__": "test.foo", + }, + }, + "baz", # id_to_extend + "file", # state_mod_name + "require", # req_type + "foo", # req_id + "file", # req_state_mod_name + { # expected + "bar": { + "file": [{"require": [{"file": "foo"}]}], + "__env__": "base", + "__sls__": "test.foo", + }, + "baz": { + "file": [{"require": [{"file": "foo"}]}], + "__env__": "base", + "__sls__": "test.foo", + }, + }, + ), + ( + { # extend + "/tmp/bar": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + "baz": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + "foo": HashableOrderedDict( + [("file", [{"prerequired": [{"pkg": "quux-pkg"}]}])] + ), + "quux-pkg": HashableOrderedDict( + [ + ("file", [{"prereq": [{"pkg": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + }, + "/tmp/baz", # id_to_extend + "file", # state_mod_name + "require", # req_type + "bar", # req_id + "file", # req_state_mod_name + { # expected + "/tmp/bar": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + "baz": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + "foo": HashableOrderedDict( + [("file", [{"prerequired": [{"pkg": "quux-pkg"}]}])] + ), + "quux-pkg": HashableOrderedDict( + [ + ("file", [{"prereq": [{"pkg": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + "/tmp/baz": HashableOrderedDict( + [("file", [{"require": [{"file": "bar"}]}])] + ), + }, + ), + ( + { + "bar": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + "baz": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + "foo": HashableOrderedDict( + [("file", [{"prerequired": [{"pkg": "quux"}]}])] + ), + "quux": HashableOrderedDict( + [ + ("file", [{"prereq": [{"pkg": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + }, + "baz", + "file", + "require", + "bar", + "file", + { + "bar": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + "baz": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}, {"file": "bar"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + "foo": HashableOrderedDict( + [("file", [{"prerequired": [{"pkg": "quux"}]}])] + ), + "quux": HashableOrderedDict( + [ + ("file", [{"prereq": [{"pkg": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + }, + ), + ( + { + "bar": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + "baz": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + }, + "baz", + "file", + "require", + "bar", + "file", + { + "bar": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + "baz": HashableOrderedDict( + [ + ("file", [{"require": [{"file": "foo"}, {"file": "bar"}]}]), + ("__env__", "base"), + ("__sls__", "test.foo"), + ] + ), + }, + ), +] + + +@pytest.mark.parametrize( + "extend,id_to_extend,state_mod_name,req_type,req_id,req_state_mod_name,expected", + simple_extend_test_cases, +) +def test_simple_extend( + extend, id_to_extend, state_mod_name, req_type, req_id, req_state_mod_name, expected +): + # local copy of extend, as it is modified by _add_to_extend + _extend = deepcopy(extend) + + salt.state.State._add_to_extend( + _extend, id_to_extend, state_mod_name, req_type, req_id, req_state_mod_name + ) + assert _extend == expected From db7d658eb684728f1b8d641a0ec6986f502e04dc Mon Sep 17 00:00:00 2001 From: guillaume pernot Date: Sun, 19 Oct 2025 21:57:52 +0200 Subject: [PATCH 2/7] fix: remove no-op --- salt/state.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/salt/state.py b/salt/state.py index 09a6917ff581..20480200664c 100644 --- a/salt/state.py +++ b/salt/state.py @@ -2002,15 +2002,12 @@ def requisite_in(self, high): ) ) _state = _state.split(".")[0] - found = self._add_to_extend( + self._add_to_extend( extend, name, _state, rkey, id_, state ) extend[name]["__env__"] = body["__env__"] extend[name]["__sls__"] = body["__sls__"] - if found: - continue - - if isinstance(items, list): + elif isinstance(items, list): # Formed as a list of requisite additions hinges = [] for ind in items: @@ -2146,15 +2143,13 @@ def requisite_in(self, high): continue extend[id_][state].append(arg) continue - found = self._add_to_extend( + self._add_to_extend( extend, name, _state, rkey, id_, state ) extend[name]["__env__"] = body["__env__"] extend[name]["__sls__"] = body["__sls__"] - if found: - continue high["__extend__"] = [] for key, val in extend.items(): high["__extend__"].append({key: val}) From aa56dc488e27fdeb2923c267196f893846ef007b Mon Sep 17 00:00:00 2001 From: guillaume pernot Date: Mon, 20 Oct 2025 06:08:41 +0200 Subject: [PATCH 3/7] clarifications --- salt/state.py | 19 +- .../pytests/unit/state/test_add_to_extend.py | 229 ++++++------------ 2 files changed, 87 insertions(+), 161 deletions(-) diff --git a/salt/state.py b/salt/state.py index 20480200664c..1ec896f041e4 100644 --- a/salt/state.py +++ b/salt/state.py @@ -1913,30 +1913,33 @@ def _add_to_extend( req_state_mod_name, ): """ - Add requisiste `req_id` as a requisite for `id_to_extend` + Add req_id as a requisite for id_to_extend """ if id_to_extend not in extend: # The state does not exist yet: create it extend[id_to_extend] = HashableOrderedDict() - if (req_states := extend[id_to_extend].get(req_state_mod_name)) is None: - # The requisited state is not present yet, create it and initialize it + if (_req_module := extend[id_to_extend].get(req_state_mod_name)) is None: + # The requisited state module is not present yet, create it and initialize it extend[id_to_extend][req_state_mod_name] = [ {req_type: [{state_mod_name: req_id}]} ] return - # Lookup req_type if req_states - for state_arg in req_states: - if (req_items := state_arg.get(req_type)) is not None: + # Lookup req_type in _req_module + for _mod_req in _req_module: + if (req_items := _mod_req.get(req_type)) is not None: for req_item in req_items: - # (state_mode_name, req_id) is already defined as a requisiste if req_item.get(state_mod_name) == req_id: + # (state_mode_name, req_id) is already defined as a requisiste break else: # Extending again - state_arg[req_type].append({state_mod_name: req_id}) + _mod_req[req_type].append({state_mod_name: req_id}) + else: + # req_type does not exist already for this module + _mod_req[req_type] = [{state_mod_name: req_id}] def requisite_in(self, high): """ diff --git a/tests/pytests/unit/state/test_add_to_extend.py b/tests/pytests/unit/state/test_add_to_extend.py index 0d4cb8053cbb..aa7e31b62dc6 100644 --- a/tests/pytests/unit/state/test_add_to_extend.py +++ b/tests/pytests/unit/state/test_add_to_extend.py @@ -1,43 +1,24 @@ -import logging from copy import deepcopy import pytest -import salt.config import salt.state from salt.utils.odict import HashableOrderedDict -log = logging.getLogger(__name__) - - pytestmark = [ pytest.mark.core_test, ] -@pytest.fixture -def minion_config(minion_opts): - minion_opts["file_client"] = "local" - minion_opts["id"] = "foo01" - return minion_opts - - -single_extend_test_cases = [ - ( - {}, # extend - "bar", # id_to_extend - "file", # state_mod_name - "require", # req_type - "foo", # req_id - "file", # req_state_mod_name - { - "bar": HashableOrderedDict({"file": [{"require": [{"file": "foo"}]}]}) - }, # expected - ) -] - simple_extend_test_cases = [ ( + # Simple addition + # + # add_to_extend: + # foo: + # file.managed: + # - required_in: + # - file: bar {}, # extend "bar", # id_to_extend "file", # state_mod_name @@ -47,6 +28,22 @@ def minion_config(minion_opts): {"bar": {"file": [{"require": [{"file": "foo"}]}]}}, # expected ), ( + # Requisite already exists + # + # bar: + # file.managed: + # require: + # - file: foo + # baz: + # file.managed: + # - require: + # - file: foo + # + # add_to_extend: + # foo: + # file.managed: + # - required_in: + # - file: baz { # extend "bar": { "file": [{"require": [{"file": "foo"}]}], @@ -78,163 +75,89 @@ def minion_config(minion_opts): }, ), ( + # Append requisite + # + # bar: + # file.managed: + # require: + # - file: foo + # baz: + # file.managed: + # - require: + # - file: foo + # + # add_to_extend: + # bar: + # file.managed: + # - require_in: + # - file: baz { # extend - "/tmp/bar": HashableOrderedDict( + "bar": HashableOrderedDict( [ ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), ] ), "baz": HashableOrderedDict( [ ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "foo": HashableOrderedDict( - [("file", [{"prerequired": [{"pkg": "quux-pkg"}]}])] - ), - "quux-pkg": HashableOrderedDict( - [ - ("file", [{"prereq": [{"pkg": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), ] ), }, - "/tmp/baz", # id_to_extend + "baz", # id_to_extend "file", # state_mod_name "require", # req_type "bar", # req_id "file", # req_state_mod_name { # expected - "/tmp/bar": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "baz": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "foo": HashableOrderedDict( - [("file", [{"prerequired": [{"pkg": "quux-pkg"}]}])] - ), - "quux-pkg": HashableOrderedDict( - [ - ("file", [{"prereq": [{"pkg": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "/tmp/baz": HashableOrderedDict( - [("file", [{"require": [{"file": "bar"}]}])] - ), - }, - ), - ( - { "bar": HashableOrderedDict( [ ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "baz": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "foo": HashableOrderedDict( - [("file", [{"prerequired": [{"pkg": "quux"}]}])] - ), - "quux": HashableOrderedDict( - [ - ("file", [{"prereq": [{"pkg": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - }, - "baz", - "file", - "require", - "bar", - "file", - { - "bar": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), ] ), "baz": HashableOrderedDict( [ ("file", [{"require": [{"file": "foo"}, {"file": "bar"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "foo": HashableOrderedDict( - [("file", [{"prerequired": [{"pkg": "quux"}]}])] - ), - "quux": HashableOrderedDict( - [ - ("file", [{"prereq": [{"pkg": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), ] ), }, ), ( - { - "bar": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "baz": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), + # Append with different requisite type + # + # bar: + # file.managed: + # require: + # - file: foo + # baz: + # file.managed: + # - require: + # - file: foo + # + # add_to_extend: + # bar: + # file.managed: + # - prereq_in: + # - file: baz + { # extend + "bar": { + "file": [{"require": [{"file": "foo"}]}], + }, + "baz": { + "file": [{"require": [{"file": "foo"}]}], + }, }, - "baz", - "file", - "require", - "bar", - "file", - { - "bar": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "baz": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}, {"file": "bar"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), + "baz", # id_to_extend + "file", # state_mod_name + "prereq", # req_type + "bar", # req_id + "file", # req_state_mod_name + { # expected + "bar": { + "file": [{"require": [{"file": "foo"}]}], + }, + "baz": { + "file": [{"require": [{"file": "foo"}], "prereq": [{"file": "bar"}]}], + }, }, ), ] From 3ef0957d17ac3ec60fdda92316b756ab6e2077c1 Mon Sep 17 00:00:00 2001 From: guillaume pernot Date: Mon, 20 Oct 2025 06:08:41 +0200 Subject: [PATCH 4/7] bug fix and improved tests --- salt/state.py | 27 +- .../pytests/unit/state/test_add_to_extend.py | 290 ++++++++---------- 2 files changed, 143 insertions(+), 174 deletions(-) diff --git a/salt/state.py b/salt/state.py index 20480200664c..05563590935c 100644 --- a/salt/state.py +++ b/salt/state.py @@ -1913,30 +1913,33 @@ def _add_to_extend( req_state_mod_name, ): """ - Add requisiste `req_id` as a requisite for `id_to_extend` + Add req_id as a requisite for id_to_extend """ if id_to_extend not in extend: # The state does not exist yet: create it extend[id_to_extend] = HashableOrderedDict() - if (req_states := extend[id_to_extend].get(req_state_mod_name)) is None: - # The requisited state is not present yet, create it and initialize it - extend[id_to_extend][req_state_mod_name] = [ - {req_type: [{state_mod_name: req_id}]} + if (_req_module := extend[id_to_extend].get(state_mod_name)) is None: + # The state module is not present yet, create it and initialize it + extend[id_to_extend][state_mod_name] = [ + {req_type: [{req_state_mod_name: req_id}]} ] return - # Lookup req_type if req_states - for state_arg in req_states: - if (req_items := state_arg.get(req_type)) is not None: + # Lookup req_type in _req_module + for _mod_req in _req_module: + if (req_items := _mod_req.get(req_type)) is not None: for req_item in req_items: - # (state_mode_name, req_id) is already defined as a requisiste - if req_item.get(state_mod_name) == req_id: - break + if req_item.get(req_state_mod_name) == req_id: + # (req_state_mode_name, req_id) is already defined as a requisiste + return else: # Extending again - state_arg[req_type].append({state_mod_name: req_id}) + _mod_req[req_type].append({req_state_mod_name: req_id}) + return + # req_type does not exist already for this state module + _req_module.append({req_type: [{req_state_mod_name: req_id}]}) def requisite_in(self, high): """ diff --git a/tests/pytests/unit/state/test_add_to_extend.py b/tests/pytests/unit/state/test_add_to_extend.py index 0d4cb8053cbb..8db03ec15240 100644 --- a/tests/pytests/unit/state/test_add_to_extend.py +++ b/tests/pytests/unit/state/test_add_to_extend.py @@ -1,43 +1,24 @@ -import logging from copy import deepcopy import pytest -import salt.config import salt.state from salt.utils.odict import HashableOrderedDict -log = logging.getLogger(__name__) - - pytestmark = [ pytest.mark.core_test, ] -@pytest.fixture -def minion_config(minion_opts): - minion_opts["file_client"] = "local" - minion_opts["id"] = "foo01" - return minion_opts - - -single_extend_test_cases = [ - ( - {}, # extend - "bar", # id_to_extend - "file", # state_mod_name - "require", # req_type - "foo", # req_id - "file", # req_state_mod_name - { - "bar": HashableOrderedDict({"file": [{"require": [{"file": "foo"}]}]}) - }, # expected - ) -] - simple_extend_test_cases = [ ( + # Simple addition + # + # add_to_extend: + # foo: + # file.managed: + # - required_in: + # - file: bar {}, # extend "bar", # id_to_extend "file", # state_mod_name @@ -47,16 +28,28 @@ def minion_config(minion_opts): {"bar": {"file": [{"require": [{"file": "foo"}]}]}}, # expected ), ( + # Requisite already exists + # + # bar: + # file.managed: + # require: + # - file: foo + # baz: + # file.managed: + # - require: + # - file: foo + # + # add_to_extend: + # foo: + # file.managed: + # - required_in: + # - file: baz { # extend "bar": { "file": [{"require": [{"file": "foo"}]}], - "__env__": "base", - "__sls__": "test.foo", }, "baz": { "file": [{"require": [{"file": "foo"}]}], - "__env__": "base", - "__sls__": "test.foo", }, }, "baz", # id_to_extend @@ -67,179 +60,152 @@ def minion_config(minion_opts): { # expected "bar": { "file": [{"require": [{"file": "foo"}]}], - "__env__": "base", - "__sls__": "test.foo", }, "baz": { "file": [{"require": [{"file": "foo"}]}], - "__env__": "base", - "__sls__": "test.foo", }, }, ), ( + # Append requisite + # + # bar: + # file.managed: + # require: + # - file: foo + # baz: + # file.managed: + # - require: + # - file: foo + # + # add_to_extend: + # bar: + # file.managed: + # - require_in: + # - file: baz { # extend - "/tmp/bar": HashableOrderedDict( + "bar": HashableOrderedDict( [ ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), ] ), "baz": HashableOrderedDict( [ ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "foo": HashableOrderedDict( - [("file", [{"prerequired": [{"pkg": "quux-pkg"}]}])] - ), - "quux-pkg": HashableOrderedDict( - [ - ("file", [{"prereq": [{"pkg": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), ] ), }, - "/tmp/baz", # id_to_extend + "baz", # id_to_extend "file", # state_mod_name "require", # req_type "bar", # req_id "file", # req_state_mod_name { # expected - "/tmp/bar": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "baz": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "foo": HashableOrderedDict( - [("file", [{"prerequired": [{"pkg": "quux-pkg"}]}])] - ), - "quux-pkg": HashableOrderedDict( - [ - ("file", [{"prereq": [{"pkg": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "/tmp/baz": HashableOrderedDict( - [("file", [{"require": [{"file": "bar"}]}])] - ), - }, - ), - ( - { - "bar": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "baz": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "foo": HashableOrderedDict( - [("file", [{"prerequired": [{"pkg": "quux"}]}])] - ), - "quux": HashableOrderedDict( - [ - ("file", [{"prereq": [{"pkg": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - }, - "baz", - "file", - "require", - "bar", - "file", - { "bar": HashableOrderedDict( [ ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), ] ), "baz": HashableOrderedDict( [ ("file", [{"require": [{"file": "foo"}, {"file": "bar"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "foo": HashableOrderedDict( - [("file", [{"prerequired": [{"pkg": "quux"}]}])] - ), - "quux": HashableOrderedDict( - [ - ("file", [{"prereq": [{"pkg": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), ] ), }, ), ( - { - "bar": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "baz": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), + # Append with different requisite type + # + # bar: + # file.managed: + # require: + # - file: foo + # baz: + # file.managed: + # - require: + # - file: foo + # + # add_to_extend: + # bar: + # file.managed: + # - prereq_in: + # - file: baz + { # extend + "bar": { + "file": [{"require": [{"file": "foo"}]}], + }, + "baz": { + "file": [{"require": [{"file": "foo"}]}], + }, }, - "baz", - "file", - "require", - "bar", - "file", - { - "bar": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), - "baz": HashableOrderedDict( - [ - ("file", [{"require": [{"file": "foo"}, {"file": "bar"}]}]), - ("__env__", "base"), - ("__sls__", "test.foo"), - ] - ), + "baz", # id_to_extend + "file", # state_mod_name + "prereq", # req_type + "bar", # req_id + "file", # req_state_mod_name + { # expected + "bar": { + "file": [ + { + "require": [ + {"file": "foo"}, + ] + }, + ], + }, + "baz": { + "file": [ + { + "require": [ + {"file": "foo"}, + ] + }, + { + "prereq": [ + {"file": "bar"}, + ] + }, + ], + }, }, ), ] +def _flatten_extend(a): + return { + (k1, k2, k4, k6, v6) + for k1, v1 in a.items() + for k2, v2 in v1.items() + for v3 in v2 + for k4, v4 in v3.items() + for v5 in v4 + for k6, v6 in v5.items() + } + + +def test_flatten(): + a = { + "bar": {"file": [{"require": [{"file": "foo"}]}]}, + "baz": { + "file": [ + {"require": [{"file": "foo"}]}, + {"prereq": [{"file": "bar"}]}, + ] + }, + } + b = { + "baz": { + "file": [ + {"prereq": [{"file": "bar"}]}, + {"require": [{"file": "foo"}]}, + ] + }, + "bar": {"file": [{"require": [{"file": "foo"}]}]}, + } + assert _flatten_extend(a) == _flatten_extend(b) + + @pytest.mark.parametrize( "extend,id_to_extend,state_mod_name,req_type,req_id,req_state_mod_name,expected", simple_extend_test_cases, @@ -253,4 +219,4 @@ def test_simple_extend( salt.state.State._add_to_extend( _extend, id_to_extend, state_mod_name, req_type, req_id, req_state_mod_name ) - assert _extend == expected + assert _flatten_extend(_extend) == _flatten_extend(expected) From b44d1f6ddc40e5bd2d1fc485704de525ab5b8e28 Mon Sep 17 00:00:00 2001 From: guillaume pernot Date: Tue, 21 Oct 2025 15:37:11 +0200 Subject: [PATCH 5/7] rename _req_module to _state_args --- salt/state.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/salt/state.py b/salt/state.py index 05563590935c..a03eb9532465 100644 --- a/salt/state.py +++ b/salt/state.py @@ -1920,15 +1920,15 @@ def _add_to_extend( # The state does not exist yet: create it extend[id_to_extend] = HashableOrderedDict() - if (_req_module := extend[id_to_extend].get(state_mod_name)) is None: + if (_state_args := extend[id_to_extend].get(state_mod_name)) is None: # The state module is not present yet, create it and initialize it extend[id_to_extend][state_mod_name] = [ {req_type: [{req_state_mod_name: req_id}]} ] return - # Lookup req_type in _req_module - for _mod_req in _req_module: + # Lookup req_type in _state_args + for _mod_req in _state_args: if (req_items := _mod_req.get(req_type)) is not None: for req_item in req_items: if req_item.get(req_state_mod_name) == req_id: @@ -1939,7 +1939,7 @@ def _add_to_extend( _mod_req[req_type].append({req_state_mod_name: req_id}) return # req_type does not exist already for this state module - _req_module.append({req_type: [{req_state_mod_name: req_id}]}) + _state_args.append({req_type: [{req_state_mod_name: req_id}]}) def requisite_in(self, high): """ From be6dba00ed7a59892d64e08e35c42fa4b6b8a8db Mon Sep 17 00:00:00 2001 From: guillaume pernot Date: Thu, 23 Oct 2025 08:31:04 +0200 Subject: [PATCH 6/7] rename _mod_req to state_arg --- salt/state.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/salt/state.py b/salt/state.py index a03eb9532465..d298448ff522 100644 --- a/salt/state.py +++ b/salt/state.py @@ -1920,26 +1920,25 @@ def _add_to_extend( # The state does not exist yet: create it extend[id_to_extend] = HashableOrderedDict() - if (_state_args := extend[id_to_extend].get(state_mod_name)) is None: + if (state_args := extend[id_to_extend].get(state_mod_name)) is None: # The state module is not present yet, create it and initialize it extend[id_to_extend][state_mod_name] = [ {req_type: [{req_state_mod_name: req_id}]} ] return - # Lookup req_type in _state_args - for _mod_req in _state_args: - if (req_items := _mod_req.get(req_type)) is not None: + # Lookup req_type in state_args + for state_arg in state_args: + if (req_items := state_arg.get(req_type)) is not None: for req_item in req_items: if req_item.get(req_state_mod_name) == req_id: # (req_state_mode_name, req_id) is already defined as a requisiste return - else: - # Extending again - _mod_req[req_type].append({req_state_mod_name: req_id}) - return + # Extending again + state_arg[req_type].append({req_state_mod_name: req_id}) + return # req_type does not exist already for this state module - _state_args.append({req_type: [{req_state_mod_name: req_id}]}) + state_args.append({req_type: [{req_state_mod_name: req_id}]}) def requisite_in(self, high): """ From 574e90da83012b6aeebffb192139da94162813ec Mon Sep 17 00:00:00 2001 From: guillaume pernot Date: Tue, 28 Oct 2025 10:16:15 +0100 Subject: [PATCH 7/7] add changelog for #68408 --- changelog/68408.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/68408.fixed.md diff --git a/changelog/68408.fixed.md b/changelog/68408.fixed.md new file mode 100644 index 000000000000..23a852bdd1c4 --- /dev/null +++ b/changelog/68408.fixed.md @@ -0,0 +1 @@ +Fixed issue with check_requisite checking several times the same state