From 0fdc7a626918bda09e01d4e97bfe6c400312ba8f Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Mon, 29 Dec 2014 22:18:15 +0000 Subject: [PATCH 1/5] Default value of 'sudo' set to False. --- st2actions/st2actions/bootstrap/runnersregistrar.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/st2actions/st2actions/bootstrap/runnersregistrar.py b/st2actions/st2actions/bootstrap/runnersregistrar.py index 6a244c9479..291ec0fe5c 100644 --- a/st2actions/st2actions/bootstrap/runnersregistrar.py +++ b/st2actions/st2actions/bootstrap/runnersregistrar.py @@ -116,7 +116,8 @@ def register_runner_types(): }, 'sudo': { 'description': 'The remote command will be executed with sudo.', - 'type': 'boolean' + 'type': 'boolean', + 'default': False }, 'dir': { 'description': 'The working directory where the command will be ' @@ -158,7 +159,8 @@ def register_runner_types(): }, 'sudo': { 'description': 'The remote command will be executed with sudo.', - 'type': 'boolean' + 'type': 'boolean', + 'default': False }, 'dir': { 'description': 'The working directory where the command will be ' From c64eaa7a868443d04be778f5298ca22fabe75a38 Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Mon, 29 Dec 2014 22:18:32 +0000 Subject: [PATCH 2/5] Runner param can be defined immutable in action * A runner param can be declared immutable in action. In this case, we can use the 'default' value supplied in either runner or action. We need default to be specified in at least one place. --- st2actions/st2actions/utils/param_utils.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/st2actions/st2actions/utils/param_utils.py b/st2actions/st2actions/utils/param_utils.py index bd604cec12..f7f14eae68 100644 --- a/st2actions/st2actions/utils/param_utils.py +++ b/st2actions/st2actions/utils/param_utils.py @@ -104,14 +104,18 @@ def _get_resolved_runner_params(runner_parameters, action_parameters, # No override if param is immutable if param_value.get('immutable', False): continue + # Check if param exists in action_parameters and if it has a default value then # pickup the override. - if param_name in action_parameters and 'default' in action_parameters[param_name]: + if param_name in action_parameters: action_param = action_parameters[param_name] - resolved_params[param_name] = action_param['default'] - # No further override if param is immutable + if action_param.get('default', False): + resolved_params[param_name] = action_param['default'] + + # No further override (from actionexecution) if param is immutable if action_param.get('immutable', False): continue + # Finally pick up override from actionexec_runner_parameters if param_name in actionexec_runner_parameters: resolved_params[param_name] = actionexec_runner_parameters[param_name] From 0a3ba6196a4bc14001d73864a5882af6a518ed07 Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Mon, 29 Dec 2014 22:20:15 +0000 Subject: [PATCH 3/5] Fix action validation regarding immutability and default values * Default value for a runner param made immutable in either runner or action can also come from either. --- st2common/st2common/validators/api/action.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/st2common/st2common/validators/api/action.py b/st2common/st2common/validators/api/action.py index 52e4024629..96dc95cbc2 100644 --- a/st2common/st2common/validators/api/action.py +++ b/st2common/st2common/validators/api/action.py @@ -66,6 +66,10 @@ def _validate_parameters(action_params=None, runner_params=None): msg = 'Param %s is declared immutable in runner. ' % param + \ 'Cannot override in action.' raise ValueValidationException(msg) - if 'default' not in action_param_meta: - msg = 'Immutable param %s requires a default value.' % param - raise ValueValidationException(msg) + if 'default' not in action_param_meta and 'default' not in runner_param_meta: + msg = 'Immutable param %s requires a default value.' % param + raise ValueValidationException(msg) + else: + if 'default' not in action_param_meta: + msg = 'Immutable param %s requires a default value.' % param + raise ValueValidationException(msg) From 73d6accb8388c5f472544d127465a24c6819c200 Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Mon, 29 Dec 2014 22:21:33 +0000 Subject: [PATCH 4/5] Fix packs in st2 repo. --- contrib/core/actions/sendmail.json | 9 --------- contrib/core/actions/stormbot_say.json | 6 ------ contrib/examples/actions/ubuntu_pkg_info.json | 9 --------- 3 files changed, 24 deletions(-) diff --git a/contrib/core/actions/sendmail.json b/contrib/core/actions/sendmail.json index 9ddd6fa9ab..d590d5333c 100644 --- a/contrib/core/actions/sendmail.json +++ b/contrib/core/actions/sendmail.json @@ -23,17 +23,8 @@ "required": true, "position": 2 }, - "user": { - "immutable": true - }, - "dir": { - "immutable": true - }, "sudo": { "immutable": true - }, - "parallel": { - "immutable": true } } } diff --git a/contrib/core/actions/stormbot_say.json b/contrib/core/actions/stormbot_say.json index 1e865925de..584a53bafc 100644 --- a/contrib/core/actions/stormbot_say.json +++ b/contrib/core/actions/stormbot_say.json @@ -7,12 +7,6 @@ "sudo": { "immutable": true }, - "user": { - "immutable": true - }, - "dir": { - "immutable": true - }, "source": { "type": "string", "position": 0 diff --git a/contrib/examples/actions/ubuntu_pkg_info.json b/contrib/examples/actions/ubuntu_pkg_info.json index fedd4db18d..097b01043c 100644 --- a/contrib/examples/actions/ubuntu_pkg_info.json +++ b/contrib/examples/actions/ubuntu_pkg_info.json @@ -11,17 +11,8 @@ "required": true, "position": 0 }, - "user": { - "immutable": true - }, - "dir": { - "immutable": true - }, "sudo": { "immutable": true - }, - "parallel": { - "immutable": true } } } From c183bf0fa7f63cbe9fd707ca9487477a727ca736 Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Mon, 29 Dec 2014 22:52:21 +0000 Subject: [PATCH 5/5] Add test cases --- st2actions/tests/unit/test_param_utils.py | 7 +++++++ st2common/tests/unit/test_action_api_validator.py | 14 ++++++++++++++ st2tests/st2tests/fixtures/history/actions.json | 11 +++++++++++ st2tests/st2tests/fixtures/history/runners.json | 4 ++++ 4 files changed, 36 insertions(+) diff --git a/st2actions/tests/unit/test_param_utils.py b/st2actions/tests/unit/test_param_utils.py index c6fd366d00..88c9512fe0 100644 --- a/st2actions/tests/unit/test_param_utils.py +++ b/st2actions/tests/unit/test_param_utils.py @@ -105,6 +105,8 @@ def test_get_resolved_params(self): self.assertEqual(runner_params.get('runnerint'), 555) # Assert that a runner param can be overriden by action param default. self.assertEqual(runner_params.get('runnerdummy'), 'actiondummy') + # Asser that runner param made immutable in action can use default value in runner. + self.assertEqual(runner_params.get('runnerfoo'), 'FOO') # Assert that an immutable param cannot be overriden by action param or execution param. self.assertEqual(runner_params.get('runnerimmutable'), 'runnerimmutable') @@ -328,6 +330,10 @@ def _setup_runner_models(cls): 'description': 'Foo int param.', 'type': 'number' }, + 'runnerfoo': { + 'description': 'Some foo param.', + 'default': 'FOO' + }, 'runnerdummy': { 'description': 'Dummy param.', 'type': 'string', @@ -358,6 +364,7 @@ def _setup_action_models(cls): 'actionstr': {'type': 'string', 'required': True}, 'actionint': {'type': 'number', 'default': 10}, 'runnerdummy': {'type': 'string', 'default': 'actiondummy', 'immutable': True}, + 'runnerfoo': {'type': 'string', 'immutable': True}, 'runnerimmutable': {'type': 'string', 'default': 'failed_override'}, 'actionimmutable': {'type': 'string', 'default': 'actionimmutable', 'immutable': True} } diff --git a/st2common/tests/unit/test_action_api_validator.py b/st2common/tests/unit/test_action_api_validator.py index 518e2a937b..e7a6c0acec 100644 --- a/st2common/tests/unit/test_action_api_validator.py +++ b/st2common/tests/unit/test_action_api_validator.py @@ -82,3 +82,17 @@ def test_validate_action_param_immutable(self): self.fail('Action validation should not have passed. %s' % json.dumps(action_api_dict)) except ValueValidationException as e: self.assertTrue('requires a default value.' in e.message) + + @mock.patch.object(action_validator, '_is_valid_pack', mock.MagicMock( + return_value=True)) + def test_validate_action_param_immutable_no_default(self): + action_api_dict = fixture.ARTIFACTS['actions']['action-immutable-runner-param-no-default'] + action_api = ActionAPI(**action_api_dict) + + # Runner param sudo is decalred immutable in action but no defualt value + # supplied in action. We should pick up default value from runner. + try: + action_validator.validate_action(action_api) + except ValueValidationException as e: + print(e) + self.fail('Action validation should have passed. %s' % json.dumps(action_api_dict)) diff --git a/st2tests/st2tests/fixtures/history/actions.json b/st2tests/st2tests/fixtures/history/actions.json index 49f0e229f3..909ff354c0 100644 --- a/st2tests/st2tests/fixtures/history/actions.json +++ b/st2tests/st2tests/fixtures/history/actions.json @@ -43,5 +43,16 @@ "immutable": true } } + }, + "action-immutable-runner-param-no-default": { + "runner_type": "run-local", + "name": "action-immutable-param-no-default", + "enabled": true, + "pack": "core", + "parameters": { + "sudo": { + "immutable": true + } + } } } diff --git a/st2tests/st2tests/fixtures/history/runners.json b/st2tests/st2tests/fixtures/history/runners.json index 186045b72e..e29e71692b 100644 --- a/st2tests/st2tests/fixtures/history/runners.json +++ b/st2tests/st2tests/fixtures/history/runners.json @@ -11,6 +11,10 @@ }, "cmd": { "type": "string" + }, + "sudo": { + "type": "boolean", + "default": false } }, "runner_module": "st2actions.runners.fabricrunner"