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 } } } 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 ' 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] 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/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) 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"