From f9e5029ae9572937bc7ae30c95f5732514e7b05f Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Mon, 25 Sep 2023 12:19:31 -0700 Subject: [PATCH 1/7] keep track when an included file only includes sls files but is a requisite. --- salt/state.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/salt/state.py b/salt/state.py index ca3e295e558..1848f21335d 100644 --- a/salt/state.py +++ b/salt/state.py @@ -106,6 +106,7 @@ STATE_RUNTIME_KEYWORDS = frozenset( "__env__", "__sls__", "__id__", + "__included__", "__orchestration_jid__", "__pub_user", "__pub_arg", @@ -648,6 +649,8 @@ class Compiler: chunk["__sls__"] = body["__sls__"] if "__env__" in body: chunk["__env__"] = body["__env__"] + if "__included__" in body: + chunk["__included__"] = body["__included__"] chunk["__id__"] = name for arg in run: if isinstance(arg, str): @@ -1676,6 +1679,8 @@ class State: chunk["__sls__"] = body["__sls__"] if "__env__" in body: chunk["__env__"] = body["__env__"] + if "__included__" in body: + chunk["__included__"] = body["__included__"] chunk["__id__"] = name for arg in run: if isinstance(arg, str): @@ -2885,7 +2890,9 @@ class State: for chunk in chunks: if req_key == "sls": # Allow requisite tracking of entire sls files - if fnmatch.fnmatch(chunk["__sls__"], req_val): + if fnmatch.fnmatch( + chunk["__sls__"], req_val + ) or req_val in chunk.get("__included__", []): found = True reqs[r_state].append(chunk) continue @@ -3100,7 +3107,9 @@ class State: continue if req_key == "sls": # Allow requisite tracking of entire sls files - if fnmatch.fnmatch(chunk["__sls__"], req_val): + if fnmatch.fnmatch( + chunk["__sls__"], req_val + ) or req_val in chunk.get("__included__", []): if requisite == "prereq": chunk["__prereq__"] = True reqs.append(chunk) @@ -4298,6 +4307,9 @@ class BaseHighState: else: include = state.pop("include") + # Keep track if the state only includes includes + empty_state_dict = len(state) == 0 + self._handle_extend(state, sls, saltenv, errors) self._handle_exclude(state, sls, saltenv, errors) self._handle_state_decls(state, sls, saltenv, errors) @@ -4398,6 +4410,12 @@ class BaseHighState: context=context, ) if nstate: + if empty_state_dict: + for item in nstate: + if "__included__" not in nstate[item]: + nstate[item]["__included__"] = [] + nstate[item]["__included__"].append(sls) + self.merge_included_states(state, nstate, errors) state.update(nstate) if err: From 08e4057148ac2965ab2624e1a3498c6fcad14f32 Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Mon, 2 Oct 2023 12:57:15 -0700 Subject: [PATCH 2/7] Adding tests. --- salt/state.py | 25 ++-- .../integration/states/test_include.py | 108 ++++++++++++++++++ 2 files changed, 121 insertions(+), 12 deletions(-) diff --git a/salt/state.py b/salt/state.py index 1848f21335d..a638f141660 100644 --- a/salt/state.py +++ b/salt/state.py @@ -106,7 +106,7 @@ STATE_RUNTIME_KEYWORDS = frozenset( "__env__", "__sls__", "__id__", - "__included__", + "__sls_included_from__", "__orchestration_jid__", "__pub_user", "__pub_arg", @@ -649,8 +649,8 @@ class Compiler: chunk["__sls__"] = body["__sls__"] if "__env__" in body: chunk["__env__"] = body["__env__"] - if "__included__" in body: - chunk["__included__"] = body["__included__"] + if "__sls_included_from__" in body: + chunk["__sls_included_from__"] = body["__sls_included_from__"] chunk["__id__"] = name for arg in run: if isinstance(arg, str): @@ -1679,8 +1679,8 @@ class State: chunk["__sls__"] = body["__sls__"] if "__env__" in body: chunk["__env__"] = body["__env__"] - if "__included__" in body: - chunk["__included__"] = body["__included__"] + if "__sls_included_from__" in body: + chunk["__sls_included_from__"] = body["__sls_included_from__"] chunk["__id__"] = name for arg in run: if isinstance(arg, str): @@ -2892,7 +2892,7 @@ class State: # Allow requisite tracking of entire sls files if fnmatch.fnmatch( chunk["__sls__"], req_val - ) or req_val in chunk.get("__included__", []): + ) or req_val in chunk.get("__sls_included_from__", []): found = True reqs[r_state].append(chunk) continue @@ -3109,7 +3109,7 @@ class State: # Allow requisite tracking of entire sls files if fnmatch.fnmatch( chunk["__sls__"], req_val - ) or req_val in chunk.get("__included__", []): + ) or req_val in chunk.get("__sls_included_from__", []): if requisite == "prereq": chunk["__prereq__"] = True reqs.append(chunk) @@ -4410,11 +4410,12 @@ class BaseHighState: context=context, ) if nstate: - if empty_state_dict: - for item in nstate: - if "__included__" not in nstate[item]: - nstate[item]["__included__"] = [] - nstate[item]["__included__"].append(sls) + for item in nstate: + if "__sls_included_from__" not in nstate[item]: + nstate[item]["__sls_included_from__"] = [] + nstate[item]["__sls_included_from__"].append( + sls + ) self.merge_included_states(state, nstate, errors) state.update(nstate) diff --git a/tests/pytests/integration/states/test_include.py b/tests/pytests/integration/states/test_include.py index f814328c5e4..6ee3df25078 100644 --- a/tests/pytests/integration/states/test_include.py +++ b/tests/pytests/integration/states/test_include.py @@ -38,3 +38,111 @@ def test_issue_64111(salt_master, salt_minion, salt_call_cli): with tf("common/file1.sls", file1_sls): ret = salt_call_cli.run("state.apply", "common") assert ret.returncode == 0 + + +@pytest.mark.slow_test +def test_issue_65080(salt_master, salt_minion, salt_call_cli): + """ + Test scenario when a state includes another state that only includes a third state + """ + + only_include_sls = """ +include: + - includetest.another-include + """ + + another_include_sls = """ +/tmp/from-test-file.txt: + file.managed: + - contents: Hello from test-file.sls + """ + + init_sls = """ +include: + - includetest.only-include + +/tmp/from-init.txt: + file.managed: + - contents: Hello from init.sls + - require: + - sls: includetest.only-include + """ + + tf = salt_master.state_tree.base.temp_file + + with tf("includetest/init.sls", init_sls): + with tf("includetest/another-include.sls", another_include_sls): + with tf("includetest/only-include.sls", only_include_sls): + ret = salt_call_cli.run("state.apply", "includetest") + assert ret.returncode == 0 + + ret = salt_call_cli.run("state.show_low_sls", "includetest") + assert "__sls_included_from__" in ret.data[0] + assert "includetest.only-include" in ret.data[0]["__included_from__"] + + +@pytest.mark.slow_test +def test_issue_65080_multiple_includes(salt_master, salt_minion, salt_call_cli): + """ + Test scenario when a state includes another state that only includes a third state + """ + + only_include_one_sls = """ +include: + - includetest.include-one + """ + + only_include_two_sls = """ +include: + - includetest.include-two + """ + + include_one_sls = """ +/tmp/from-test-file1.txt: + file.managed: + - contents: Hello from test-file.sls + """ + + include_two_sls = """ +/tmp/from-test-file2.txt: + file.managed: + - contents: Hello from test-file.sls + """ + + init_sls = """ +include: + - includetest.only-include-one + - includetest.only-include-two + +/tmp/from-init.txt: + file.managed: + - contents: Hello from init.sls + - require: + - sls: includetest.only-include-one + - sls: includetest.only-include-two + """ + + tf = salt_master.state_tree.base.temp_file + + with tf("includetest/init.sls", init_sls): + with tf("includetest/include-one.sls", include_one_sls), tf( + "includetest/include-two.sls", include_two_sls + ): + with tf("includetest/only-include-one.sls", only_include_one_sls), tf( + "includetest/only-include-two.sls", only_include_two_sls + ): + ret = salt_call_cli.run("state.apply", "includetest") + assert ret.returncode == 0 + + ret = salt_call_cli.run("state.show_low_sls", "includetest") + assert "__sls_included_from__" in ret.data[0] + assert ( + "includetest.only-include-one" + in ret.data[0]["__sls_included_from__"] + ) + + assert "__sls_included_from__" in ret.data[1] + assert ( + "includetest.only-include-two" + in ret.data[1]["__sls_included_from__"] + ) From 2c6749e71410e17b103d54dbe28f0ea891d93655 Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Mon, 2 Oct 2023 13:02:15 -0700 Subject: [PATCH 3/7] Adding changleog. --- changelog/65080.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/65080.fixed.md diff --git a/changelog/65080.fixed.md b/changelog/65080.fixed.md new file mode 100644 index 00000000000..92226b222fa --- /dev/null +++ b/changelog/65080.fixed.md @@ -0,0 +1 @@ +Keep track when an included file only includes sls files but is a requisite. From 27f6fb4d3c5c53556161c853394a20cfeb77c3e6 Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Mon, 2 Oct 2023 15:12:55 -0700 Subject: [PATCH 4/7] fixing failing tests --- salt/state.py | 3 +++ tests/pytests/unit/state/test_state_highstate.py | 1 + 2 files changed, 4 insertions(+) diff --git a/salt/state.py b/salt/state.py index a638f141660..5e2f772b170 100644 --- a/salt/state.py +++ b/salt/state.py @@ -4411,6 +4411,9 @@ class BaseHighState: ) if nstate: for item in nstate: + # Skip existing state keywords + if item.startswith("__"): + continue if "__sls_included_from__" not in nstate[item]: nstate[item]["__sls_included_from__"] = [] nstate[item]["__sls_included_from__"].append( diff --git a/tests/pytests/unit/state/test_state_highstate.py b/tests/pytests/unit/state/test_state_highstate.py index 5278c4a4b72..f90abc50491 100644 --- a/tests/pytests/unit/state/test_state_highstate.py +++ b/tests/pytests/unit/state/test_state_highstate.py @@ -323,6 +323,7 @@ def test_dont_extend_in_excluded_sls_file(highstate, state_tree_dir): ), ("__sls__", "test2"), ("__env__", "base"), + ("__sls_included_from__", ["test1"]), ] ), ), From bd457156c08ed097fe565d23c3f5870b5551a526 Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Mon, 2 Oct 2023 16:23:23 -0700 Subject: [PATCH 5/7] Update test to use the correct key. --- tests/pytests/integration/states/test_include.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/pytests/integration/states/test_include.py b/tests/pytests/integration/states/test_include.py index 6ee3df25078..d4cc13c5b6f 100644 --- a/tests/pytests/integration/states/test_include.py +++ b/tests/pytests/integration/states/test_include.py @@ -78,7 +78,9 @@ include: ret = salt_call_cli.run("state.show_low_sls", "includetest") assert "__sls_included_from__" in ret.data[0] - assert "includetest.only-include" in ret.data[0]["__included_from__"] + assert ( + "includetest.only-include" in ret.data[0]["__sls_included_from__"] + ) @pytest.mark.slow_test From 00356791ffdb961fefa9d27f1d6f508f2c77de37 Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Mon, 9 Oct 2023 14:55:20 -0700 Subject: [PATCH 6/7] Include tests to ensure required includes that are using Salt environments work as expected. --- .../integration/states/test_include.py | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/pytests/integration/states/test_include.py b/tests/pytests/integration/states/test_include.py index d4cc13c5b6f..62ee7efeaf8 100644 --- a/tests/pytests/integration/states/test_include.py +++ b/tests/pytests/integration/states/test_include.py @@ -148,3 +148,47 @@ include: "includetest.only-include-two" in ret.data[1]["__sls_included_from__"] ) + + +@pytest.mark.slow_test +def test_issue_65080_saltenv(salt_master, salt_minion, salt_call_cli): + """ + Test scenario when a state includes another state that only includes a third state + """ + + only_include_sls = """ +include: + - includetest.another-include + """ + + another_include_sls = """ +/tmp/from-test-file.txt: + file.managed: + - contents: Hello from test-file.sls + """ + + init_sls = """ +include: + - prod: includetest.only-include + +/tmp/from-init.txt: + file.managed: + - contents: Hello from init.sls + - require: + - sls: includetest.only-include + """ + + base_tf = salt_master.state_tree.base.temp_file + prod_tf = salt_master.state_tree.prod.temp_file + + with base_tf("includetest/init.sls", init_sls): + with prod_tf("includetest/only-include.sls", only_include_sls): + with prod_tf("includetest/another-include.sls", another_include_sls): + ret = salt_call_cli.run("state.apply", "includetest") + assert ret.returncode == 0 + + ret = salt_call_cli.run("state.show_low_sls", "includetest") + assert "__sls_included_from__" in ret.data[0] + assert ( + "includetest.only-include" in ret.data[0]["__sls_included_from__"] + ) From 68846785409023220bf5ac42b5dd061f3fd07715 Mon Sep 17 00:00:00 2001 From: David Murphy Date: Mon, 4 Dec 2023 13:47:59 -0700 Subject: [PATCH 7/7] Update salt/state.py Co-authored-by: Pedro Algarvio --- salt/state.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/salt/state.py b/salt/state.py index fdd5cf8fd4c..067f568b504 100644 --- a/salt/state.py +++ b/salt/state.py @@ -4369,9 +4369,6 @@ class BaseHighState: else: include = state.pop("include") - # Keep track if the state only includes includes - empty_state_dict = len(state) == 0 - self._handle_extend(state, sls, saltenv, errors) self._handle_exclude(state, sls, saltenv, errors) self._handle_state_decls(state, sls, saltenv, errors)