From da3f859c095e62a810e8a16effab1ff5115aeafe Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Wed, 24 Dec 2014 00:25:48 +0000 Subject: [PATCH 01/13] Action registration should use same validations as API --- .../st2actions/bootstrap/actionsregistrar.py | 37 ++++++++----------- .../tests/unit/test_actions_registrar.py | 25 ++++++++----- st2common/st2common/validators/api/action.py | 19 ++++++---- 3 files changed, 43 insertions(+), 38 deletions(-) diff --git a/st2actions/st2actions/bootstrap/actionsregistrar.py b/st2actions/st2actions/bootstrap/actionsregistrar.py index eb31ef37f0..99ab562669 100644 --- a/st2actions/st2actions/bootstrap/actionsregistrar.py +++ b/st2actions/st2actions/bootstrap/actionsregistrar.py @@ -24,9 +24,10 @@ from st2common.content.loader import (ContentPackLoader, MetaLoader) from st2common.content.validators import RequirementsValidator from st2common.persistence.action import Action -from st2common.models.db.action import ActionDB +from st2common.models.api.action import ActionAPI from st2common.models.system.common import ResourceReference import st2common.util.action_db as action_utils +import st2common.validators.api.action as action_validator LOG = logging.getLogger(__name__) @@ -48,24 +49,24 @@ def _get_actions_from_pack(self, actions_dir): def _register_action(self, pack, action): content = self._meta_loader.load(action) + pack_field = content.get('pack', None) + if not pack_field: + content['pack'] = pack + pack_field = pack + if pack_field != pack: + raise Exception('Model is in pack: %s but field "pack" is different. %s' % + (pack, content.get(pack))) + action_ref = ResourceReference(pack=pack, name=str(content['name'])) model = action_utils.get_action_by_ref(action_ref) if not model: - model = ActionDB() - model.name = content['name'] - model.description = content['description'] - model.enabled = content['enabled'] - model.pack = pack - model.entry_point = content['entry_point'] - model.parameters = content.get('parameters', {}) - runner_type = str(content['runner_type']) - valid_runner_type, runner_type_db = self._has_valid_runner_type(runner_type) - if valid_runner_type: - model.runner_type = {'name': runner_type_db.name} + LOG.info('Action %s not found. Creating new one with: %s', action_ref, content) else: - LOG.exception('Runner type %s doesn\'t exist.', runner_type) - raise - + LOG.info('Action %s found. Will be updated from: %s to: %s', + action_ref, model, content) + action_api = ActionAPI(**content) + model = ActionAPI.to_model(action_api) + action_validator.validate_action(model) try: model = Action.add_or_update(model) LOG.audit('Action created. Action %s from %s.', model, action) @@ -73,12 +74,6 @@ def _register_action(self, pack, action): LOG.exception('Failed to write action to db %s.', model.name) raise - def _has_valid_runner_type(self, runner_type): - try: - return True, action_utils.get_runnertype_by_name(runner_type) - except: - return False, None - def _register_actions_from_pack(self, pack, actions): for action in actions: try: diff --git a/st2actions/tests/unit/test_actions_registrar.py b/st2actions/tests/unit/test_actions_registrar.py index 8d01e41b66..195ea3cbf5 100644 --- a/st2actions/tests/unit/test_actions_registrar.py +++ b/st2actions/tests/unit/test_actions_registrar.py @@ -17,14 +17,15 @@ import simplejson as json except ImportError: import json -import os import mock import st2actions.bootstrap.actionsregistrar as actions_registrar from st2common.persistence.action import Action +import st2common.validators.api.action as action_validator from st2common.models.db.action import RunnerTypeDB import st2tests.base as tests_base +import st2tests.fixturesloader as fixtures_loader MOCK_RUNNER_TYPE_DB = RunnerTypeDB() MOCK_RUNNER_TYPE_DB.name = 'run-local' @@ -32,18 +33,20 @@ class ActionsRegistrarTest(tests_base.DbTestCase): - @mock.patch.object(actions_registrar.ActionsRegistrar, '_has_valid_runner_type', - mock.MagicMock(return_value=(True, MOCK_RUNNER_TYPE_DB))) + @mock.patch.object(action_validator, '_is_valid_pack', mock.MagicMock(return_value=True)) + @mock.patch.object(action_validator, '_get_runner_model', + mock.MagicMock(return_value=MOCK_RUNNER_TYPE_DB)) def test_register_all_actions(self): try: - packs_base_path = os.path.join(tests_base.get_fixtures_path()) + packs_base_path = fixtures_loader.get_fixtures_base_path() all_actions_in_db = Action.get_all() actions_registrar.register_actions(packs_base_path=packs_base_path) - all_actions_in_db = Action.get_all() - self.assertTrue(len(all_actions_in_db) > 0) except Exception as e: print(str(e)) self.fail('All actions must be registered without exceptions.') + else: + all_actions_in_db = Action.get_all() + self.assertTrue(len(all_actions_in_db) > 0) def test_register_actions_from_bad_pack(self): packs_base_path = tests_base.get_fixtures_path() @@ -53,12 +56,14 @@ def test_register_actions_from_bad_pack(self): except: pass - @mock.patch.object(actions_registrar.ActionsRegistrar, '_has_valid_runner_type', - mock.MagicMock(return_value=(True, MOCK_RUNNER_TYPE_DB))) + @mock.patch.object(action_validator, '_is_valid_pack', mock.MagicMock(return_value=True)) + @mock.patch.object(action_validator, '_get_runner_model', + mock.MagicMock(return_value=MOCK_RUNNER_TYPE_DB)) def test_pack_name_missing(self): registrar = actions_registrar.ActionsRegistrar() - action_file = os.path.join(tests_base.get_fixtures_path(), - 'wolfpack/actions/action_3_pack_missing.json') + loader = fixtures_loader.FixturesLoader() + action_file = loader.get_fixture_file_path_abs( + 'generic', 'actions', 'action_3_pack_missing.json') registrar._register_action('dummy', action_file) action_name = None with open(action_file, 'r') as fd: diff --git a/st2common/st2common/validators/api/action.py b/st2common/st2common/validators/api/action.py index 23458b3039..52e4024629 100644 --- a/st2common/st2common/validators/api/action.py +++ b/st2common/st2common/validators/api/action.py @@ -28,13 +28,7 @@ def validate_action(action_api): - runner_db = None - # Check if runner exists. - try: - runner_db = get_runnertype_by_name(action_api.runner_type) - except StackStormDBObjectNotFoundError: - msg = 'RunnerType %s is not found.' % action_api.runner_type - raise ValueValidationException(msg) + runner_db = _get_runner_model(action_api) # Check if pack is valid. if not _is_valid_pack(action_api.pack): @@ -47,6 +41,17 @@ def validate_action(action_api): _validate_parameters(action_api.parameters, runner_db.runner_parameters) +def _get_runner_model(action_api): + runner_db = None + # Check if runner exists. + try: + runner_db = get_runnertype_by_name(action_api.runner_type) + except StackStormDBObjectNotFoundError: + msg = 'RunnerType %s is not found.' % action_api.runner_type + raise ValueValidationException(msg) + return runner_db + + def _is_valid_pack(pack): base_path = cfg.CONF.content.packs_base_path return os.path.exists(os.path.join(base_path, pack, 'actions')) From 38e45f13564bf0b533bfe9ca583aa0e0581e1c76 Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Wed, 24 Dec 2014 00:26:43 +0000 Subject: [PATCH 02/13] Refactor fixtures --- .../actions/action_2_bad_json.json | 0 .../actions/action_3_pack_missing.json | 0 .../st2tests/fixtures/wolfpack/actions/action_1.json | 12 ------------ 3 files changed, 12 deletions(-) rename st2tests/st2tests/fixtures/{wolfpack => generic}/actions/action_2_bad_json.json (100%) rename st2tests/st2tests/fixtures/{wolfpack => generic}/actions/action_3_pack_missing.json (100%) delete mode 100644 st2tests/st2tests/fixtures/wolfpack/actions/action_1.json diff --git a/st2tests/st2tests/fixtures/wolfpack/actions/action_2_bad_json.json b/st2tests/st2tests/fixtures/generic/actions/action_2_bad_json.json similarity index 100% rename from st2tests/st2tests/fixtures/wolfpack/actions/action_2_bad_json.json rename to st2tests/st2tests/fixtures/generic/actions/action_2_bad_json.json diff --git a/st2tests/st2tests/fixtures/wolfpack/actions/action_3_pack_missing.json b/st2tests/st2tests/fixtures/generic/actions/action_3_pack_missing.json similarity index 100% rename from st2tests/st2tests/fixtures/wolfpack/actions/action_3_pack_missing.json rename to st2tests/st2tests/fixtures/generic/actions/action_3_pack_missing.json diff --git a/st2tests/st2tests/fixtures/wolfpack/actions/action_1.json b/st2tests/st2tests/fixtures/wolfpack/actions/action_1.json deleted file mode 100644 index b4053452bd..0000000000 --- a/st2tests/st2tests/fixtures/wolfpack/actions/action_1.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "name": "st2.dummy.action1", - "description": "test description", - "enabled": true, - "pack": "wolfpack", - "entry_point": "/tmp/test/action1.sh", - "runner_type": "run-local", - "parameters": { - "a": {"type": "string", "default": "A1"}, - "b": {"type": "string", "default": "B1"} - } -} From 6d0a7f92e7c9851d3d6a7f65ee01842e9c7ba3ff Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Wed, 24 Dec 2014 00:39:17 +0000 Subject: [PATCH 03/13] Add test case for catching schema issues in action --- .../tests/unit/test_actions_registrar.py | 16 +++++++++- .../actions/action-invalid-schema-params.yaml | 32 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 st2tests/st2tests/fixtures/generic/actions/action-invalid-schema-params.yaml diff --git a/st2actions/tests/unit/test_actions_registrar.py b/st2actions/tests/unit/test_actions_registrar.py index 195ea3cbf5..7781d11706 100644 --- a/st2actions/tests/unit/test_actions_registrar.py +++ b/st2actions/tests/unit/test_actions_registrar.py @@ -17,7 +17,7 @@ import simplejson as json except ImportError: import json - +import jsonschema import mock import st2actions.bootstrap.actionsregistrar as actions_registrar @@ -73,3 +73,17 @@ def test_pack_name_missing(self): self.assertEqual(action_db.pack, 'dummy', 'Content pack must be ' + 'set to dummy') Action.delete(action_db) + + @mock.patch.object(action_validator, '_is_valid_pack', mock.MagicMock(return_value=True)) + @mock.patch.object(action_validator, '_get_runner_model', + mock.MagicMock(return_value=MOCK_RUNNER_TYPE_DB)) + def test_invalid_params_schema(self): + registrar = actions_registrar.ActionsRegistrar() + loader = fixtures_loader.FixturesLoader() + action_file = loader.get_fixture_file_path_abs( + 'generic', 'actions', 'action-invalid-schema-params.yaml') + try: + registrar._register_action('dummy', action_file) + self.fail('Invalid action schema. Should have failed.') + except jsonschema.ValidationError: + pass diff --git a/st2tests/st2tests/fixtures/generic/actions/action-invalid-schema-params.yaml b/st2tests/st2tests/fixtures/generic/actions/action-invalid-schema-params.yaml new file mode 100644 index 0000000000..2bbe3c7a9e --- /dev/null +++ b/st2tests/st2tests/fixtures/generic/actions/action-invalid-schema-params.yaml @@ -0,0 +1,32 @@ +--- + name: "st2_upgrade" + runner_type: "action-chain" + description: "Upgrades an existing st2 installation" + enabled: true + entry_point: "workflows/st2_upgrade.yaml" + parameters: + hostname: + type: "string" + description: "Host to upgrade st2 on" + required: true + action: + type: "string" + description: "Action to run after upgrade" + default: "core.local" + action_params: + type: "string" + description: "Parameters of action to be run" + default: "date" + repo: + type: "string" + # Note the quote after description. This is invalid. + description": "Git repository for this project" + build_server: + type: "string" + description: "build server" + branch: + type: "string" + description: "branch" + environment: + type: "string" + description: "environment" From a5d1661cea6c7e682be94e403e34e6ae1b3946d0 Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Wed, 24 Dec 2014 00:42:22 +0000 Subject: [PATCH 04/13] Update CHANGELOG --- CHANGELOG.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ba34d5c6a6..84688ad010 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -31,6 +31,9 @@ Docs: http://docks.stackstorm.com/latest and ``set_poll_interval`` methods respectively. (new-feature) * Add support for a ``standalone`` mode to the st2auth service. In the standalone mode, authentication is handled inside the st2auth service using the defined backend. (new feature) +* Timer is not a sensor anymore. It is spun as part of rules_engine process (refactor) +* Fix a bug with action registration where action with invalid schema for + parameters get registered. (bug-fix) v0.6.0 - December 8, 2014 ------------------------- From e4442f3fb255c2364a37661d82a0990a5d28ed47 Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Wed, 24 Dec 2014 01:07:22 +0000 Subject: [PATCH 05/13] Swap order of API object validation and model conversion --- st2actions/st2actions/bootstrap/actionsregistrar.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2actions/st2actions/bootstrap/actionsregistrar.py b/st2actions/st2actions/bootstrap/actionsregistrar.py index 99ab562669..cd450ca733 100644 --- a/st2actions/st2actions/bootstrap/actionsregistrar.py +++ b/st2actions/st2actions/bootstrap/actionsregistrar.py @@ -65,8 +65,8 @@ def _register_action(self, pack, action): LOG.info('Action %s found. Will be updated from: %s to: %s', action_ref, model, content) action_api = ActionAPI(**content) + action_validator.validate_action(action_api) model = ActionAPI.to_model(action_api) - action_validator.validate_action(model) try: model = Action.add_or_update(model) LOG.audit('Action created. Action %s from %s.', model, action) From 68cf5941b1cf40a486e92855a431df14f65cd2d4 Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Mon, 29 Dec 2014 20:19:29 +0000 Subject: [PATCH 06/13] Fix bug in action update during registration --- .../st2actions/bootstrap/actionsregistrar.py | 19 ++++++++++-------- .../tests/unit/test_actions_registrar.py | 20 +++++++++++++++++++ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/st2actions/st2actions/bootstrap/actionsregistrar.py b/st2actions/st2actions/bootstrap/actionsregistrar.py index cd450ca733..cddfa81d08 100644 --- a/st2actions/st2actions/bootstrap/actionsregistrar.py +++ b/st2actions/st2actions/bootstrap/actionsregistrar.py @@ -54,19 +54,22 @@ def _register_action(self, pack, action): content['pack'] = pack pack_field = pack if pack_field != pack: - raise Exception('Model is in pack: %s but field "pack" is different. %s' % - (pack, content.get(pack))) + raise Exception('Model is in pack "%s" but field "pack" is different: %s' % + (pack, pack_field)) + + action_api = ActionAPI(**content) + action_validator.validate_action(action_api) + model = ActionAPI.to_model(action_api) action_ref = ResourceReference(pack=pack, name=str(content['name'])) - model = action_utils.get_action_by_ref(action_ref) - if not model: + existing = action_utils.get_action_by_ref(action_ref) + if not existing: LOG.info('Action %s not found. Creating new one with: %s', action_ref, content) else: LOG.info('Action %s found. Will be updated from: %s to: %s', - action_ref, model, content) - action_api = ActionAPI(**content) - action_validator.validate_action(action_api) - model = ActionAPI.to_model(action_api) + action_ref, existing, model) + model.id = existing.id + try: model = Action.add_or_update(model) LOG.audit('Action created. Action %s from %s.', model, action) diff --git a/st2actions/tests/unit/test_actions_registrar.py b/st2actions/tests/unit/test_actions_registrar.py index 7781d11706..9d0fd179c2 100644 --- a/st2actions/tests/unit/test_actions_registrar.py +++ b/st2actions/tests/unit/test_actions_registrar.py @@ -87,3 +87,23 @@ def test_invalid_params_schema(self): self.fail('Invalid action schema. Should have failed.') except jsonschema.ValidationError: pass + + @mock.patch.object(action_validator, '_is_valid_pack', mock.MagicMock(return_value=True)) + @mock.patch.object(action_validator, '_get_runner_model', + mock.MagicMock(return_value=MOCK_RUNNER_TYPE_DB)) + def test_action_update(self): + registrar = actions_registrar.ActionsRegistrar() + loader = fixtures_loader.FixturesLoader() + action_file = loader.get_fixture_file_path_abs( + 'generic', 'actions', 'action1.json') + registrar._register_action('wolfpack', action_file) + # try registering again. this should not throw errors. + registrar._register_action('wolfpack', action_file) + action_name = None + with open(action_file, 'r') as fd: + content = json.load(fd) + action_name = str(content['name']) + action_db = Action.get_by_name(action_name) + self.assertEqual(action_db.pack, 'wolfpack', 'Content pack must be ' + + 'set to wolfpack') + Action.delete(action_db) From 239d6f453f937f51328e7c9f35a8076f81910ea2 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 29 Dec 2014 21:33:12 +0100 Subject: [PATCH 07/13] Update register-content.py script to default to registering nothing and update st2ctl reload command to pass --register-sensors --register-actions flags to the command if user doesn't specify which resource to register. --- st2common/st2common/content/bootstrap.py | 4 ++-- tools/st2ctl | 9 ++++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/st2common/st2common/content/bootstrap.py b/st2common/st2common/content/bootstrap.py index a66b8b7fde..55e4f4967b 100644 --- a/st2common/st2common/content/bootstrap.py +++ b/st2common/st2common/content/bootstrap.py @@ -29,8 +29,8 @@ def register_opts(): content_opts = [ cfg.BoolOpt('all', default=False, help='Register sensors, actions and rules.'), - cfg.BoolOpt('sensors', default=True, help='Register sensors.'), - cfg.BoolOpt('actions', default=True, help='Register actions.'), + cfg.BoolOpt('sensors', default=False, help='Register sensors.'), + cfg.BoolOpt('actions', default=False, help='Register actions.'), cfg.BoolOpt('rules', default=False, help='Register rules.') ] try: diff --git a/tools/st2ctl b/tools/st2ctl index 4af533bc8a..77b1e8e983 100755 --- a/tools/st2ctl +++ b/tools/st2ctl @@ -120,7 +120,14 @@ function restart_component() { function register_content() { echo "Registering content..." - $PYTHON ${PYTHONPACK}/st2common/bin/registercontent.py --config-file ${STANCONF} ${1} + + if [ ! ${1} ]; then + REGISTER_FLAGS="--register-sensors --register-actions" + else + REGISTER_FLAGS="--register-${1}" + fi + + $PYTHON ${PYTHONPACK}/st2common/bin/registercontent.py --config-file ${STANCONF} ${REGISTER_FLAGS} } clean_db() { From 0fdc7a626918bda09e01d4e97bfe6c400312ba8f Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Mon, 29 Dec 2014 22:18:15 +0000 Subject: [PATCH 08/13] 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 09/13] 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 10/13] 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 11/13] 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 12/13] 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" From 0815b8465f8c5cc6c125fcc31f6052be5b9de41f Mon Sep 17 00:00:00 2001 From: Lakshmi Kannan Date: Tue, 30 Dec 2014 00:12:28 +0000 Subject: [PATCH 13/13] Add bug-fix to CHANGELOG --- CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 84688ad010..9d27649bb8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -34,6 +34,7 @@ Docs: http://docks.stackstorm.com/latest * Timer is not a sensor anymore. It is spun as part of rules_engine process (refactor) * Fix a bug with action registration where action with invalid schema for parameters get registered. (bug-fix) +* Fix a bug with 'default' param values inheritance in runner/actions. (bug-fix) v0.6.0 - December 8, 2014 -------------------------