diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7a08f8743f..a7dcb9b739 100644 --- a/CHANGELOG.rst +++ b/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 diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index fb2f94c0f2..48964746a5 100644 --- a/st2client/st2client/commands/action.py +++ b/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 diff --git a/st2client/tests/unit/test_command_actionrun.py b/st2client/tests/unit/test_command_actionrun.py index 1e312e0786..afd6fb418d 100644 --- a/st2client/tests/unit/test_command_actionrun.py +++ b/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)