File fix-onlyif-unless-when-multiple-conditions-bsc-11808.patch of Package salt
From 828ca76e2083d87ace12b488277e51d4e30c8c9a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?=
<psuarezhernandez@suse.com>
Date: Thu, 21 Jan 2021 11:19:07 +0000
Subject: [PATCH] Fix onlyif/unless when multiple conditions
(bsc#1180818)
Add unit tests to ensure right onlyif/unless behavior
Add extra unit test to cover valid cases
Add unit tests cases to ensure proper onlyif/unless behavior
Change tests to use 'exit' cmd and work outside Linux
---
salt/state.py | 20 ++++--
tests/unit/test_state.py | 148 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 163 insertions(+), 5 deletions(-)
diff --git a/salt/state.py b/salt/state.py
index cc6db7e1b2..070a914636 100644
--- a/salt/state.py
+++ b/salt/state.py
@@ -947,8 +947,10 @@ class State:
"result": True,
}
)
+ return False
elif cmd == 0:
ret.update({"comment": "onlyif condition is true", "result": False})
+ return True
for entry in low_data_onlyif:
if isinstance(entry, str):
@@ -960,7 +962,8 @@ class State:
# Command failed, notify onlyif to skip running the item
cmd = 100
log.debug("Last command return code: %s", cmd)
- _check_cmd(cmd)
+ if not _check_cmd(cmd):
+ return ret
elif isinstance(entry, dict):
if "fun" not in entry:
ret["comment"] = "no `fun` argument in onlyif: {}".format(entry)
@@ -972,7 +975,8 @@ class State:
if get_return:
result = salt.utils.data.traverse_dict_and_list(result, get_return)
if self.state_con.get("retcode", 0):
- _check_cmd(self.state_con["retcode"])
+ if not _check_cmd(self.state_con["retcode"]):
+ return ret
elif not result:
ret.update(
{
@@ -981,6 +985,7 @@ class State:
"result": True,
}
)
+ return ret
else:
ret.update({"comment": "onlyif condition is true", "result": False})
@@ -991,6 +996,7 @@ class State:
"result": False,
}
)
+ return ret
return ret
def _run_check_unless(self, low_data, cmd_opts):
@@ -1013,8 +1019,10 @@ class State:
"result": True,
}
)
+ return False
elif cmd != 0:
ret.update({"comment": "unless condition is false", "result": False})
+ return True
for entry in low_data_unless:
if isinstance(entry, str):
@@ -1026,7 +1034,8 @@ class State:
except CommandExecutionError:
# Command failed, so notify unless to skip the item
cmd = 0
- _check_cmd(cmd)
+ if not _check_cmd(cmd):
+ return ret
elif isinstance(entry, dict):
if "fun" not in entry:
ret["comment"] = "no `fun` argument in unless: {}".format(entry)
@@ -1038,7 +1047,8 @@ class State:
if get_return:
result = salt.utils.data.traverse_dict_and_list(result, get_return)
if self.state_con.get("retcode", 0):
- _check_cmd(self.state_con["retcode"])
+ if not _check_cmd(self.state_con["retcode"]):
+ return ret
elif result:
ret.update(
{
@@ -1047,6 +1057,7 @@ class State:
"result": True,
}
)
+ return ret
else:
ret.update(
{"comment": "unless condition is false", "result": False}
@@ -1058,6 +1069,7 @@ class State:
"result": False,
}
)
+ return ret
# No reason to stop, return ret
return ret
diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py
index b1bcf8fe83..95018a9cf3 100644
--- a/tests/unit/test_state.py
+++ b/tests/unit/test_state.py
@@ -205,6 +205,152 @@ class StateCompilerTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
)
self.assertEqual(expected_result, return_result)
+ def test_verify_unless_list_cmd(self):
+ low_data = {
+ "state": "cmd",
+ "name": 'echo "something"',
+ "__sls__": "tests.cmd",
+ "__env__": "base",
+ "__id__": "check unless",
+ "unless": ["exit 0", "exit 1"],
+ "order": 10001,
+ "fun": "run",
+ }
+ expected_result = {
+ "comment": "unless condition is true",
+ "result": True,
+ "skip_watch": True,
+ }
+ with patch("salt.state.State._gather_pillar") as state_patch:
+ minion_opts = self.get_temp_config("minion")
+ state_obj = salt.state.State(minion_opts)
+ return_result = state_obj._run_check_unless(low_data, {})
+ self.assertEqual(expected_result, return_result)
+
+ def test_verify_unless_list_cmd_different_order(self):
+ low_data = {
+ "state": "cmd",
+ "name": 'echo "something"',
+ "__sls__": "tests.cmd",
+ "__env__": "base",
+ "__id__": "check unless",
+ "unless": ["exit 1", "exit 0"],
+ "order": 10001,
+ "fun": "run",
+ }
+ expected_result = {
+ "comment": "unless condition is true",
+ "result": True,
+ "skip_watch": True,
+ }
+ with patch("salt.state.State._gather_pillar") as state_patch:
+ minion_opts = self.get_temp_config("minion")
+ state_obj = salt.state.State(minion_opts)
+ return_result = state_obj._run_check_unless(low_data, {})
+ self.assertEqual(expected_result, return_result)
+
+ def test_verify_onlyif_list_cmd_different_order(self):
+ low_data = {
+ "state": "cmd",
+ "name": 'echo "something"',
+ "__sls__": "tests.cmd",
+ "__env__": "base",
+ "__id__": "check onlyif",
+ "onlyif": ["exit 1", "exit 0"],
+ "order": 10001,
+ "fun": "run",
+ }
+ expected_result = {
+ "comment": "onlyif condition is false",
+ "result": True,
+ "skip_watch": True,
+ }
+ with patch("salt.state.State._gather_pillar") as state_patch:
+ minion_opts = self.get_temp_config("minion")
+ state_obj = salt.state.State(minion_opts)
+ return_result = state_obj._run_check_onlyif(low_data, {})
+ self.assertEqual(expected_result, return_result)
+
+ def test_verify_unless_list_cmd_valid(self):
+ low_data = {
+ "state": "cmd",
+ "name": 'echo "something"',
+ "__sls__": "tests.cmd",
+ "__env__": "base",
+ "__id__": "check unless",
+ "unless": ["exit 1", "exit 1"],
+ "order": 10001,
+ "fun": "run",
+ }
+ expected_result = {"comment": "unless condition is false", "result": False}
+ with patch("salt.state.State._gather_pillar") as state_patch:
+ minion_opts = self.get_temp_config("minion")
+ state_obj = salt.state.State(minion_opts)
+ return_result = state_obj._run_check_unless(low_data, {})
+ self.assertEqual(expected_result, return_result)
+
+ def test_verify_onlyif_list_cmd_valid(self):
+ low_data = {
+ "state": "cmd",
+ "name": 'echo "something"',
+ "__sls__": "tests.cmd",
+ "__env__": "base",
+ "__id__": "check onlyif",
+ "onlyif": ["exit 0", "exit 0"],
+ "order": 10001,
+ "fun": "run",
+ }
+ expected_result = {"comment": "onlyif condition is true", "result": False}
+ with patch("salt.state.State._gather_pillar") as state_patch:
+ minion_opts = self.get_temp_config("minion")
+ state_obj = salt.state.State(minion_opts)
+ return_result = state_obj._run_check_onlyif(low_data, {})
+ self.assertEqual(expected_result, return_result)
+
+ def test_verify_unless_list_cmd_invalid(self):
+ low_data = {
+ "state": "cmd",
+ "name": 'echo "something"',
+ "__sls__": "tests.cmd",
+ "__env__": "base",
+ "__id__": "check unless",
+ "unless": ["exit 0", "exit 0"],
+ "order": 10001,
+ "fun": "run",
+ }
+ expected_result = {
+ "comment": "unless condition is true",
+ "result": True,
+ "skip_watch": True,
+ }
+ with patch("salt.state.State._gather_pillar") as state_patch:
+ minion_opts = self.get_temp_config("minion")
+ state_obj = salt.state.State(minion_opts)
+ return_result = state_obj._run_check_unless(low_data, {})
+ self.assertEqual(expected_result, return_result)
+
+ def test_verify_onlyif_list_cmd_invalid(self):
+ low_data = {
+ "state": "cmd",
+ "name": 'echo "something"',
+ "__sls__": "tests.cmd",
+ "__env__": "base",
+ "__id__": "check onlyif",
+ "onlyif": ["exit 1", "exit 1"],
+ "order": 10001,
+ "fun": "run",
+ }
+ expected_result = {
+ "comment": "onlyif condition is false",
+ "result": True,
+ "skip_watch": True,
+ }
+ with patch("salt.state.State._gather_pillar") as state_patch:
+ minion_opts = self.get_temp_config("minion")
+ state_obj = salt.state.State(minion_opts)
+ return_result = state_obj._run_check_onlyif(low_data, {})
+ self.assertEqual(expected_result, return_result)
+
def test_verify_unless_parse(self):
low_data = {
"unless": [{"fun": "test.arg", "args": ["arg1", "arg2"]}],
@@ -376,7 +522,7 @@ class StateCompilerTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
"__sls__": "tests.cmd",
"__env__": "base",
"__id__": "check onlyif",
- "onlyif": ["/bin/true", "/bin/false"],
+ "onlyif": ["exit 0", "exit 1"],
"order": 10001,
"fun": "run",
}
--
2.29.2
