Merge pull request #6030 from rebrowning/fix/4676_inherit_env

Fix #4676

Author: cognifloyd

CHANGELOG.rst

@@ -25,6 +25,8 @@ Fixed
 
 * Fix codecov failures for stackstorm/st2 tests. #6035, #6046, #6048
 
+* Fix #4676, edge case where --inherit-env is skipped if the action has no parameters
+
 * Update cryptography 3.4.7 -> 39.0.1, pyOpenSSL 21.0.0 -> 23.1.0, paramiko 2.10.5 -> 2.11.0 (security). #6055
 
 * Bumped `eventlet` to `0.33.3` and `gunicorn` to `21.2.0` to fix `RecursionError` bug in setting `SSLContext` `minimum_version` property. #6061

st2client/st2client/commands/action.py

@@ -945,6 +945,9 @@ def normalize(name, value, action_params=None, auto_dict=False):
 
         result = {}
 
+        if args.inherit_env:
+            result["env"] = self._get_inherited_env_vars()
+
         if not args.parameters:
             return result
 
@@ -1008,9 +1011,6 @@ def normalize(name, value, action_params=None, auto_dict=False):
 
             del result["_file_name"]
 
-        if args.inherit_env:
-            result["env"] = self._get_inherited_env_vars()
-
         return result
 
     @add_auth_token_to_kwargs_from_cli

st2client/tests/unit/test_command_actionrun.py

@@ -261,3 +261,135 @@ def test_get_params_from_args_with_multiple_declarations(self):
 
         # set auto_dict back to default
         mockarg.auto_dict = False
+
+    def test_correctly_process_inherit_env_when_no_parameters_set(self):
+        """test_correctly_process_inherit_env_when_no_parameters_set
+
+        This tests that we still correctly pass through the environment variables
+        when --inherit-env is set and we run an action that does not have parameters
+        """
+
+        runner = RunnerType()
+        runner.runner_parameters = {}
+
+        action = Action()
+        action.ref = "test.action"
+
+        subparser = mock.Mock()
+        command = ActionRunCommand(action, self, subparser, name="test")
+
+        mockarg = mock.Mock()
+        mockarg.inherit_env = True
+        mockarg.auto_dict = True
+        mockarg.parameters = []
+
+        k1 = "key1"
+        v1 = "value1"
+        k2 = "key2"
+        v2 = "value2"
+
+        with mock.patch("os.environ.copy") as mockCopy:
+            mockCopy.return_value = {k1: v1, k2: v2}
+            param = command._get_action_parameters_from_args(
+                action=action, runner=runner, args=mockarg
+            )
+
+        self.assertIn("env", param)
+
+        env_params = param["env"]
+        self.assertIn(k1, env_params)
+        self.assertIn(k2, env_params)
+        self.assertEqual(v1, env_params[k1])
+        self.assertEqual(v2, env_params[k2])
+
+    def test_correctly_process_inherit_env_when_parameters_set(self):
+        """test_correctly_process_inherit_env_when_parameters_set
+
+        This tests that we still correctly pass through the environment variables
+        when --inherit-env is set and we run an action that has action parameters set
+        """
+
+        runner = RunnerType()
+        runner.runner_parameters = {}
+
+        action = Action()
+        action.ref = "test.action"
+        action.parameters = {
+            "param_string": {"type": "string"},
+            "param_array": {"type": "array"},
+            "param_array_of_dicts": {"type": "array"},
+        }
+
+        subparser = mock.Mock()
+        command = ActionRunCommand(action, self, subparser, name="test")
+
+        p_string = "param_string"
+        p_array = "param_array"
+        p_ra_dicts = "param_array_of_dicts"
+        mockarg = mock.Mock()
+        mockarg.inherit_env = True
+        mockarg.auto_dict = True
+        mockarg.parameters = [
+            f"{p_string}=hoge",
+            f"{p_array}=foo,bar",
+            f"{p_ra_dicts}=foo:1,bar:2",
+        ]
+
+        k1 = "key1"
+        v1 = "value1"
+        k2 = "key2"
+        v2 = "value2"
+
+        with mock.patch("os.environ.copy") as mockCopy:
+            mockCopy.return_value = {k1: v1, k2: v2}
+            param = command._get_action_parameters_from_args(
+                action=action, runner=runner, args=mockarg
+            )
+
+        self.assertIn("env", param)
+
+        env_params = param["env"]
+        self.assertIn(k1, env_params)
+        self.assertIn(k2, env_params)
+        self.assertEqual(v1, env_params[k1])
+        self.assertEqual(v2, env_params[k2])
+        self.assertIn(p_string, param)
+        self.assertEqual("hoge", param[p_string])
+        self.assertIn(p_array, param)
+        self.assertIn("foo", param[p_array])
+        self.assertIn("bar", param[p_array])
+        self.assertIn(p_ra_dicts, param)
+        self.assertDictEqual({"foo": "1", "bar": "2"}, param[p_ra_dicts][0])
+
+    def test_correctly_generate_empty_params_no_inherit_empty_parameters(self):
+        """test_correctly_generate_empty_params_no_inherit_empty_parameters
+
+        Verifies that we return an empty dict when we do not provide inherit env and parameters
+        """
+
+        runner = RunnerType()
+        runner.runner_parameters = {}
+
+        action = Action()
+        action.ref = "test.action"
+
+        subparser = mock.Mock()
+        command = ActionRunCommand(action, self, subparser, name="test")
+
+        mockarg = mock.Mock()
+        mockarg.inherit_env = False
+        mockarg.auto_dict = True
+        mockarg.parameters = []
+
+        k1 = "key1"
+        v1 = "value1"
+        k2 = "key2"
+        v2 = "value2"
+
+        with mock.patch("os.environ.copy") as mockCopy:
+            mockCopy.return_value = {k1: v1, k2: v2}
+            param = command._get_action_parameters_from_args(
+                action=action, runner=runner, args=mockarg
+            )
+
+        self.assertDictEqual({}, param)