From 03eb0332f62c5b97db6f99e339c6ccfc8ec16cbb 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 12:37:52 +0000
Subject: [PATCH] Fix onlyif/unless when multiple conditions
 (bsc#1180818)

Add unit tests cases to ensure proper onlyif/unless behavior
---
 salt/state.py            |  20 ++++--
 tests/unit/test_state.py | 146 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 162 insertions(+), 4 deletions(-)

diff --git a/salt/state.py b/salt/state.py
index 2fa5f64ca5..f5df82fce2 100644
--- a/salt/state.py
+++ b/salt/state.py
@@ -897,15 +897,18 @@ class State(object):
                 ret.update({'comment': 'onlyif condition is false',
                             'skip_watch': True,
                             '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, six.string_types):
                 cmd = self.functions['cmd.retcode'](
                     entry, ignore_retcode=True, python_shell=True, **cmd_opts)
                 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: {0}'.format(entry)
@@ -914,17 +917,20 @@ class State(object):
 
                 result = self._run_check_function(entry)
                 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({'comment': 'onlyif condition is false',
                                 'skip_watch': True,
                                 'result': True})
+                    return ret
                 else:
                     ret.update({'comment': 'onlyif condition is true',
                                 'result': False})
 
             else:
                 ret.update({'comment': 'onlyif execution failed, bad type passed', 'result': False})
+                return ret
         return ret
 
     def _run_check_unless(self, low_data, cmd_opts):
@@ -943,14 +949,17 @@ class State(object):
                 ret.update({'comment': 'unless condition is true',
                             'skip_watch': True,
                             '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, six.string_types):
                 cmd = self.functions['cmd.retcode'](entry, ignore_retcode=True, python_shell=True, **cmd_opts)
                 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 unless: {0}'.format(entry)
@@ -959,16 +968,19 @@ class State(object):
 
                 result = self._run_check_function(entry)
                 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({'comment': 'unless condition is true',
                                 'skip_watch': True,
                                 'result': True})
+                    return ret
                 else:
                     ret.update({'comment': 'unless condition is false',
                                 'result': False})
             else:
                 ret.update({'comment': 'unless condition is false, bad type passed', '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 b58982b37c..6769141614 100644
--- a/tests/unit/test_state.py
+++ b/tests/unit/test_state.py
@@ -221,6 +221,152 @@ class StateCompilerTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
             return_result = state_obj._run_check_unless(low_data, '')
             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": ["/bin/true", "/bin/false"],
+            "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": ["/bin/false", "/bin/true"],
+            "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": ["/bin/false", "/bin/true"],
+            "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": ["/bin/false", "/bin/false"],
+            "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": ["/bin/true", "/bin/true"],
+            "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": ["/bin/true", "/bin/true"],
+            "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": ["/bin/false", "/bin/false"],
+            "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)
+
 
 class HighStateTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
     def setUp(self):
-- 
2.29.2


