diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f3f3d768e3..63f25ef12a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -37,6 +37,14 @@ Changed Contributed by @khushboobhatia01 +Fixed +~~~~~ + +* Correct error reported when encrypted key value is reported, and another key value parameter that requires conversion is present. #5328 + Contributed by @amanda11, Ammeon Solutions + + + 3.5.0 - June 23, 2021 --------------------- diff --git a/st2api/st2api/controllers/v1/actionexecutions.py b/st2api/st2api/controllers/v1/actionexecutions.py index cc20c61f52..70d709192e 100644 --- a/st2api/st2api/controllers/v1/actionexecutions.py +++ b/st2api/st2api/controllers/v1/actionexecutions.py @@ -219,6 +219,7 @@ def _schedule_execution( liveaction=liveaction_db, action_db=action_db, runnertype_db=runnertype_db, + validate_params=False, ) # By this point the execution is already in the DB therefore need to mark it failed. diff --git a/st2api/tests/unit/controllers/v1/test_executions.py b/st2api/tests/unit/controllers/v1/test_executions.py index 5afcdbaeda..fb80a35917 100644 --- a/st2api/tests/unit/controllers/v1/test_executions.py +++ b/st2api/tests/unit/controllers/v1/test_executions.py @@ -198,6 +198,26 @@ "non_secret_param_2": {"type": "string", "required": True}, }, } +ACTION_DEFAULT_ENCRYPT_AND_BOOL = { + "name": "st2.dummy.default_encrypted_and_bool", + "description": "An action that uses a jinja template with decrypt_kv filter " + "in default parameter", + "enabled": True, + "pack": "starterpack", + "runner_type": "local-shell-cmd", + "parameters": { + "encrypted_param": { + "type": "string", + "default": "{{ st2kv.system.secret | decrypt_kv }}", + "secret": True, + }, + "bool_param": { + "type": "boolean", + "default": "{{ st2kv.system.test_bool }}", + }, + }, +} + LIVE_ACTION_1 = { "action": "sixpack.st2.dummy.action1", @@ -298,6 +318,9 @@ LIVE_ACTION_DEFAULT_ENCRYPT = { "action": "starterpack.st2.dummy.default_encrypted_value", } +LIVE_ACTION_DEFAULT_ENCRYPT_AND_BOOL = { + "action": "starterpack.st2.dummy.default_encrypted_and_bool", +} LIVE_ACTION_DEFAULT_ENCRYPT_SECRET_PARAM = { "action": "starterpack.st2.dummy.default_encrypted_value_secret_param", } @@ -359,6 +382,10 @@ def setUpClass(cls): post_resp = cls.app.post_json("/v1/actions", cls.action_decrypt) cls.action_decrypt["id"] = post_resp.json["id"] + cls.action_decrypt_and_bool = copy.deepcopy(ACTION_DEFAULT_ENCRYPT_AND_BOOL) + post_resp = cls.app.post_json("/v1/actions", cls.action_decrypt_and_bool) + cls.action_decrypt_and_bool["id"] = post_resp.json["id"] + cls.action_decrypt_secret_param = copy.deepcopy( ACTION_DEFAULT_ENCRYPT_SECRET_PARAMS ) @@ -390,6 +417,8 @@ def tearDownClass(cls): cls.app.delete("/v1/actions/%s" % cls.action_inquiry["id"]) cls.app.delete("/v1/actions/%s" % cls.action_template["id"]) cls.app.delete("/v1/actions/%s" % cls.action_decrypt["id"]) + cls.app.delete("/v1/actions/%s" % cls.action_decrypt_secret_param["id"]) + cls.app.delete("/v1/actions/%s" % cls.action_decrypt_and_bool["id"]) cls.app.delete( "/v1/actions/%s" % cls.action_with_output_schema_secret_param["id"] ) @@ -913,6 +942,26 @@ def test_template_param(self): # was not rendered, and the provided parameter was used self.assertEqual(post_resp.json["parameters"]["intparam"], live_int_param) + def test_template_encrypted_and_bool(self): + # register datastore values which are used in this test case + KeyValuePairAPI._setup_crypto() + register_items = [ + { + "name": "test_bool", + "value": "true", + }, + ] + [KeyValuePair.add_or_update(KeyValuePairDB(**x)) for x in register_items] + + post_resp = self._do_post( + LIVE_ACTION_DEFAULT_ENCRYPT_AND_BOOL, expect_errors=True + ) + self.assertEqual(post_resp.status_int, 400) + self.assertEqual( + post_resp.json["faultstring"], + 'Failed to render parameter "encrypted_param": Referenced datastore item "st2kv.system.secret" doesn\'t exist or it contains an empty string', + ) + def test_template_encrypted_params(self): # register datastore values which are used in this test case KeyValuePairAPI._setup_crypto() diff --git a/st2common/st2common/services/action.py b/st2common/st2common/services/action.py index b7bf650bde..cb5c9ebbe6 100644 --- a/st2common/st2common/services/action.py +++ b/st2common/st2common/services/action.py @@ -55,7 +55,9 @@ def _get_immutable_params(parameters): return [k for k, v in six.iteritems(parameters) if v.get("immutable", False)] -def create_request(liveaction, action_db=None, runnertype_db=None): +def create_request( + liveaction, action_db=None, runnertype_db=None, validate_params=True +): """ Create an action execution. @@ -66,6 +68,9 @@ def create_request(liveaction, action_db=None, runnertype_db=None): :param runnertype_db: Runner model to operate one. If not provided, one is retrieved from the database using values from "liveaction". :type runnertype_db: :class:`RunnerTypeDB` + :param validate_params: Whether to validate parameters against schema. Default to True, but + set to False when raising a request to report an error. + :type validate_params: ``bool`` :return: (liveaction, execution) :rtype: tuple @@ -108,13 +113,14 @@ def create_request(liveaction, action_db=None, runnertype_db=None): # Validate action parameters. schema = util_schema.get_schema_for_action_parameters(action_db, runnertype_db) validator = util_schema.get_validator() - util_schema.validate( - liveaction.parameters, - schema, - validator, - use_default=True, - allow_default_none=True, - ) + if validate_params: + util_schema.validate( + liveaction.parameters, + schema, + validator, + use_default=True, + allow_default_none=True, + ) # validate that no immutable params are being overriden. Although possible to # ignore the override it is safer to inform the user to avoid surprises. diff --git a/st2common/tests/unit/services/test_action.py b/st2common/tests/unit/services/test_action.py index ab8db72329..e27126ec86 100644 --- a/st2common/tests/unit/services/test_action.py +++ b/st2common/tests/unit/services/test_action.py @@ -624,3 +624,25 @@ def test_root_liveaction(self): child, expected_root = self._create_nested_executions(depth=i) actual_root = action_service.get_root_liveaction(child) self.assertEqual(expected_root["id"], actual_root["id"]) + + def test_invalid_json_request_validate(self): + parameters = {"hosts": "127.0.0.1", "cmd": "uname -a", "arg_default_value": 123} + liveaction = LiveActionDB(action=ACTION_REF, parameters=parameters) + + self.assertRaisesRegexp( + jsonschema.ValidationError, + "123 is not of type 'string'", + action_service.create_request, + liveaction, + ) + + def test_invalid_json_request_skipvalidate(self): + parameters = {"hosts": "127.0.0.1", "cmd": "uname -a", "arg_default_value": 123} + liveaction = LiveActionDB(action=ACTION_REF, parameters=parameters) + + # Validate that if skip validation that no exception raised + (action, execution) = action_service.create_request( + liveaction, validate_params=False + ) + self.assertTrue(action) + self.assertTrue(execution)