From 7589139cec106c8f01e037f0186ed3ad84ebe007 Mon Sep 17 00:00:00 2001 From: shivani-orch Date: Thu, 29 Apr 2021 11:44:00 +0000 Subject: [PATCH 01/40] Secret masking in output_schema feature is added --- st2common/st2common/util/output_schema.py | 48 +++++++++- .../tests/unit/test_util_output_schema.py | 90 +++++++++++++++++++ 2 files changed, 134 insertions(+), 4 deletions(-) diff --git a/st2common/st2common/util/output_schema.py b/st2common/st2common/util/output_schema.py index 2bde19c3c0..3934435c5a 100644 --- a/st2common/st2common/util/output_schema.py +++ b/st2common/st2common/util/output_schema.py @@ -20,6 +20,7 @@ from st2common.util import schema from st2common.constants import action as action_constants +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE LOG = logging.getLogger(__name__) @@ -37,17 +38,56 @@ def _validate_runner(runner_schema, result): schema.validate(result, runner_schema, cls=schema.get_validator("custom")) -def _validate_action(action_schema, result, output_key): - LOG.debug("Validating action output: %s", action_schema) +def _prepare_action_schema(action_schema): + """ + Prepares the final action schema. - final_result = result[output_key] + :param action_schema: action schema of a action execution ouput. + :return: final_action_schema: along with type and additionalProperties flag. + :rtype: ``dict``. + """ - action_schema = { + final_action_schema = { "type": "object", "properties": action_schema, "additionalProperties": False, } + return final_action_schema + + +def output_schema_secret_masking(result, output_key, action_schema): + """ + Masks the secret parameters provided in output schema. + + :param result: result of the action execution. + :param output_key: key for parsing specific result from action execution result. + :param action_schema: action schema of a action execution ouput. + :return: final_result: to be displayed in CLI or Web UI with masked secrets. + :rtype: ``dict``. + """ + + final_result = result[output_key] + + final_action_schema = action_schema + + # accessing parameters marked secret as true in the output_schema in + # action_schema and masking them for the final result of the output + for key in final_action_schema["properties"]: + if final_action_schema.get("properties", {}).get(key).get("secret", False): + final_result[key] = MASKED_ATTRIBUTE_VALUE + + return final_result + + +def _validate_action(action_schema, result, output_key): + LOG.debug("Validating action output: %s", action_schema) + + action_schema = _prepare_action_schema(action_schema) + final_result = output_schema_secret_masking(result=result, + output_key=output_key, + action_schema=action_schema) + schema.validate(final_result, action_schema, cls=schema.get_validator("custom")) diff --git a/st2common/tests/unit/test_util_output_schema.py b/st2common/tests/unit/test_util_output_schema.py index af9570d4fa..a138ef9b64 100644 --- a/st2common/tests/unit/test_util_output_schema.py +++ b/st2common/tests/unit/test_util_output_schema.py @@ -58,6 +58,60 @@ "not_a_key_you_have": {"type": "string"}, } +ACTION_SCHEMA_WITH_SECRET_PARAMS = { + "type": "object", + "properties": { + "param_1": {"type": "string", "required": True, "secret": True}, + "param_2": {"type": "string", "required": True}, + "param_3": { + "type": "string", + "required": True, + "secret": True, + "description": "sample description", + }, + }, + "additionalProperties": False, +} + +RESULT_BEFORE_MASKING_WITH_SECRET_PARAMS = { + "stdout": "", + "stderr": "", + "exit_code": 0, + "result": { + "param_1": "to_be_masked", + "param_2": "not_to_be_masked", + "param_3": "to_be_masked", + }, +} + +ACTION_SCHEMA_WITHOUT_SECRET_PARAMS = { + "type": "object", + "properties": { + "param_1": { + "type": "string", + "required": True, + }, + "param_2": {"type": "string", "required": True}, + "param_3": { + "type": "string", + "required": True, + "description": "sample description", + }, + }, + "additionalProperties": False, +} + +RESULT_BEFORE_MASKING_WITHOUT_SECRET_PARAMS = { + "stdout": "", + "stderr": "", + "exit_code": 0, + "result": { + "param_1": "not_to_be_masked", + "param_2": "not_to_be_masked", + "param_3": "not_to_be_masked", + }, +} + OUTPUT_KEY = "output" @@ -117,3 +171,39 @@ def test_invalid_action_schema(self): self.assertIn(expected_result["error"], result["error"]) self.assertEqual(result["message"], expected_result["message"]) self.assertEqual(status, LIVEACTION_STATUS_FAILED) + + def test_output_schema_secret_masking(self): + """Test case for testing final output result for the output schema with secret parameters.""" + + OUTPUT_KEY = "result" + result = output_schema.output_schema_secret_masking( + result=RESULT_BEFORE_MASKING_WITH_SECRET_PARAMS, + output_key=OUTPUT_KEY, + action_schema=ACTION_SCHEMA_WITH_SECRET_PARAMS, + ) + + expected_result = { + "param_1": MASKED_ATTRIBUTE_VALUE, + "param_2": "not_to_be_masked", + "param_3": MASKED_ATTRIBUTE_VALUE, + } + + self.assertEqual(result, expected_result) + + def test_output_schema_without_secret_params(self): + """Test case for testing final output result for the output schema without secret parameters.""" + + OUTPUT_KEY = "result" + result = output_schema.output_schema_secret_masking( + result=RESULT_BEFORE_MASKING_WITHOUT_SECRET_PARAMS, + output_key=OUTPUT_KEY, + action_schema=ACTION_SCHEMA_WITHOUT_SECRET_PARAMS, + ) + + expected_result = { + "param_1": "not_to_be_masked", + "param_2": "not_to_be_masked", + "param_3": "not_to_be_masked", + } + + self.assertEqual(result, expected_result) From 44952711eb12ef3016bdcaa93df9e0ac9ef37cd2 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 29 Apr 2021 19:30:22 +0530 Subject: [PATCH 02/40] Update test_util_output_schema.py adding a missed import --- st2common/tests/unit/test_util_output_schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_util_output_schema.py b/st2common/tests/unit/test_util_output_schema.py index a138ef9b64..5294dad049 100644 --- a/st2common/tests/unit/test_util_output_schema.py +++ b/st2common/tests/unit/test_util_output_schema.py @@ -16,7 +16,7 @@ import unittest2 from st2common.util import output_schema - +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE from st2common.constants.action import ( LIVEACTION_STATUS_SUCCEEDED, LIVEACTION_STATUS_FAILED, From 475d631cf8ff87e4649eefe9a751087650a1e80b Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 10 May 2021 10:48:26 +0530 Subject: [PATCH 03/40] Update output_schema.py --- st2common/st2common/util/output_schema.py | 48 ++--------------------- 1 file changed, 4 insertions(+), 44 deletions(-) diff --git a/st2common/st2common/util/output_schema.py b/st2common/st2common/util/output_schema.py index 3934435c5a..2bde19c3c0 100644 --- a/st2common/st2common/util/output_schema.py +++ b/st2common/st2common/util/output_schema.py @@ -20,7 +20,6 @@ from st2common.util import schema from st2common.constants import action as action_constants -from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE LOG = logging.getLogger(__name__) @@ -38,56 +37,17 @@ def _validate_runner(runner_schema, result): schema.validate(result, runner_schema, cls=schema.get_validator("custom")) -def _prepare_action_schema(action_schema): - """ - Prepares the final action schema. +def _validate_action(action_schema, result, output_key): + LOG.debug("Validating action output: %s", action_schema) - :param action_schema: action schema of a action execution ouput. - :return: final_action_schema: along with type and additionalProperties flag. - :rtype: ``dict``. - """ + final_result = result[output_key] - final_action_schema = { + action_schema = { "type": "object", "properties": action_schema, "additionalProperties": False, } - return final_action_schema - - -def output_schema_secret_masking(result, output_key, action_schema): - """ - Masks the secret parameters provided in output schema. - - :param result: result of the action execution. - :param output_key: key for parsing specific result from action execution result. - :param action_schema: action schema of a action execution ouput. - :return: final_result: to be displayed in CLI or Web UI with masked secrets. - :rtype: ``dict``. - """ - - final_result = result[output_key] - - final_action_schema = action_schema - - # accessing parameters marked secret as true in the output_schema in - # action_schema and masking them for the final result of the output - for key in final_action_schema["properties"]: - if final_action_schema.get("properties", {}).get(key).get("secret", False): - final_result[key] = MASKED_ATTRIBUTE_VALUE - - return final_result - - -def _validate_action(action_schema, result, output_key): - LOG.debug("Validating action output: %s", action_schema) - - action_schema = _prepare_action_schema(action_schema) - final_result = output_schema_secret_masking(result=result, - output_key=output_key, - action_schema=action_schema) - schema.validate(final_result, action_schema, cls=schema.get_validator("custom")) From 335d9933391ead23fedecd29c27ef08a0807a399 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 10 May 2021 10:53:28 +0530 Subject: [PATCH 04/40] Update test_util_output_schema.py --- .../tests/unit/test_util_output_schema.py | 92 +------------------ 1 file changed, 1 insertion(+), 91 deletions(-) diff --git a/st2common/tests/unit/test_util_output_schema.py b/st2common/tests/unit/test_util_output_schema.py index 5294dad049..af9570d4fa 100644 --- a/st2common/tests/unit/test_util_output_schema.py +++ b/st2common/tests/unit/test_util_output_schema.py @@ -16,7 +16,7 @@ import unittest2 from st2common.util import output_schema -from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE + from st2common.constants.action import ( LIVEACTION_STATUS_SUCCEEDED, LIVEACTION_STATUS_FAILED, @@ -58,60 +58,6 @@ "not_a_key_you_have": {"type": "string"}, } -ACTION_SCHEMA_WITH_SECRET_PARAMS = { - "type": "object", - "properties": { - "param_1": {"type": "string", "required": True, "secret": True}, - "param_2": {"type": "string", "required": True}, - "param_3": { - "type": "string", - "required": True, - "secret": True, - "description": "sample description", - }, - }, - "additionalProperties": False, -} - -RESULT_BEFORE_MASKING_WITH_SECRET_PARAMS = { - "stdout": "", - "stderr": "", - "exit_code": 0, - "result": { - "param_1": "to_be_masked", - "param_2": "not_to_be_masked", - "param_3": "to_be_masked", - }, -} - -ACTION_SCHEMA_WITHOUT_SECRET_PARAMS = { - "type": "object", - "properties": { - "param_1": { - "type": "string", - "required": True, - }, - "param_2": {"type": "string", "required": True}, - "param_3": { - "type": "string", - "required": True, - "description": "sample description", - }, - }, - "additionalProperties": False, -} - -RESULT_BEFORE_MASKING_WITHOUT_SECRET_PARAMS = { - "stdout": "", - "stderr": "", - "exit_code": 0, - "result": { - "param_1": "not_to_be_masked", - "param_2": "not_to_be_masked", - "param_3": "not_to_be_masked", - }, -} - OUTPUT_KEY = "output" @@ -171,39 +117,3 @@ def test_invalid_action_schema(self): self.assertIn(expected_result["error"], result["error"]) self.assertEqual(result["message"], expected_result["message"]) self.assertEqual(status, LIVEACTION_STATUS_FAILED) - - def test_output_schema_secret_masking(self): - """Test case for testing final output result for the output schema with secret parameters.""" - - OUTPUT_KEY = "result" - result = output_schema.output_schema_secret_masking( - result=RESULT_BEFORE_MASKING_WITH_SECRET_PARAMS, - output_key=OUTPUT_KEY, - action_schema=ACTION_SCHEMA_WITH_SECRET_PARAMS, - ) - - expected_result = { - "param_1": MASKED_ATTRIBUTE_VALUE, - "param_2": "not_to_be_masked", - "param_3": MASKED_ATTRIBUTE_VALUE, - } - - self.assertEqual(result, expected_result) - - def test_output_schema_without_secret_params(self): - """Test case for testing final output result for the output schema without secret parameters.""" - - OUTPUT_KEY = "result" - result = output_schema.output_schema_secret_masking( - result=RESULT_BEFORE_MASKING_WITHOUT_SECRET_PARAMS, - output_key=OUTPUT_KEY, - action_schema=ACTION_SCHEMA_WITHOUT_SECRET_PARAMS, - ) - - expected_result = { - "param_1": "not_to_be_masked", - "param_2": "not_to_be_masked", - "param_3": "not_to_be_masked", - } - - self.assertEqual(result, expected_result) From 78fad6bf106c6f0d2c21574d20a2b7fc6302993f Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 10 May 2021 12:01:19 +0530 Subject: [PATCH 05/40] Update execution.py --- st2common/st2common/models/api/execution.py | 30 +++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/st2common/st2common/models/api/execution.py b/st2common/st2common/models/api/execution.py index 76aa9fbdf8..807a0c4d4d 100644 --- a/st2common/st2common/models/api/execution.py +++ b/st2common/st2common/models/api/execution.py @@ -29,6 +29,7 @@ from st2common.models.api.action import RunnerTypeAPI, ActionAPI, LiveActionAPI from st2common import log as logging from st2common.util.deep_copy import fast_deepcopy_dict +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE __all__ = ["ActionExecutionAPI", "ActionExecutionOutputAPI"] @@ -151,10 +152,35 @@ class ActionExecutionAPI(BaseAPI): @classmethod def from_model(cls, model, mask_secrets=False): - doc = cls._from_model(model, mask_secrets=mask_secrets) + """ + Retrieve provided DB model instance and create API model class instance for the same. + + :param model: DB model class instance. + :type model: :class: ``ActionExecutionDB`` - doc["result"] = ActionExecutionDB.result.parse_field_value(doc["result"]) + :param mask_secrets: flag to mask the secrets or not. + :type mask_secrets: ``boolean`` + :return: cls(**attrs): to be displayed in CLI or Web UI with masked secrets. + :rtype: class: ``ActionExecutionAPI`` + """ + + doc = cls._from_model(model, mask_secrets=mask_secrets) + output_schema_result = ActionExecutionDB.result.parse_field_value(doc["result"]) + + # accessing parameters marked secret as true in the output_schema + # and masking them for the result in action execution API + if output_schema_result: + for key in doc["action"]["output_schema"]: + if ( + doc.get("action", {}) + .get("output_schema", {}) + .get(key) + .get("secret", False) + ): + output_schema_result["result"][key] = MASKED_ATTRIBUTE_VALUE + + doc["result"] = output_schema_result start_timestamp = model.start_timestamp start_timestamp_iso = isotime.format(start_timestamp, offset=False) doc["start_timestamp"] = start_timestamp_iso From 2720762976f8191afca63ff32f70c8fa9b8c0cc6 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 12 May 2021 10:29:12 +0530 Subject: [PATCH 06/40] Moving logic to /st2common/models/db/execution.py --- st2common/st2common/models/api/execution.py | 32 ++------------------- 1 file changed, 3 insertions(+), 29 deletions(-) diff --git a/st2common/st2common/models/api/execution.py b/st2common/st2common/models/api/execution.py index 807a0c4d4d..3f037412a5 100644 --- a/st2common/st2common/models/api/execution.py +++ b/st2common/st2common/models/api/execution.py @@ -29,7 +29,6 @@ from st2common.models.api.action import RunnerTypeAPI, ActionAPI, LiveActionAPI from st2common import log as logging from st2common.util.deep_copy import fast_deepcopy_dict -from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE __all__ = ["ActionExecutionAPI", "ActionExecutionOutputAPI"] @@ -152,35 +151,10 @@ class ActionExecutionAPI(BaseAPI): @classmethod def from_model(cls, model, mask_secrets=False): - """ - Retrieve provided DB model instance and create API model class instance for the same. - - :param model: DB model class instance. - :type model: :class: ``ActionExecutionDB`` - - :param mask_secrets: flag to mask the secrets or not. - :type mask_secrets: ``boolean`` - - :return: cls(**attrs): to be displayed in CLI or Web UI with masked secrets. - :rtype: class: ``ActionExecutionAPI`` - """ - doc = cls._from_model(model, mask_secrets=mask_secrets) - output_schema_result = ActionExecutionDB.result.parse_field_value(doc["result"]) - - # accessing parameters marked secret as true in the output_schema - # and masking them for the result in action execution API - if output_schema_result: - for key in doc["action"]["output_schema"]: - if ( - doc.get("action", {}) - .get("output_schema", {}) - .get(key) - .get("secret", False) - ): - output_schema_result["result"][key] = MASKED_ATTRIBUTE_VALUE - - doc["result"] = output_schema_result + + doc["result"] = ActionExecutionDB.result.parse_field_value(doc["result"]) + start_timestamp = model.start_timestamp start_timestamp_iso = isotime.format(start_timestamp, offset=False) doc["start_timestamp"] = start_timestamp_iso From 4511f057d7e353122c272f0507c25202dda6c617 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 12 May 2021 10:44:25 +0530 Subject: [PATCH 07/40] added code for masking output schema secret params --- st2common/st2common/models/db/execution.py | 33 ++++++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/st2common/st2common/models/db/execution.py b/st2common/st2common/models/db/execution.py index 4950965a7e..472dc9ae97 100644 --- a/st2common/st2common/models/db/execution.py +++ b/st2common/st2common/models/db/execution.py @@ -28,6 +28,7 @@ from st2common.util.secrets import mask_inquiry_response from st2common.util.secrets import mask_secret_parameters from st2common.constants.types import ResourceType +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE __all__ = ["ActionExecutionDB", "ActionExecutionOutputDB"] @@ -105,6 +106,16 @@ def get_uid(self): return ":".join(uid) def mask_secrets(self, value): + """ + Masks the secret parameters in input and output schema for action execution output. + + :param value: action execution object. + :type value: ``dict`` + + :return: result: action execution object with masked secret paramters in input and output schema. + :rtype: result: ``dict`` + """ + result = copy.deepcopy(value) liveaction = result["liveaction"] @@ -141,11 +152,27 @@ def mask_secrets(self, value): p: "string" for p in liveaction["parameters"]["response"] }, ) - + + output_schema_result = ActionExecutionDB.result.parse_field_value( + result["result"] + ) + + # accessing parameters marked secret as true in the output_schema + # and masking them for the result in action execution API + if output_schema_result: + for key in result["action"]["output_schema"]: + if ( + result.get("action", {}) + .get("output_schema", {}) + .get(key) + .get("secret", False) + ): + output_schema_result["result"][key] = MASKED_ATTRIBUTE_VALUE + + result["result"] = output_schema_result + # TODO(mierdin): This logic should be moved to the dedicated Inquiry # data model once it exists. - result["result"] = ActionExecutionDB.result.parse_field_value(result["result"]) - if self.runner.get("name") == "inquirer": schema = result["result"].get("schema", {}) response = result["result"].get("response", {}) From 1e38f982425e26b7fa8ffa85e067750fcd73aeb5 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 12 May 2021 11:10:43 +0530 Subject: [PATCH 08/40] adding a unit test and updating existing tests --- st2common/tests/unit/test_db_execution.py | 48 ++++++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/st2common/tests/unit/test_db_execution.py b/st2common/tests/unit/test_db_execution.py index e94ccb3d94..e4c551732e 100644 --- a/st2common/tests/unit/test_db_execution.py +++ b/st2common/tests/unit/test_db_execution.py @@ -72,21 +72,48 @@ "action": "st2.inquiry.respond", } +OUTPUT_SCHEMA_RESULT = { + "stdout": "", + "stderr": "", + "result": { + "os_secret_param": "to_be_masked", + "os_non_secret_param": "not_to_be_masked", + }, +} + +OUTPUT_SCHEMA_LIVEACTION = { + "action": "core.ask", + "parameters": {}, +} + ACTIONEXECUTIONS = { "execution_1": { - "action": {"uid": "action:core:ask"}, + "action": {"uid": "action:core:ask", "output_schema": {}}, "status": "succeeded", "runner": {"name": "inquirer"}, "liveaction": INQUIRY_LIVEACTION, "result": INQUIRY_RESULT, }, "execution_2": { - "action": {"uid": "action:st2:inquiry.respond"}, + "action": {"uid": "action:st2:inquiry.respond", "output_schema": {}}, "status": "succeeded", "runner": {"name": "python-script"}, "liveaction": RESPOND_LIVEACTION, "result": {"exit_code": 0, "result": None, "stderr": "", "stdout": ""}, }, + "execution_3": { + "action": { + "uid": "action:core:ask", + "output_schema": { + "os_secret_param": {"type": "string", "required": True, "secret": True}, + "os_non_secret_param": {"type": "string", "required": True}, + }, + }, + "status": "succeeded", + "runner": {"name": "inquirer"}, + "liveaction": OUTPUT_SCHEMA_LIVEACTION, + "result": OUTPUT_SCHEMA_RESULT, + }, } @@ -166,6 +193,23 @@ def test_execution_inquiry_response_action(self): for value in masked["parameters"]["response"].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) + def test_ouput_schema_secret_param_masking(self): + """Test that the parameter marked secret as true in output schema is maked in the output result + + here out of two parameters in output schema is marked secret as true and we are asserting only + that is masked in the output result. + """ + + masked = self.executions["execution_3"].mask_secrets( + self.executions["execution_3"].to_serializable_dict() + ) + self.assertEqual( + masked["result"]["result"]["os_secret_param"], MASKED_ATTRIBUTE_VALUE + ) + self.assertEqual( + masked["result"]["result"]["os_non_secret_param"], "not_to_be_masked" + ) + @staticmethod def _save_execution(execution): return ActionExecution.add_or_update(execution) From 8f8822ab37fb3d2df44e63567a4a78d0b29c8cdc Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 12 May 2021 17:47:34 +0530 Subject: [PATCH 09/40] Update st2common/st2common/models/api/execution.py Co-authored-by: Jacob Floyd --- st2common/st2common/models/api/execution.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/st2common/models/api/execution.py b/st2common/st2common/models/api/execution.py index 3f037412a5..e68deb5b62 100644 --- a/st2common/st2common/models/api/execution.py +++ b/st2common/st2common/models/api/execution.py @@ -152,7 +152,6 @@ class ActionExecutionAPI(BaseAPI): @classmethod def from_model(cls, model, mask_secrets=False): doc = cls._from_model(model, mask_secrets=mask_secrets) - doc["result"] = ActionExecutionDB.result.parse_field_value(doc["result"]) start_timestamp = model.start_timestamp From a97f5313c1c348e26b1b64d4e4b709c41fae66e0 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 12 May 2021 17:47:59 +0530 Subject: [PATCH 10/40] Update st2common/st2common/models/api/execution.py Co-authored-by: Jacob Floyd --- st2common/st2common/models/api/execution.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/st2common/models/api/execution.py b/st2common/st2common/models/api/execution.py index e68deb5b62..743d272d3d 100644 --- a/st2common/st2common/models/api/execution.py +++ b/st2common/st2common/models/api/execution.py @@ -153,7 +153,6 @@ class ActionExecutionAPI(BaseAPI): def from_model(cls, model, mask_secrets=False): doc = cls._from_model(model, mask_secrets=mask_secrets) doc["result"] = ActionExecutionDB.result.parse_field_value(doc["result"]) - start_timestamp = model.start_timestamp start_timestamp_iso = isotime.format(start_timestamp, offset=False) doc["start_timestamp"] = start_timestamp_iso From f65c27d0c70d93252c2bebf59c9872bd8cadbb17 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 12 May 2021 17:52:31 +0530 Subject: [PATCH 11/40] Adding blank lines in st2common/st2common/models/api/execution.py --- st2common/st2common/models/api/execution.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/st2common/st2common/models/api/execution.py b/st2common/st2common/models/api/execution.py index 743d272d3d..76aa9fbdf8 100644 --- a/st2common/st2common/models/api/execution.py +++ b/st2common/st2common/models/api/execution.py @@ -152,7 +152,9 @@ class ActionExecutionAPI(BaseAPI): @classmethod def from_model(cls, model, mask_secrets=False): doc = cls._from_model(model, mask_secrets=mask_secrets) + doc["result"] = ActionExecutionDB.result.parse_field_value(doc["result"]) + start_timestamp = model.start_timestamp start_timestamp_iso = isotime.format(start_timestamp, offset=False) doc["start_timestamp"] = start_timestamp_iso From b220ae440d5141ff5e86b1272039259330b9acc6 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 12 May 2021 18:02:17 +0530 Subject: [PATCH 12/40] Updating /st2common/tests/unit/test_db_execution.py --- st2common/tests/unit/test_db_execution.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/st2common/tests/unit/test_db_execution.py b/st2common/tests/unit/test_db_execution.py index e4c551732e..80c0124f5e 100644 --- a/st2common/tests/unit/test_db_execution.py +++ b/st2common/tests/unit/test_db_execution.py @@ -193,10 +193,11 @@ def test_execution_inquiry_response_action(self): for value in masked["parameters"]["response"].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) - def test_ouput_schema_secret_param_masking(self): - """Test that the parameter marked secret as true in output schema is maked in the output result + def test_output_schema_secret_param_masking(self): + """ + Test that the parameter marked secret as true in output schema is masked in the output result - here out of two parameters in output schema is marked secret as true and we are asserting only + here, out of two parameters in output schema is marked secret as true and we are asserting only that is masked in the output result. """ From 86ae3cf93c2ae147a975ac6ca7f1545310efc2ff Mon Sep 17 00:00:00 2001 From: W Chan Date: Sat, 15 May 2021 04:36:51 +0000 Subject: [PATCH 13/40] Add unit tests to cover masking of output for various action runners The output_key for each action runners are different. Python runner writes output to "result", HTTP runner writes output to "body", and Orquesta runner writes output to "output". Additional unit tests are added here to make sure secrets in output are masked appropriately for different runners. The output_key from the runner model should be used to dynamically determine which field to apply the masking to. --- st2actions/tests/unit/test_output_schema.py | 129 ++++++++++++++++++ st2common/st2common/models/db/execution.py | 19 ++- .../packs/dummy_pack_1/actions/my_action.py | 3 +- .../packs/dummy_pack_1/actions/my_action.yaml | 23 ++-- .../dummy_pack_1/actions/my_http_action.yaml | 22 +++ st2tests/st2tests/mocks/liveaction.py | 9 ++ 6 files changed, 185 insertions(+), 20 deletions(-) create mode 100644 st2actions/tests/unit/test_output_schema.py create mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_http_action.yaml diff --git a/st2actions/tests/unit/test_output_schema.py b/st2actions/tests/unit/test_output_schema.py new file mode 100644 index 0000000000..ac81c0a1bb --- /dev/null +++ b/st2actions/tests/unit/test_output_schema.py @@ -0,0 +1,129 @@ +# Copyright 2021 The StackStorm Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import absolute_import + +import mock + +from http_runner import http_runner +from python_runner import python_runner + +import st2tests + +import st2tests.config as tests_config + +tests_config.parse_args() + +from st2common.bootstrap import actionsregistrar +from st2common.bootstrap import runnersregistrar +from st2common.constants import action as ac_const +from st2common.constants import secrets as secrets_const +from st2common.models.api import execution as ex_api_models +from st2common.models.db import liveaction as lv_db_models +from st2common.services import action as action_service +from st2common.transport import liveaction as lv_ac_xport +from st2common.transport import publishers +from st2tests.mocks import liveaction as mock_lv_ac_xport + + +TEST_PACK = "dummy_pack_1" +TEST_PACK_PATH = st2tests.fixturesloader.get_fixtures_packs_base_path() + "/" + TEST_PACK + +PACKS = [ + TEST_PACK_PATH, +] + +MOCK_PYTHON_ACTION_RESULT = { + 'stderr': '', + 'stdout': '', + 'result': {'k1': 'foobar', 'k2': 'shhhh!'}, + 'exit_code': 0, +} + +MOCK_PYTHON_RUNNER_OUTPUT = (ac_const.LIVEACTION_STATUS_SUCCEEDED, MOCK_PYTHON_ACTION_RESULT, None) + +MOCK_HTTP_ACTION_RESULT = { + 'status_code': 200, + 'body': {'k1': 'foobar', 'k2': 'shhhh!'}, +} + +MOCK_HTTP_RUNNER_OUTPUT = (ac_const.LIVEACTION_STATUS_SUCCEEDED, MOCK_HTTP_ACTION_RESULT, None) + + +@mock.patch.object( + publishers.CUDPublisher, "publish_update", mock.MagicMock(return_value=None) +) +@mock.patch.object( + publishers.CUDPublisher, + "publish_create", + mock.MagicMock(side_effect=mock_lv_ac_xport.MockLiveActionPublisher.publish_create), +) +@mock.patch.object( + lv_ac_xport.LiveActionPublisher, + "publish_state", + mock.MagicMock(side_effect=mock_lv_ac_xport.MockLiveActionPublisher.publish_state), +) +class ActionExecutionOutputSchemaTest(st2tests.ExecutionDbTestCase): + @classmethod + def setUpClass(cls): + super(ActionExecutionOutputSchemaTest, cls).setUpClass() + + # Register runners. + runnersregistrar.register_runners() + + # Register test pack(s). + actions_registrar = actionsregistrar.ActionsRegistrar( + use_pack_cache=False, fail_on_failure=True + ) + + for pack in PACKS: + actions_registrar.register_from_pack(pack) + + @mock.patch.object( + python_runner.PythonRunner, "run", + mock.MagicMock(return_value=MOCK_PYTHON_RUNNER_OUTPUT), + ) + def test_python_action(self): + # Execute a python action with output schema and secret + lv_ac_db = lv_db_models.LiveActionDB(action="dummy_pack_1.my_action") + lv_ac_db, ac_ex_db = action_service.request(lv_ac_db) + ac_ex_db = self._wait_on_ac_ex_status(ac_ex_db, ac_const.LIVEACTION_STATUS_SUCCEEDED) + + # Assert expected output written to the database + expected_output = {'k1': 'foobar', 'k2': 'shhhh!'} + self.assertDictEqual(ac_ex_db.result['result'], expected_output) + + # Assert expected output on conversion to API model + ac_ex_api = ex_api_models.ActionExecutionAPI.from_model(ac_ex_db, mask_secrets=True) + expected_masked_output = {'k1': 'foobar', 'k2': secrets_const.MASKED_ATTRIBUTE_VALUE} + self.assertDictEqual(ac_ex_api.result['result'], expected_masked_output) + + @mock.patch.object( + http_runner.HttpRunner, "run", + mock.MagicMock(return_value=MOCK_HTTP_RUNNER_OUTPUT), + ) + def test_http_action(self): + # Execute a http action with output schema and secret + lv_ac_db = lv_db_models.LiveActionDB(action="dummy_pack_1.my_http_action") + lv_ac_db, ac_ex_db = action_service.request(lv_ac_db) + ac_ex_db = self._wait_on_ac_ex_status(ac_ex_db, ac_const.LIVEACTION_STATUS_SUCCEEDED) + + # Assert expected output written to the database + expected_output = {'k1': 'foobar', 'k2': 'shhhh!'} + self.assertDictEqual(ac_ex_db.result['body'], expected_output) + + # Assert expected output on conversion to API model + ac_ex_api = ex_api_models.ActionExecutionAPI.from_model(ac_ex_db, mask_secrets=True) + expected_masked_output = {'k1': 'foobar', 'k2': secrets_const.MASKED_ATTRIBUTE_VALUE} + self.assertDictEqual(ac_ex_api.result['body'], expected_masked_output) diff --git a/st2common/st2common/models/db/execution.py b/st2common/st2common/models/db/execution.py index 472dc9ae97..ba6a952722 100644 --- a/st2common/st2common/models/db/execution.py +++ b/st2common/st2common/models/db/execution.py @@ -16,6 +16,7 @@ from __future__ import absolute_import import copy +import six import mongoengine as me @@ -115,7 +116,7 @@ def mask_secrets(self, value): :return: result: action execution object with masked secret paramters in input and output schema. :rtype: result: ``dict`` """ - + result = copy.deepcopy(value) liveaction = result["liveaction"] @@ -152,25 +153,21 @@ def mask_secrets(self, value): p: "string" for p in liveaction["parameters"]["response"] }, ) - + output_schema_result = ActionExecutionDB.result.parse_field_value( result["result"] ) - + # accessing parameters marked secret as true in the output_schema # and masking them for the result in action execution API if output_schema_result: - for key in result["action"]["output_schema"]: - if ( - result.get("action", {}) - .get("output_schema", {}) - .get(key) - .get("secret", False) - ): + for key, spec in six.iteritems(result["action"]["output_schema"]): + if spec.get("secret", False): + # TODO: Change to output_schema_result[output_key][key] = MASKED_ATTRIBUTE_VALUE output_schema_result["result"][key] = MASKED_ATTRIBUTE_VALUE result["result"] = output_schema_result - + # TODO(mierdin): This logic should be moved to the dedicated Inquiry # data model once it exists. if self.runner.get("name") == "inquirer": diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.py b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.py index a6de0dd980..f1677b050e 100644 --- a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.py +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.py @@ -18,5 +18,6 @@ class MyAction(Action): + def run(self): - pass + return {'k1': 'v1', 'k2': 'v2'} diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.yaml b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.yaml index 1efcaf3523..2fa6f6ee1a 100644 --- a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.yaml +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.yaml @@ -1,9 +1,16 @@ --- -"name": "local" -"runner_type": "local-shell-script" -"description": "Action that executes an arbitrary Linux command on the localhost." -"enabled": true -"entry_point": "my_action.py" -"parameters": - "sudo": - "immutable": true +name: my_action +runner_type: python-script +description: A simple python action for testing purposes. +enabled: true +entry_point: my_action.py +output_schema: + k1: + type: string + required: true + k2: + type: string + required: true + secret: true + k3: + type: boolean diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_http_action.yaml b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_http_action.yaml new file mode 100644 index 0000000000..dadeb4b59c --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_http_action.yaml @@ -0,0 +1,22 @@ +--- +name: my_http_action +runner_type: http-request +description: A simple mock http action for testing purposes. +enabled: true +entry_point: '' +parameters: + url: + type: string + required: true + immutable: true + default: https://api.stackstorm.com +output_schema: + k1: + type: string + required: true + k2: + type: string + required: true + secret: true + k3: + type: boolean diff --git a/st2tests/st2tests/mocks/liveaction.py b/st2tests/st2tests/mocks/liveaction.py index 2b329e6b25..e5b6fd74e5 100644 --- a/st2tests/st2tests/mocks/liveaction.py +++ b/st2tests/st2tests/mocks/liveaction.py @@ -32,6 +32,10 @@ class MockLiveActionPublisher(object): @classmethod def process(cls, payload): + print() + print("PAYLOAD") + print(payload) + ex_req = scheduling.get_scheduler_entrypoint().process(payload) if ex_req is not None: @@ -47,6 +51,11 @@ def publish_create(cls, payload): @classmethod def publish_state(cls, payload, state): try: + print() + print("PUBLISH_STATE") + print(payload) + print(state) + if isinstance(payload, LiveActionDB): if state == action_constants.LIVEACTION_STATUS_REQUESTED: cls.process(payload) From 38b46a56e80196a02834d8cd07a7804c1c8631ef Mon Sep 17 00:00:00 2001 From: W Chan Date: Sat, 15 May 2021 04:41:35 +0000 Subject: [PATCH 14/40] Minor clean up to codes added for troubleshooting --- st2tests/st2tests/mocks/liveaction.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/st2tests/st2tests/mocks/liveaction.py b/st2tests/st2tests/mocks/liveaction.py index e5b6fd74e5..2b329e6b25 100644 --- a/st2tests/st2tests/mocks/liveaction.py +++ b/st2tests/st2tests/mocks/liveaction.py @@ -32,10 +32,6 @@ class MockLiveActionPublisher(object): @classmethod def process(cls, payload): - print() - print("PAYLOAD") - print(payload) - ex_req = scheduling.get_scheduler_entrypoint().process(payload) if ex_req is not None: @@ -51,11 +47,6 @@ def publish_create(cls, payload): @classmethod def publish_state(cls, payload, state): try: - print() - print("PUBLISH_STATE") - print(payload) - print(state) - if isinstance(payload, LiveActionDB): if state == action_constants.LIVEACTION_STATUS_REQUESTED: cls.process(payload) From 71ff4bfaf4335d3497dbf11b5527b1f9f798477e Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Sat, 15 May 2021 21:36:43 +0530 Subject: [PATCH 15/40] shifted output schema secret masking block --- st2common/st2common/util/output_schema.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/st2common/st2common/util/output_schema.py b/st2common/st2common/util/output_schema.py index 2bde19c3c0..66a9f9f8b6 100644 --- a/st2common/st2common/util/output_schema.py +++ b/st2common/st2common/util/output_schema.py @@ -20,6 +20,7 @@ from st2common.util import schema from st2common.constants import action as action_constants +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE LOG = logging.getLogger(__name__) @@ -51,6 +52,23 @@ def _validate_action(action_schema, result, output_key): schema.validate(final_result, action_schema, cls=schema.get_validator("custom")) +def mask_secret_output(value, output_schema_result): + value = copy.deepcopy(value) + for key in value["action"]["output_schema"]: + if ( + value.get("action", {}) + .get("output_schema", {}) + .get(key) + .get("secret", False) + ): + if output_schema_result.get("result"): + output_schema_result["result"][key] = MASKED_ATTRIBUTE_VALUE + if output_schema_result.get("body"): + output_schema_result["body"][key] = MASKED_ATTRIBUTE_VALUE + + return output_schema_result + + def validate_output(runner_schema, action_schema, result, status, output_key): """Validate output of action with runner and action schema.""" try: From adc1afdc3c4847ac8e9acd2b34894e0690bb5625 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Sat, 15 May 2021 21:45:18 +0530 Subject: [PATCH 16/40] Moved output masking block to output_schema.py --- st2common/st2common/models/db/execution.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/st2common/st2common/models/db/execution.py b/st2common/st2common/models/db/execution.py index ba6a952722..94e62fd063 100644 --- a/st2common/st2common/models/db/execution.py +++ b/st2common/st2common/models/db/execution.py @@ -25,11 +25,11 @@ from st2common.fields import JSONDictEscapedFieldCompatibilityField from st2common.fields import ComplexDateTimeField from st2common.util import date as date_utils +from st2common.util import output_schema from st2common.util.secrets import get_secret_parameters from st2common.util.secrets import mask_inquiry_response from st2common.util.secrets import mask_secret_parameters from st2common.constants.types import ResourceType -from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE __all__ = ["ActionExecutionDB", "ActionExecutionOutputDB"] @@ -157,16 +157,10 @@ def mask_secrets(self, value): output_schema_result = ActionExecutionDB.result.parse_field_value( result["result"] ) - - # accessing parameters marked secret as true in the output_schema - # and masking them for the result in action execution API - if output_schema_result: - for key, spec in six.iteritems(result["action"]["output_schema"]): - if spec.get("secret", False): - # TODO: Change to output_schema_result[output_key][key] = MASKED_ATTRIBUTE_VALUE - output_schema_result["result"][key] = MASKED_ATTRIBUTE_VALUE - - result["result"] = output_schema_result + masked_output_schema_result = output_schema.mask_secret_output( + value, output_schema_result + ) + result["result"] = masked_output_schema_result # TODO(mierdin): This logic should be moved to the dedicated Inquiry # data model once it exists. From 4ef3765037f88ea89e054e1811c7ef394c6419d4 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Sat, 15 May 2021 21:52:16 +0530 Subject: [PATCH 17/40] added unit tests for mask secret output --- .../tests/unit/test_util_output_schema.py | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/st2common/tests/unit/test_util_output_schema.py b/st2common/tests/unit/test_util_output_schema.py index af9570d4fa..9f291a94ce 100644 --- a/st2common/tests/unit/test_util_output_schema.py +++ b/st2common/tests/unit/test_util_output_schema.py @@ -12,7 +12,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from __future__ import absolute_import import copy +import mock import unittest2 from st2common.util import output_schema @@ -21,6 +23,14 @@ LIVEACTION_STATUS_SUCCEEDED, LIVEACTION_STATUS_FAILED, ) +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE +from st2common.exceptions.db import StackStormDBObjectNotFoundError +from st2common.models.db.execution import ActionExecutionDB +from st2common.persistence.execution import ActionExecution +from st2common.transport.publishers import PoolPublisher + +from st2tests import DbTestCase + ACTION_RESULT = { "output": { @@ -58,6 +68,59 @@ "not_a_key_you_have": {"type": "string"}, } +OUTPUT_SCHEMA_RESULT_1 = { + "stdout": "", + "stderr": "", + "result": { + "os_secret_param": "to_be_masked", + }, +} + +OUTPUT_SCHEMA_RESULT_2 = { + "stdout": "", + "stderr": "", + "result": { + "os_non_secret_param": "not_to_be_masked", + }, +} + +OUTPUT_SCHEMA_LIVEACTION_1 = { + "action": "core.ask", + "parameters": {}, +} + +OUTPUT_SCHEMA_LIVEACTION_2 = { + "action": "core.ask", + "parameters": {}, +} + +ACTIONEXECUTIONS = { + "execution_1": { + "action": { + "uid": "action:core:ask", + "output_schema": { + "os_secret_param": {"type": "string", "required": True, "secret": True}, + }, + }, + "status": "succeeded", + "runner": {"name": "inquirer"}, + "liveaction": OUTPUT_SCHEMA_LIVEACTION_1, + "result": OUTPUT_SCHEMA_RESULT_1, + }, + "execution_2": { + "action": { + "uid": "action:core:ask", + "output_schema": { + "os_non_secret_param": {"type": "string", "required": True}, + }, + }, + "status": "succeeded", + "runner": {"name": "inquirer"}, + "liveaction": OUTPUT_SCHEMA_LIVEACTION_2, + "result": OUTPUT_SCHEMA_RESULT_2, + }, +} + OUTPUT_KEY = "output" @@ -117,3 +180,75 @@ def test_invalid_action_schema(self): self.assertIn(expected_result["error"], result["error"]) self.assertEqual(result["message"], expected_result["message"]) self.assertEqual(status, LIVEACTION_STATUS_FAILED) + + + @mock.patch.object(PoolPublisher, "publish", mock.MagicMock()) +class ActionExecutionModelTest(DbTestCase): + def setUp(self): + + self.executions = {} + + for name, execution in ACTIONEXECUTIONS.items(): + + created = ActionExecutionDB() + created.action = execution["action"] + created.status = execution["status"] + created.runner = execution["runner"] + created.liveaction = execution["liveaction"] + created.result = execution["result"] + saved = ActionExecutionModelTest._save_execution(created) + retrieved = ActionExecution.get_by_id(saved.id) + self.assertEqual( + saved.action, retrieved.action, "Same action was not returned." + ) + + self.executions[name] = retrieved + + def tearDown(self): + + for name, execution in self.executions.items(): + ActionExecutionModelTest._delete([execution]) + try: + retrieved = ActionExecution.get_by_id(execution.id) + except StackStormDBObjectNotFoundError: + retrieved = None + self.assertIsNone(retrieved, "managed to retrieve after failure.") + + def test_output_schema_secret_param_masking(self): + """ + Test that the parameter marked secret as true in output schema is masked in the output + result. Here the parameter in output schema is marked secret as true and we are + asserting this is masked in the output result. + """ + + masked = self.executions["execution_1"].mask_secrets( + self.executions["execution_1"].to_serializable_dict() + ) + print("masked: ", masked) + self.assertEqual( + masked["result"]["result"]["os_secret_param"], MASKED_ATTRIBUTE_VALUE + ) + + def test_output_schema_non_secret_param_not_masking(self): + """ + Test that the parameters is marked secret as true in output schema is not masked in + the output result. Here the parameter in output schema is not marked secret as + true and we are asserting this isn't masked in the output result. + """ + + non_masked = self.executions["execution_2"].mask_secrets( + self.executions["execution_2"].to_serializable_dict() + ) + print("non_masked: ", non_masked) + self.assertEqual( + non_masked["result"]["result"]["os_non_secret_param"], "not_to_be_masked" + ) + + @staticmethod + def _save_execution(execution): + return ActionExecution.add_or_update(execution) + + @staticmethod + def _delete(model_objects): + for model_object in model_objects: + model_object.delete() From 67461d93841a8b1ce9c444243fa17bb6c401b4b6 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Sat, 15 May 2021 22:01:28 +0530 Subject: [PATCH 18/40] adding new file --- .../dummy_pack_1/actions/my_py_action.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.py diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.py b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.py new file mode 100644 index 0000000000..8e3e15e463 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.py @@ -0,0 +1,25 @@ +# Copyright 2020 The StackStorm Authors. +# Copyright 2019 Extreme Networks, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import absolute_import +from st2common.runners.base_action import Action + + +class MyAction(Action): + def run(self): + k1 = "xyz" + k2 = "abc" + k3 = True + return {"k1": k1, "k2": k2, "k3": k3} From 2006d26282fb18e8fc4d8da98c0b615f56ee1c5b Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Sat, 15 May 2021 22:02:56 +0530 Subject: [PATCH 19/40] adding new file --- .../packs/dummy_pack_1/actions/my_py_action.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.yaml diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.yaml b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.yaml new file mode 100644 index 0000000000..87553c9a76 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_py_action.yaml @@ -0,0 +1,16 @@ +--- +name: my_py_action +runner_type: "python-script" +description: A simple python action for testing purposes. +enabled: true +entry_point: my_py_action.py +output_schema: + k1: + type: string + required: true + k2: + type: string + required: true + secret: true + k3: + type: boolean From d9a1f8488876885911e1a22a3ec42869c3fd2fb1 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Sat, 15 May 2021 22:08:31 +0530 Subject: [PATCH 20/40] updating my_action.yaml --- .../packs/dummy_pack_1/actions/my_action.yaml | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.yaml b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.yaml index 2fa6f6ee1a..1efcaf3523 100644 --- a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.yaml +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.yaml @@ -1,16 +1,9 @@ --- -name: my_action -runner_type: python-script -description: A simple python action for testing purposes. -enabled: true -entry_point: my_action.py -output_schema: - k1: - type: string - required: true - k2: - type: string - required: true - secret: true - k3: - type: boolean +"name": "local" +"runner_type": "local-shell-script" +"description": "Action that executes an arbitrary Linux command on the localhost." +"enabled": true +"entry_point": "my_action.py" +"parameters": + "sudo": + "immutable": true From 83d321469313a153b25915f89b06eb117e7d67d6 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Sat, 15 May 2021 22:13:13 +0530 Subject: [PATCH 21/40] updating test_output_schema.py --- st2actions/tests/unit/test_output_schema.py | 74 ++++++++++++++------- 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/st2actions/tests/unit/test_output_schema.py b/st2actions/tests/unit/test_output_schema.py index ac81c0a1bb..14d3f5961f 100644 --- a/st2actions/tests/unit/test_output_schema.py +++ b/st2actions/tests/unit/test_output_schema.py @@ -38,27 +38,37 @@ TEST_PACK = "dummy_pack_1" -TEST_PACK_PATH = st2tests.fixturesloader.get_fixtures_packs_base_path() + "/" + TEST_PACK +TEST_PACK_PATH = ( + st2tests.fixturesloader.get_fixtures_packs_base_path() + "/" + TEST_PACK +) PACKS = [ TEST_PACK_PATH, ] MOCK_PYTHON_ACTION_RESULT = { - 'stderr': '', - 'stdout': '', - 'result': {'k1': 'foobar', 'k2': 'shhhh!'}, - 'exit_code': 0, + "stderr": "", + "stdout": "", + "result": {"k1": "foobar", "k2": "shhhh!"}, + "exit_code": 0, } -MOCK_PYTHON_RUNNER_OUTPUT = (ac_const.LIVEACTION_STATUS_SUCCEEDED, MOCK_PYTHON_ACTION_RESULT, None) +MOCK_PYTHON_RUNNER_OUTPUT = ( + ac_const.LIVEACTION_STATUS_SUCCEEDED, + MOCK_PYTHON_ACTION_RESULT, + None, +) MOCK_HTTP_ACTION_RESULT = { - 'status_code': 200, - 'body': {'k1': 'foobar', 'k2': 'shhhh!'}, + "status_code": 200, + "body": {"k1": "foobar", "k2": "shhhh!"}, } -MOCK_HTTP_RUNNER_OUTPUT = (ac_const.LIVEACTION_STATUS_SUCCEEDED, MOCK_HTTP_ACTION_RESULT, None) +MOCK_HTTP_RUNNER_OUTPUT = ( + ac_const.LIVEACTION_STATUS_SUCCEEDED, + MOCK_HTTP_ACTION_RESULT, + None, +) @mock.patch.object( @@ -91,39 +101,55 @@ def setUpClass(cls): actions_registrar.register_from_pack(pack) @mock.patch.object( - python_runner.PythonRunner, "run", + python_runner.PythonRunner, + "run", mock.MagicMock(return_value=MOCK_PYTHON_RUNNER_OUTPUT), ) def test_python_action(self): # Execute a python action with output schema and secret - lv_ac_db = lv_db_models.LiveActionDB(action="dummy_pack_1.my_action") + lv_ac_db = lv_db_models.LiveActionDB(action="dummy_pack_1.my_py_action") lv_ac_db, ac_ex_db = action_service.request(lv_ac_db) - ac_ex_db = self._wait_on_ac_ex_status(ac_ex_db, ac_const.LIVEACTION_STATUS_SUCCEEDED) + ac_ex_db = self._wait_on_ac_ex_status( + ac_ex_db, ac_const.LIVEACTION_STATUS_SUCCEEDED + ) # Assert expected output written to the database - expected_output = {'k1': 'foobar', 'k2': 'shhhh!'} - self.assertDictEqual(ac_ex_db.result['result'], expected_output) + expected_output = {"k1": "foobar", "k2": "shhhh!"} + self.assertDictEqual(ac_ex_db.result["result"], expected_output) # Assert expected output on conversion to API model - ac_ex_api = ex_api_models.ActionExecutionAPI.from_model(ac_ex_db, mask_secrets=True) - expected_masked_output = {'k1': 'foobar', 'k2': secrets_const.MASKED_ATTRIBUTE_VALUE} - self.assertDictEqual(ac_ex_api.result['result'], expected_masked_output) + ac_ex_api = ex_api_models.ActionExecutionAPI.from_model( + ac_ex_db, mask_secrets=True + ) + expected_masked_output = { + "k1": "foobar", + "k2": secrets_const.MASKED_ATTRIBUTE_VALUE, + } + self.assertDictEqual(ac_ex_api.result["result"], expected_masked_output) @mock.patch.object( - http_runner.HttpRunner, "run", + http_runner.HttpRunner, + "run", mock.MagicMock(return_value=MOCK_HTTP_RUNNER_OUTPUT), ) def test_http_action(self): # Execute a http action with output schema and secret lv_ac_db = lv_db_models.LiveActionDB(action="dummy_pack_1.my_http_action") lv_ac_db, ac_ex_db = action_service.request(lv_ac_db) - ac_ex_db = self._wait_on_ac_ex_status(ac_ex_db, ac_const.LIVEACTION_STATUS_SUCCEEDED) + ac_ex_db = self._wait_on_ac_ex_status( + ac_ex_db, ac_const.LIVEACTION_STATUS_SUCCEEDED + ) # Assert expected output written to the database - expected_output = {'k1': 'foobar', 'k2': 'shhhh!'} - self.assertDictEqual(ac_ex_db.result['body'], expected_output) + expected_output = {"k1": "foobar", "k2": "shhhh!"} + self.assertDictEqual(ac_ex_db.result["body"], expected_output) # Assert expected output on conversion to API model - ac_ex_api = ex_api_models.ActionExecutionAPI.from_model(ac_ex_db, mask_secrets=True) - expected_masked_output = {'k1': 'foobar', 'k2': secrets_const.MASKED_ATTRIBUTE_VALUE} - self.assertDictEqual(ac_ex_api.result['body'], expected_masked_output) + ac_ex_api = ex_api_models.ActionExecutionAPI.from_model( + ac_ex_db, mask_secrets=True + ) + expected_masked_output = { + "k1": "foobar", + "k2": secrets_const.MASKED_ATTRIBUTE_VALUE, + } + self.assertDictEqual(ac_ex_api.result["body"], expected_masked_output) From 217b5b24ec0380cc96b9e9f5faaed8d3afbf578b Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Sat, 15 May 2021 22:23:36 +0530 Subject: [PATCH 22/40] updating dummy_pack_1/my_action.py --- .../st2tests/fixtures/packs/dummy_pack_1/actions/my_action.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.py b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.py index f1677b050e..a6de0dd980 100644 --- a/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.py +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/my_action.py @@ -18,6 +18,5 @@ class MyAction(Action): - def run(self): - return {'k1': 'v1', 'k2': 'v2'} + pass From fd4399cde5c1437269899f7722511926749b2c07 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 17 May 2021 21:55:02 +0530 Subject: [PATCH 23/40] updating output_schema.py for using output_key --- st2common/st2common/util/output_schema.py | 27 ++++++++++++++--------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/st2common/st2common/util/output_schema.py b/st2common/st2common/util/output_schema.py index 66a9f9f8b6..487dc8b269 100644 --- a/st2common/st2common/util/output_schema.py +++ b/st2common/st2common/util/output_schema.py @@ -54,17 +54,22 @@ def _validate_action(action_schema, result, output_key): def mask_secret_output(value, output_schema_result): value = copy.deepcopy(value) - for key in value["action"]["output_schema"]: - if ( - value.get("action", {}) - .get("output_schema", {}) - .get(key) - .get("secret", False) - ): - if output_schema_result.get("result"): - output_schema_result["result"][key] = MASKED_ATTRIBUTE_VALUE - if output_schema_result.get("body"): - output_schema_result["body"][key] = MASKED_ATTRIBUTE_VALUE + if ( + value["action"]["runner_type"] != "local-shell-cmd" + and value["action"]["runner_type"] != "orquesta" + and value["action"]["runner_type"] != "announcement" + and value["action"]["runner_type"] != "inquirer" + and value["action"]["runner_type"] != "noop" + and value["action"]["runner_type"] != "remote-shell-script" + and value["action"]["runner_type"] != "winrm-cmd" + and value["action"]["runner_type"] != "winrm-ps-cmd" + and value["action"]["runner_type"] != "winrm-ps-script" + ): + output_key = value["runner"]["output_key"] + for key, spec in six.iteritems(value["action"]["output_schema"]): + if spec.get("secret", False): + if output_schema_result.get(output_key): + output_schema_result[output_key][key] = MASKED_ATTRIBUTE_VALUE return output_schema_result From 3a97ad15f3ce67cd4672c1741f2b85c425e8ac56 Mon Sep 17 00:00:00 2001 From: W Chan Date: Mon, 17 May 2021 19:22:51 +0000 Subject: [PATCH 24/40] Refactor mask_secret_output function to be more robust The current mask_secret_output function in the output_schema util module is too fragile with the multiple if else on checking runner type to determine if there is an output schema defined. For example, orquesta runner has an output_schema and output_key defined but then the if else already introduced a bug. The function is refactored to evaluate if output key and output schema is defined and then proceed to masking secret value if an output parameter is marked as secret in the output schema. Additional unit test is added to cover orquesta runner. The unit tests for the mask_secret_output function is also simplified. --- st2actions/tests/unit/test_output_schema.py | 46 +++- .../tests/unit/controllers/v1/test_packs.py | 6 +- st2common/st2common/models/db/execution.py | 11 +- st2common/st2common/util/output_schema.py | 35 ++- st2common/tests/unit/test_db_execution.py | 10 +- .../tests/unit/test_util_output_schema.py | 220 +++++++----------- .../orquesta_tests/actions/data-flow.yaml | 8 + 7 files changed, 151 insertions(+), 185 deletions(-) diff --git a/st2actions/tests/unit/test_output_schema.py b/st2actions/tests/unit/test_output_schema.py index 14d3f5961f..ab5ce72244 100644 --- a/st2actions/tests/unit/test_output_schema.py +++ b/st2actions/tests/unit/test_output_schema.py @@ -18,6 +18,7 @@ from http_runner import http_runner from python_runner import python_runner +from orquesta_runner import orquesta_runner import st2tests @@ -37,13 +38,9 @@ from st2tests.mocks import liveaction as mock_lv_ac_xport -TEST_PACK = "dummy_pack_1" -TEST_PACK_PATH = ( - st2tests.fixturesloader.get_fixtures_packs_base_path() + "/" + TEST_PACK -) - PACKS = [ - TEST_PACK_PATH, + st2tests.fixturesloader.get_fixtures_packs_base_path() + "/dummy_pack_1", + st2tests.fixturesloader.get_fixtures_packs_base_path() + "/orquesta_tests", ] MOCK_PYTHON_ACTION_RESULT = { @@ -70,6 +67,14 @@ None, ) +MOCK_ORQUESTA_ACTION_RESULT = {"errors": [], "output": {"a5": "foobar", "b5": "shhhh!"}} + +MOCK_ORQUESTA_RUNNER_OUTPUT = ( + ac_const.LIVEACTION_STATUS_SUCCEEDED, + MOCK_ORQUESTA_ACTION_RESULT, + None, +) + @mock.patch.object( publishers.CUDPublisher, "publish_update", mock.MagicMock(return_value=None) @@ -153,3 +158,32 @@ def test_http_action(self): "k2": secrets_const.MASKED_ATTRIBUTE_VALUE, } self.assertDictEqual(ac_ex_api.result["body"], expected_masked_output) + + @mock.patch.object( + orquesta_runner.OrquestaRunner, + "run", + mock.MagicMock(return_value=MOCK_ORQUESTA_RUNNER_OUTPUT), + ) + def test_orquesta_action(self): + # Execute an orquesta action with output schema and secret + lv_ac_db = lv_db_models.LiveActionDB( + action="orquesta_tests.data-flow", parameters={"a1": "foobar"} + ) + lv_ac_db, ac_ex_db = action_service.request(lv_ac_db) + ac_ex_db = self._wait_on_ac_ex_status( + ac_ex_db, ac_const.LIVEACTION_STATUS_SUCCEEDED + ) + + # Assert expected output written to the database + expected_output = {"a5": "foobar", "b5": "shhhh!"} + self.assertDictEqual(ac_ex_db.result["output"], expected_output) + + # Assert expected output on conversion to API model + ac_ex_api = ex_api_models.ActionExecutionAPI.from_model( + ac_ex_db, mask_secrets=True + ) + expected_masked_output = { + "a5": "foobar", + "b5": secrets_const.MASKED_ATTRIBUTE_VALUE, + } + self.assertDictEqual(ac_ex_api.result["output"], expected_masked_output) diff --git a/st2api/tests/unit/controllers/v1/test_packs.py b/st2api/tests/unit/controllers/v1/test_packs.py index 111101eb65..0af29407cd 100644 --- a/st2api/tests/unit/controllers/v1/test_packs.py +++ b/st2api/tests/unit/controllers/v1/test_packs.py @@ -616,7 +616,7 @@ def test_packs_register_endpoint(self, mock_get_packs): self.assertEqual(resp.status_int, 200) # 13 real plus 1 mock runner - self.assertEqual(resp.json, {"actions": 1, "runners": 14}) + self.assertEqual(resp.json, {"actions": 3, "runners": 14}) # Verify that plural name form also works resp = self.app.post_json( @@ -625,7 +625,7 @@ def test_packs_register_endpoint(self, mock_get_packs): self.assertEqual(resp.status_int, 200) # 13 real plus 1 mock runner - self.assertEqual(resp.json, {"actions": 1, "runners": 14}) + self.assertEqual(resp.json, {"actions": 3, "runners": 14}) # Register single resource from a single pack specified multiple times - verify that # resources from the same pack are only registered once @@ -640,7 +640,7 @@ def test_packs_register_endpoint(self, mock_get_packs): self.assertEqual(resp.status_int, 200) # 13 real plus 1 mock runner - self.assertEqual(resp.json, {"actions": 1, "runners": 14}) + self.assertEqual(resp.json, {"actions": 3, "runners": 14}) # Register resources from a single (non-existent pack) resp = self.app.post_json( diff --git a/st2common/st2common/models/db/execution.py b/st2common/st2common/models/db/execution.py index 94e62fd063..1f74661302 100644 --- a/st2common/st2common/models/db/execution.py +++ b/st2common/st2common/models/db/execution.py @@ -16,7 +16,6 @@ from __future__ import absolute_import import copy -import six import mongoengine as me @@ -154,13 +153,9 @@ def mask_secrets(self, value): }, ) - output_schema_result = ActionExecutionDB.result.parse_field_value( - result["result"] - ) - masked_output_schema_result = output_schema.mask_secret_output( - value, output_schema_result - ) - result["result"] = masked_output_schema_result + output_value = ActionExecutionDB.result.parse_field_value(result["result"]) + masked_output_value = output_schema.mask_secret_output(result, output_value) + result["result"] = masked_output_value # TODO(mierdin): This logic should be moved to the dedicated Inquiry # data model once it exists. diff --git a/st2common/st2common/util/output_schema.py b/st2common/st2common/util/output_schema.py index 487dc8b269..0a77a95fc4 100644 --- a/st2common/st2common/util/output_schema.py +++ b/st2common/st2common/util/output_schema.py @@ -12,9 +12,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import sys -import logging +import logging +import sys import traceback import jsonschema @@ -52,26 +52,17 @@ def _validate_action(action_schema, result, output_key): schema.validate(final_result, action_schema, cls=schema.get_validator("custom")) -def mask_secret_output(value, output_schema_result): - value = copy.deepcopy(value) - if ( - value["action"]["runner_type"] != "local-shell-cmd" - and value["action"]["runner_type"] != "orquesta" - and value["action"]["runner_type"] != "announcement" - and value["action"]["runner_type"] != "inquirer" - and value["action"]["runner_type"] != "noop" - and value["action"]["runner_type"] != "remote-shell-script" - and value["action"]["runner_type"] != "winrm-cmd" - and value["action"]["runner_type"] != "winrm-ps-cmd" - and value["action"]["runner_type"] != "winrm-ps-script" - ): - output_key = value["runner"]["output_key"] - for key, spec in six.iteritems(value["action"]["output_schema"]): - if spec.get("secret", False): - if output_schema_result.get(output_key): - output_schema_result[output_key][key] = MASKED_ATTRIBUTE_VALUE - - return output_schema_result +def mask_secret_output(ac_ex, output_value): + output_key = ac_ex["runner"].get("output_key") + output_schema = ac_ex["action"].get("output_schema") + + if output_key and output_schema: + if output_key in output_value: + for key, spec in output_schema.items(): + if key in output_value[output_key] and spec.get("secret", False): + output_value[output_key][key] = MASKED_ATTRIBUTE_VALUE + + return output_value def validate_output(runner_schema, action_schema, result, status, output_key): diff --git a/st2common/tests/unit/test_db_execution.py b/st2common/tests/unit/test_db_execution.py index 80c0124f5e..84f40ce813 100644 --- a/st2common/tests/unit/test_db_execution.py +++ b/st2common/tests/unit/test_db_execution.py @@ -110,7 +110,7 @@ }, }, "status": "succeeded", - "runner": {"name": "inquirer"}, + "runner": {"name": "inquirer", "output_key": "result"}, "liveaction": OUTPUT_SCHEMA_LIVEACTION, "result": OUTPUT_SCHEMA_RESULT, }, @@ -194,11 +194,11 @@ def test_execution_inquiry_response_action(self): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) def test_output_schema_secret_param_masking(self): - """ - Test that the parameter marked secret as true in output schema is masked in the output result + """Test that the output marked as secret in the output schema is masked in the output result - here, out of two parameters in output schema is marked secret as true and we are asserting only - that is masked in the output result. + In this test case, one of the output parameters is marked as secret in the output schema + while the other output parameter is not marked as secret. The value of the first output + parameter should be masked in the output result. """ masked = self.executions["execution_3"].mask_secrets( diff --git a/st2common/tests/unit/test_util_output_schema.py b/st2common/tests/unit/test_util_output_schema.py index 9f291a94ce..a3dd6d028e 100644 --- a/st2common/tests/unit/test_util_output_schema.py +++ b/st2common/tests/unit/test_util_output_schema.py @@ -12,9 +12,8 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from __future__ import absolute_import + import copy -import mock import unittest2 from st2common.util import output_schema @@ -23,33 +22,29 @@ LIVEACTION_STATUS_SUCCEEDED, LIVEACTION_STATUS_FAILED, ) -from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE -from st2common.exceptions.db import StackStormDBObjectNotFoundError -from st2common.models.db.execution import ActionExecutionDB -from st2common.persistence.execution import ActionExecution -from st2common.transport.publishers import PoolPublisher - -from st2tests import DbTestCase +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE ACTION_RESULT = { "output": { "output_1": "Bobby", "output_2": 5, + "output_3": "shhh!", "deep_output": { "deep_item_1": "Jindal", }, } } -RUNNER_SCHEMA = { +RUNNER_OUTPUT_SCHEMA = { "output": {"type": "object"}, "error": {"type": "array"}, } -ACTION_SCHEMA = { +ACTION_OUTPUT_SCHEMA = { "output_1": {"type": "string"}, "output_2": {"type": "integer"}, + "output_3": {"type": "string"}, "deep_output": { "type": "object", "parameters": { @@ -60,75 +55,36 @@ }, } -RUNNER_SCHEMA_FAIL = { +RUNNER_OUTPUT_SCHEMA_FAIL = { "not_a_key_you_have": {"type": "string"}, } -ACTION_SCHEMA_FAIL = { +ACTION_OUTPUT_SCHEMA_FAIL = { "not_a_key_you_have": {"type": "string"}, } -OUTPUT_SCHEMA_RESULT_1 = { - "stdout": "", - "stderr": "", - "result": { - "os_secret_param": "to_be_masked", - }, -} - -OUTPUT_SCHEMA_RESULT_2 = { - "stdout": "", - "stderr": "", - "result": { - "os_non_secret_param": "not_to_be_masked", - }, -} - -OUTPUT_SCHEMA_LIVEACTION_1 = { - "action": "core.ask", - "parameters": {}, -} - -OUTPUT_SCHEMA_LIVEACTION_2 = { - "action": "core.ask", - "parameters": {}, -} +OUTPUT_KEY = "output" -ACTIONEXECUTIONS = { - "execution_1": { - "action": { - "uid": "action:core:ask", - "output_schema": { - "os_secret_param": {"type": "string", "required": True, "secret": True}, - }, - }, - "status": "succeeded", - "runner": {"name": "inquirer"}, - "liveaction": OUTPUT_SCHEMA_LIVEACTION_1, - "result": OUTPUT_SCHEMA_RESULT_1, - }, - "execution_2": { - "action": { - "uid": "action:core:ask", - "output_schema": { - "os_non_secret_param": {"type": "string", "required": True}, +ACTION_OUTPUT_SCHEMA_WITH_SECRET = { + "output_1": {"type": "string"}, + "output_2": {"type": "integer"}, + "output_3": {"type": "string", "secret": True}, + "deep_output": { + "type": "object", + "parameters": { + "deep_item_1": { + "type": "string", }, }, - "status": "succeeded", - "runner": {"name": "inquirer"}, - "liveaction": OUTPUT_SCHEMA_LIVEACTION_2, - "result": OUTPUT_SCHEMA_RESULT_2, }, } -OUTPUT_KEY = "output" - class OutputSchemaTestCase(unittest2.TestCase): def test_valid_schema(self): result, status = output_schema.validate_output( - copy.deepcopy(RUNNER_SCHEMA), - copy.deepcopy(ACTION_SCHEMA), + copy.deepcopy(RUNNER_OUTPUT_SCHEMA), + copy.deepcopy(ACTION_OUTPUT_SCHEMA), copy.deepcopy(ACTION_RESULT), LIVEACTION_STATUS_SUCCEEDED, OUTPUT_KEY, @@ -139,8 +95,8 @@ def test_valid_schema(self): def test_invalid_runner_schema(self): result, status = output_schema.validate_output( - copy.deepcopy(RUNNER_SCHEMA_FAIL), - copy.deepcopy(ACTION_SCHEMA), + copy.deepcopy(RUNNER_OUTPUT_SCHEMA_FAIL), + copy.deepcopy(ACTION_OUTPUT_SCHEMA), copy.deepcopy(ACTION_RESULT), LIVEACTION_STATUS_SUCCEEDED, OUTPUT_KEY, @@ -148,12 +104,12 @@ def test_invalid_runner_schema(self): expected_result = { "error": ( - "Additional properties are not allowed ('output' was unexpected)" - "\n\nFailed validating 'additionalProperties' in schema:\n {'addi" - "tionalProperties': False,\n 'properties': {'not_a_key_you_have': " - "{'type': 'string'}},\n 'type': 'object'}\n\nOn instance:\n {'" - "output': {'deep_output': {'deep_item_1': 'Jindal'},\n " - "'output_1': 'Bobby',\n 'output_2': 5}}" + "Additional properties are not allowed ('output' was unexpected)\n\n" + "Failed validating 'additionalProperties' in schema:\n " + "{'additionalProperties': False,\n 'properties': {'not_a_key_you_have': " + "{'type': 'string'}},\n 'type': 'object'}\n\nOn instance:\n {'output': " + "{'deep_output': {'deep_item_1': 'Jindal'},\n 'output_1': 'Bobby'," + "\n 'output_2': 5,\n 'output_3': 'shhh!'}}" ), "message": "Error validating output. See error output for more details.", } @@ -163,8 +119,8 @@ def test_invalid_runner_schema(self): def test_invalid_action_schema(self): result, status = output_schema.validate_output( - copy.deepcopy(RUNNER_SCHEMA), - copy.deepcopy(ACTION_SCHEMA_FAIL), + copy.deepcopy(RUNNER_OUTPUT_SCHEMA), + copy.deepcopy(ACTION_OUTPUT_SCHEMA_FAIL), copy.deepcopy(ACTION_RESULT), LIVEACTION_STATUS_SUCCEEDED, OUTPUT_KEY, @@ -180,75 +136,57 @@ def test_invalid_action_schema(self): self.assertIn(expected_result["error"], result["error"]) self.assertEqual(result["message"], expected_result["message"]) self.assertEqual(status, LIVEACTION_STATUS_FAILED) - - - @mock.patch.object(PoolPublisher, "publish", mock.MagicMock()) -class ActionExecutionModelTest(DbTestCase): - def setUp(self): - - self.executions = {} - - for name, execution in ACTIONEXECUTIONS.items(): - - created = ActionExecutionDB() - created.action = execution["action"] - created.status = execution["status"] - created.runner = execution["runner"] - created.liveaction = execution["liveaction"] - created.result = execution["result"] - saved = ActionExecutionModelTest._save_execution(created) - retrieved = ActionExecution.get_by_id(saved.id) - self.assertEqual( - saved.action, retrieved.action, "Same action was not returned." - ) - - self.executions[name] = retrieved - - def tearDown(self): - - for name, execution in self.executions.items(): - ActionExecutionModelTest._delete([execution]) - try: - retrieved = ActionExecution.get_by_id(execution.id) - except StackStormDBObjectNotFoundError: - retrieved = None - self.assertIsNone(retrieved, "managed to retrieve after failure.") - - def test_output_schema_secret_param_masking(self): - """ - Test that the parameter marked secret as true in output schema is masked in the output - result. Here the parameter in output schema is marked secret as true and we are - asserting this is masked in the output result. - """ - - masked = self.executions["execution_1"].mask_secrets( - self.executions["execution_1"].to_serializable_dict() - ) - print("masked: ", masked) - self.assertEqual( - masked["result"]["result"]["os_secret_param"], MASKED_ATTRIBUTE_VALUE - ) - def test_output_schema_non_secret_param_not_masking(self): - """ - Test that the parameters is marked secret as true in output schema is not masked in - the output result. Here the parameter in output schema is not marked secret as - true and we are asserting this isn't masked in the output result. - """ + def test_mask_secret_output(self): + ac_ex = { + "action": { + "output_schema": ACTION_OUTPUT_SCHEMA_WITH_SECRET, + }, + "runner": { + "output_key": OUTPUT_KEY, + "output_schema": RUNNER_OUTPUT_SCHEMA, + }, + } + + expected_masked_output = { + "output": { + "output_1": "Bobby", + "output_2": 5, + "output_3": MASKED_ATTRIBUTE_VALUE, + "deep_output": { + "deep_item_1": "Jindal", + }, + } + } - non_masked = self.executions["execution_2"].mask_secrets( - self.executions["execution_2"].to_serializable_dict() - ) - print("non_masked: ", non_masked) - self.assertEqual( - non_masked["result"]["result"]["os_non_secret_param"], "not_to_be_masked" + masked_output = output_schema.mask_secret_output( + ac_ex, copy.deepcopy(ACTION_RESULT) ) + self.assertDictEqual(masked_output, expected_masked_output) + + def test_mask_secret_output_no_secret(self): + ac_ex = { + "action": { + "output_schema": ACTION_OUTPUT_SCHEMA, + }, + "runner": { + "output_key": OUTPUT_KEY, + "output_schema": RUNNER_OUTPUT_SCHEMA, + }, + } - @staticmethod - def _save_execution(execution): - return ActionExecution.add_or_update(execution) + expected_masked_output = { + "output": { + "output_1": "Bobby", + "output_2": 5, + "output_3": "shhh!", + "deep_output": { + "deep_item_1": "Jindal", + }, + } + } - @staticmethod - def _delete(model_objects): - for model_object in model_objects: - model_object.delete() + masked_output = output_schema.mask_secret_output( + ac_ex, copy.deepcopy(ACTION_RESULT) + ) + self.assertDictEqual(masked_output, expected_masked_output) diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/data-flow.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/data-flow.yaml index 589e966e94..b11dc84ab6 100644 --- a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/data-flow.yaml +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/data-flow.yaml @@ -9,3 +9,11 @@ parameters: a1: required: true type: string +output_schema: + a5: + type: string + required: true + b5: + type: string + required: true + secret: true From 241cdca5f5737ed8e4c9e4c2657cff6d159649da Mon Sep 17 00:00:00 2001 From: W Chan Date: Mon, 17 May 2021 20:15:01 +0000 Subject: [PATCH 25/40] Minor fix to test on number of actions New actions were added to dummy_pack_1. The number of actions have changed in some of the checks in tests. --- st2common/tests/integration/test_register_content_script.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/integration/test_register_content_script.py b/st2common/tests/integration/test_register_content_script.py index 24f1712083..1e06ebc796 100644 --- a/st2common/tests/integration/test_register_content_script.py +++ b/st2common/tests/integration/test_register_content_script.py @@ -52,7 +52,7 @@ def test_register_from_pack_success(self): ] cmd = BASE_REGISTER_ACTIONS_CMD_ARGS + opts exit_code, _, stderr = run_command(cmd=cmd) - self.assertIn("Registered 1 actions.", stderr) + self.assertIn("Registered 3 actions.", stderr) self.assertEqual(exit_code, 0) def test_register_from_pack_fail_on_failure_pack_dir_doesnt_exist(self): From 66afa347dfb8890d7731ec9ae29be88d64397275 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 18 May 2021 21:53:06 +0530 Subject: [PATCH 26/40] adding a workflow unit test for ouptut schema --- .../tests/unit/test_data_flow.py | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py b/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py index d1c0c249ab..b1e8cf0fe1 100644 --- a/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py +++ b/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py @@ -201,3 +201,98 @@ def test_string(self): def test_unicode_string(self): self.assert_data_flow("床前明月光 疑是地上霜 舉頭望明月 低頭思故鄉") + + def test_workflow_for_action_with_output_schema(self, data): + wf_meta = base.get_wf_fixture_meta_data(TEST_PACK_PATH, "secret-params-flow.yaml") + wf_input = {"a1": data} + lv_ac_db = lv_db_models.LiveActionDB( + action=wf_meta["name"], parameters=wf_input + ) + lv_ac_db, ac_ex_db = ac_svc.request(lv_ac_db) + + # Assert action execution is running. + lv_ac_db = lv_db_access.LiveAction.get_by_id(str(lv_ac_db.id)) + self.assertEqual( + lv_ac_db.status, ac_const.LIVEACTION_STATUS_RUNNING, lv_ac_db.result + ) + wf_ex_db = wf_db_access.WorkflowExecution.query( + action_execution=str(ac_ex_db.id) + )[0] + self.assertEqual(wf_ex_db.status, ac_const.LIVEACTION_STATUS_RUNNING) + + # Assert task1 is already completed. + query_filters = {"workflow_execution": str(wf_ex_db.id), "task_id": "task1"} + tk1_ex_db = wf_db_access.TaskExecution.query(**query_filters)[0] + tk1_ac_ex_db = ex_db_access.ActionExecution.query( + task_execution=str(tk1_ex_db.id) + )[0] + tk1_lv_ac_db = lv_db_access.LiveAction.get_by_id(tk1_ac_ex_db.liveaction["id"]) + self.assertEqual(tk1_lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) + + # Manually handle action execution completion. + wf_svc.handle_action_execution_completion(tk1_ac_ex_db) + + # Assert task1 succeeded and workflow is still running. + tk1_ex_db = wf_db_access.TaskExecution.get_by_id(tk1_ex_db.id) + self.assertEqual(tk1_ex_db.status, wf_statuses.SUCCEEDED) + wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) + self.assertEqual(wf_ex_db.status, wf_statuses.RUNNING) + + # Assert task2 is already completed. + query_filters = {"workflow_execution": str(wf_ex_db.id), "task_id": "task2"} + tk2_ex_db = wf_db_access.TaskExecution.query(**query_filters)[0] + tk2_ac_ex_db = ex_db_access.ActionExecution.query( + task_execution=str(tk2_ex_db.id) + )[0] + tk2_lv_ac_db = lv_db_access.LiveAction.get_by_id(tk2_ac_ex_db.liveaction["id"]) + self.assertEqual(tk2_lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) + + # Manually handle action execution completion. + wf_svc.handle_action_execution_completion(tk2_ac_ex_db) + + # Assert task2 succeeded and workflow is still running. + tk2_ex_db = wf_db_access.TaskExecution.get_by_id(tk2_ex_db.id) + self.assertEqual(tk2_ex_db.status, wf_statuses.SUCCEEDED) + wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) + self.assertEqual(wf_ex_db.status, wf_statuses.RUNNING) + + # Assert task3 is already completed. + query_filters = {"workflow_execution": str(wf_ex_db.id), "task_id": "task3"} + tk3_ex_db = wf_db_access.TaskExecution.query(**query_filters)[0] + # tk3_ac_ex_db = ex_db_access.ActionExecution.query( + # task_execution=str(tk3_ex_db.id) + # )[0] + # tk3_lv_ac_db = lv_db_access.LiveAction.get_by_id(tk3_ac_ex_db.liveaction["id"]) + # self.assertEqual(tk3_lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) + + # # Manually handle action execution completion. + # wf_svc.handle_action_execution_completion(tk3_ac_ex_db) + + # # Assert task3 succeeded and workflow is completed. + # tk3_ex_db = wf_db_access.TaskExecution.get_by_id(tk3_ex_db.id) + # self.assertEqual(tk3_ex_db.status, wf_statuses.SUCCEEDED) + # wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) + # self.assertEqual(wf_ex_db.status, wf_statuses.SUCCEEDED) + # lv_ac_db = lv_db_access.LiveAction.get_by_id(str(lv_ac_db.id)) + # self.assertEqual(lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) + # ac_ex_db = ex_db_access.ActionExecution.get_by_id(str(ac_ex_db.id)) + # self.assertEqual(ac_ex_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) + + # # Check workflow output. + # expected_output = { + # "secretParam": wf_input["secretParam"] if six.PY3 else wf_input["secretParam"].decode("utf-8"), + # } + + # self.assertDictEqual(wf_ex_db.output, expected_output) + + # # Check liveaction and action execution result. + # expected_result = {"output": expected_output} + + # self.assertDictEqual(lv_ac_db.result, expected_result) + # self.assertDictEqual(ac_ex_db.result, expected_result) + + def test_string(self): + self.assert_data_flow("xyz") + + def test_unicode_string(self): + self.assert_data_flow("床前明月光 疑是地上霜 舉頭望明月 低頭思故鄉") From 5f99494dce33b33f22dfe0b2847d6a54ce52a9cb Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 18 May 2021 21:56:19 +0530 Subject: [PATCH 27/40] adding python action for workflow test --- .../orquesta_tests/actions/secret-params-flow.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 st2tests/st2tests/fixtures/packs/orquesta_tests/actions/secret-params-flow.yaml diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/secret-params-flow.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/secret-params-flow.yaml new file mode 100644 index 0000000000..5420fd79da --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/secret-params-flow.yaml @@ -0,0 +1,11 @@ +--- +name: secret-params-flow +description: A workflow containing action with output schema. +pack: orquesta_tests +runner_type: orquesta +entry_point: workflows/secret-params-flow.yaml +enabled: true +parameters: + a1: + required: true + type: string From 399a8ab82713853c1bcf23cfec894a05e1321243 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 18 May 2021 21:58:01 +0530 Subject: [PATCH 28/40] adding workflow for unit test for output schema --- .../actions/workflows/secret-params-flow.yaml | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/secret-params-flow.yaml diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/secret-params-flow.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/secret-params-flow.yaml new file mode 100644 index 0000000000..e9ab6e2e53 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/secret-params-flow.yaml @@ -0,0 +1,35 @@ +version: 1.0 + +description: A workflow containing action with output schema. + +input: + - a1 + +vars: + - secretParam: null + +output: + - secretParam: <% ctx().secretParam %> + +tasks: + task1: + action: core.echo + input: + message: <% ctx().a1 %> + next: + - when: <% succeeded() %> + do: task2 + task2: + action: core.echo + input: + message: <% ctx().a1 %> + next: + - when: <% succeeded() %> + do: task3 + task3: + action: core.action_with_output_schema + input: + message: pqr + next: + - when: <% succeeded() %> + publish: secretParam=<% result().result.get(secretParam)%> From ab927623392c64ff8106c75c3ac5abc312793b47 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 18 May 2021 22:00:22 +0530 Subject: [PATCH 29/40] adding action with output schema --- .../actions/action_with_output_schema.yaml | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 contrib/core/actions/action_with_output_schema.yaml diff --git a/contrib/core/actions/action_with_output_schema.yaml b/contrib/core/actions/action_with_output_schema.yaml new file mode 100644 index 0000000000..762c0e9fe5 --- /dev/null +++ b/contrib/core/actions/action_with_output_schema.yaml @@ -0,0 +1,26 @@ +--- +name: action_with_output_schema +runner_type: "python-script" +description: Action that executes the Linux echo command on the localhost. +enabled: true +entry_point: action_with_output_schema.py +pack: core +parameters: + message: + description: The message that the command will echo. + type: string + required: true + default: abc + cmd: + description: Arbitrary Linux command to be executed on the local host. + required: true + type: string + default: 'echo "{{message}}"' + immutable: true +output_schema: + secretParam: + description: secret parameter + type: string + required: true + secret: true + default: SSS From 46f11c6a4921ad7e7a6b9f060a6441b1d0cadcd4 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 18 May 2021 22:01:55 +0530 Subject: [PATCH 30/40] adding python action with output schema --- contrib/core/actions/action_with_output_schema.py | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 contrib/core/actions/action_with_output_schema.py diff --git a/contrib/core/actions/action_with_output_schema.py b/contrib/core/actions/action_with_output_schema.py new file mode 100644 index 0000000000..fd8f4a8857 --- /dev/null +++ b/contrib/core/actions/action_with_output_schema.py @@ -0,0 +1,10 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +from st2common.runners.base_action import Action + + +class EECC(Action): + def run(self, message, cmd): + secretParam = "MMNNPP" + return {"secretParam": secretParam} From add83a039db191f8bc5571154eb42876cd3af5df Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 18 May 2021 22:24:47 +0530 Subject: [PATCH 31/40] updating test_data_flow.py --- .../runners/orquesta_runner/tests/unit/test_data_flow.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py b/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py index b1e8cf0fe1..2cc04624ad 100644 --- a/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py +++ b/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py @@ -291,8 +291,5 @@ def test_workflow_for_action_with_output_schema(self, data): # self.assertDictEqual(lv_ac_db.result, expected_result) # self.assertDictEqual(ac_ex_db.result, expected_result) - def test_string(self): - self.assert_data_flow("xyz") - - def test_unicode_string(self): - self.assert_data_flow("床前明月光 疑是地上霜 舉頭望明月 低頭思故鄉") + def test_string_2(self): + self.test_workflow_for_action_with_output_schema("xyz") From 7739f235e7e1570ecce390028d97539e8caa3b4b Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 19 May 2021 12:29:42 +0530 Subject: [PATCH 32/40] updating test_data_flow.py --- contrib/runners/orquesta_runner/tests/unit/test_data_flow.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py b/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py index 2cc04624ad..43affff7ab 100644 --- a/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py +++ b/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py @@ -202,7 +202,7 @@ def test_string(self): def test_unicode_string(self): self.assert_data_flow("床前明月光 疑是地上霜 舉頭望明月 低頭思故鄉") - def test_workflow_for_action_with_output_schema(self, data): + def workflow_for_action_with_output_schema(self, data): wf_meta = base.get_wf_fixture_meta_data(TEST_PACK_PATH, "secret-params-flow.yaml") wf_input = {"a1": data} lv_ac_db = lv_db_models.LiveActionDB( @@ -292,4 +292,4 @@ def test_workflow_for_action_with_output_schema(self, data): # self.assertDictEqual(ac_ex_db.result, expected_result) def test_string_2(self): - self.test_workflow_for_action_with_output_schema("xyz") + self.workflow_for_action_with_output_schema("xyz") From 024b0386befbb51d05d51ceb3a8698b055d77bd3 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 20 May 2021 21:41:46 +0530 Subject: [PATCH 33/40] adding unit tests for GET API for output schema to test secret parameters masked in GET API --- .../unit/controllers/v1/test_executions.py | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/st2api/tests/unit/controllers/v1/test_executions.py b/st2api/tests/unit/controllers/v1/test_executions.py index 00648c5b5c..5ac2d407b6 100644 --- a/st2api/tests/unit/controllers/v1/test_executions.py +++ b/st2api/tests/unit/controllers/v1/test_executions.py @@ -171,6 +171,34 @@ }, } +ACTION_WITH_OUTPUT_SCHEMA_WITH_SECRET_PARAMS = { + "name": "st2.dummy.action_with_output_schema_secret_param", + "description": "An action that contains output_schema with secret parameters", + "enabled": True, + "entry_point": "/tmp/test/action_with_output_schema_secret_param.py", + "pack": "starterpack", + "runner_type": "python-script", + "parameters": {}, + "output_schema": { + "secret_param_1": {"type": "string", "required": True, "secret": True}, + "secret_param_2": {"type": "string", "required": True, "secret": True}, + }, +} + +ACTION_WITH_OUTPUT_SCHEMA_WITHOUT_SECRET_PARAMS = { + "name": "st2.dummy.action_with_output_schema_without_secret_params", + "description": "An action that contains output_schema without secret parameters", + "enabled": True, + "entry_point": "/tmp/test/action_with_output_schema_without_secret_params.py", + "pack": "starterpack", + "runner_type": "python-script", + "parameters": {}, + "output_schema": { + "non_secret_param_1": {"type": "string", "required": True}, + "non_secret_param_2": {"type": "string", "required": True}, + }, +} + LIVE_ACTION_1 = { "action": "sixpack.st2.dummy.action1", "parameters": { @@ -252,6 +280,16 @@ "action": "sixpack.st2.dummy.action1", } +LIVE_ACTION_WITH_OUTPUT_SCHEMA_SECRET_PARAM = { + "parameters": {}, + "action": "starterpack.st2.dummy.action_with_output_schema_secret_param", +} + +LIVE_ACTION_WITH_OUTPUT_SCHEMA_WITHOUT_SECRET_PARAM = { + "parameters": {}, + "action": "starterpack.st2.dummy.action_with_output_schema_without_secret_params", +} + # Do not add parameters to this. There are tests that will test first without params, # then make a copy with params. LIVE_ACTION_DEFAULT_TEMPLATE = { @@ -326,6 +364,22 @@ def setUpClass(cls): ) post_resp = cls.app.post_json("/v1/actions", cls.action_decrypt_secret_param) cls.action_decrypt_secret_param["id"] = post_resp.json["id"] + + cls.action_with_output_schema_secret_param = copy.deepcopy( + ACTION_WITH_OUTPUT_SCHEMA_WITH_SECRET_PARAMS + ) + post_resp = cls.app.post_json( + "/v1/actions", cls.action_with_output_schema_secret_param + ) + cls.action_with_output_schema_secret_param["id"] = post_resp.json["id"] + + cls.action_with_output_schema_without_secret_params = copy.deepcopy( + ACTION_WITH_OUTPUT_SCHEMA_WITHOUT_SECRET_PARAMS + ) + post_resp = cls.app.post_json( + "/v1/actions", cls.action_with_output_schema_without_secret_params + ) + cls.action_with_output_schema_without_secret_params["id"] = post_resp.json["id"] @classmethod def tearDownClass(cls): @@ -336,6 +390,12 @@ 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_with_output_schema_secret_param["id"] + ) + cls.app.delete( + "/v1/actions/%s" % cls.action_with_output_schema_without_secret_params["id"] + ) super(BaseActionExecutionControllerTestCase, cls).tearDownClass() def test_get_one(self): @@ -1786,6 +1846,80 @@ def test_get_single_include_attributes_and_secret_parameters(self): in resp.json["faultstring"] ) + def test_get_one_with_masked_secrets_in_output_schema(self): + """ + Test that the parameters marked secret as true in output schema are masked in + GET API of action execution. + """ + + post_resp = self._do_post(LIVE_ACTION_WITH_OUTPUT_SCHEMA_SECRET_PARAM) + actionexecution_id = self._get_actionexecution_id(post_resp) + + updates = { + "status": "succeeded", + "result": { + "exit_code": 0, + "stderr": "", + "stdout": "", + "result": { + "secret_param_1": "foo", + "secret_param_2": "bar", + }, + }, + } + + put_resp = self._do_put(actionexecution_id, updates) + self.assertEqual(put_resp.status_int, 200) + get_resp = self._do_get_one(actionexecution_id) + self.assertEqual(get_resp.status_int, 200) + self.assertEqual(self._get_actionexecution_id(get_resp), actionexecution_id) + + expected_result_in_get_resp = { + "secret_param_1": MASKED_ATTRIBUTE_VALUE, + "secret_param_2": MASKED_ATTRIBUTE_VALUE, + } + + self.assertDictEqual( + get_resp.json["result"]["result"], expected_result_in_get_resp + ) + + def test_get_one_without_masked_secrets_in_output_schema(self): + """ + Test that the parameters not marked secret as true in output schema are not masked + in GET API of action execution. + """ + + post_resp = self._do_post(LIVE_ACTION_WITH_OUTPUT_SCHEMA_WITHOUT_SECRET_PARAM) + actionexecution_id = self._get_actionexecution_id(post_resp) + + updates = { + "status": "succeeded", + "result": { + "exit_code": 0, + "stderr": "", + "stdout": "", + "result": { + "non_secret_param_1": "abc", + "non_secret_param_2": "xyz", + }, + }, + } + + put_resp = self._do_put(actionexecution_id, updates) + self.assertEqual(put_resp.status_int, 200) + get_resp = self._do_get_one(actionexecution_id) + self.assertEqual(get_resp.status_int, 200) + self.assertEqual(self._get_actionexecution_id(get_resp), actionexecution_id) + + expected_result_in_get_resp = { + "non_secret_param_1": "abc", + "non_secret_param_2": "xyz", + } + + self.assertDictEqual( + get_resp.json["result"]["result"], expected_result_in_get_resp + ) + def _insert_mock_models(self): execution_1_id = self._get_actionexecution_id(self._do_post(LIVE_ACTION_1)) execution_2_id = self._get_actionexecution_id(self._do_post(LIVE_ACTION_2)) From 595df23412de949039381856b8ef6c469d221fee Mon Sep 17 00:00:00 2001 From: W Chan Date: Thu, 20 May 2021 18:15:14 +0000 Subject: [PATCH 34/40] Refactor the test case for testing secret output in task of workflow Use the existing data flow test to include the test case for secret output in task. Add output schema and secret output to the main workflow as well. --- .../core/actions/action_with_output_schema.py | 10 -- .../actions/action_with_output_schema.yaml | 26 --- .../tests/unit/test_data_flow.py | 164 ++++++++---------- .../unit/controllers/v1/test_executions.py | 2 +- .../orquesta_tests/actions/data-flow.yaml | 10 +- .../actions/py-secret-output.py | 22 +++ .../actions/py-secret-output.yaml | 15 ++ .../actions/workflows/data-flow.yaml | 12 +- 8 files changed, 126 insertions(+), 135 deletions(-) delete mode 100644 contrib/core/actions/action_with_output_schema.py delete mode 100644 contrib/core/actions/action_with_output_schema.yaml create mode 100644 st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.py create mode 100644 st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.yaml diff --git a/contrib/core/actions/action_with_output_schema.py b/contrib/core/actions/action_with_output_schema.py deleted file mode 100644 index fd8f4a8857..0000000000 --- a/contrib/core/actions/action_with_output_schema.py +++ /dev/null @@ -1,10 +0,0 @@ -#!/usr/bin/env python3 -# -*- coding: utf-8 -*- - -from st2common.runners.base_action import Action - - -class EECC(Action): - def run(self, message, cmd): - secretParam = "MMNNPP" - return {"secretParam": secretParam} diff --git a/contrib/core/actions/action_with_output_schema.yaml b/contrib/core/actions/action_with_output_schema.yaml deleted file mode 100644 index 762c0e9fe5..0000000000 --- a/contrib/core/actions/action_with_output_schema.yaml +++ /dev/null @@ -1,26 +0,0 @@ ---- -name: action_with_output_schema -runner_type: "python-script" -description: Action that executes the Linux echo command on the localhost. -enabled: true -entry_point: action_with_output_schema.py -pack: core -parameters: - message: - description: The message that the command will echo. - type: string - required: true - default: abc - cmd: - description: Arbitrary Linux command to be executed on the local host. - required: true - type: string - default: 'echo "{{message}}"' - immutable: true -output_schema: - secretParam: - description: secret parameter - type: string - required: true - secret: true - default: SSS diff --git a/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py b/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py index 43affff7ab..dcf06c8923 100644 --- a/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py +++ b/contrib/runners/orquesta_runner/tests/unit/test_data_flow.py @@ -29,11 +29,14 @@ tests_config.parse_args() +from python_runner import python_runner from tests.unit import base from st2common.bootstrap import actionsregistrar from st2common.bootstrap import runnersregistrar from st2common.constants import action as ac_const +from st2common.constants import secrets as secrets_const +from st2common.models.api import execution as ex_api_models from st2common.models.db import liveaction as lv_db_models from st2common.persistence import execution as ex_db_access from st2common.persistence import liveaction as lv_db_access @@ -58,6 +61,23 @@ st2tests.fixturesloader.get_fixtures_packs_base_path() + "/core", ] +TEST_1 = "xyz" +TEST_2 = "床前明月光 疑是地上霜 舉頭望明月 低頭思故鄉" +MOCK_PY_RESULT_1 = { + "stderr": "", + "stdout": "", + "result": {"k2": TEST_1}, + "exit_code": 0, +} +MOCK_PY_RESULT_2 = { + "stderr": "", + "stdout": "", + "result": {"k2": TEST_2}, + "exit_code": 0, +} +MOCK_PY_OUTPUT_1 = (ac_const.LIVEACTION_STATUS_SUCCEEDED, MOCK_PY_RESULT_1, None) +MOCK_PY_OUTPUT_2 = (ac_const.LIVEACTION_STATUS_SUCCEEDED, MOCK_PY_RESULT_2, None) + @mock.patch.object( publishers.CUDPublisher, "publish_update", mock.MagicMock(return_value=None) @@ -172,10 +192,28 @@ def assert_data_flow(self, data): # Manually handle action execution completion. wf_svc.handle_action_execution_completion(tk3_ac_ex_db) - # Assert task3 succeeded and workflow is completed. + # Assert task3 succeeded and workflow is still running. tk3_ex_db = wf_db_access.TaskExecution.get_by_id(tk3_ex_db.id) self.assertEqual(tk3_ex_db.status, wf_statuses.SUCCEEDED) wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) + self.assertEqual(wf_ex_db.status, wf_statuses.RUNNING) + + # Assert task4 is already completed. + query_filters = {"workflow_execution": str(wf_ex_db.id), "task_id": "task4"} + tk4_ex_db = wf_db_access.TaskExecution.query(**query_filters)[0] + tk4_ac_ex_db = ex_db_access.ActionExecution.query( + task_execution=str(tk4_ex_db.id) + )[0] + tk4_lv_ac_db = lv_db_access.LiveAction.get_by_id(tk4_ac_ex_db.liveaction["id"]) + self.assertEqual(tk4_lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) + + # Manually handle action execution completion. + wf_svc.handle_action_execution_completion(tk4_ac_ex_db) + + # Assert task4 succeeded and workflow is completed. + tk4_ex_db = wf_db_access.TaskExecution.get_by_id(tk4_ex_db.id) + self.assertEqual(tk4_ex_db.status, wf_statuses.SUCCEEDED) + wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) self.assertEqual(wf_ex_db.status, wf_statuses.SUCCEEDED) lv_ac_db = lv_db_access.LiveAction.get_by_id(str(lv_ac_db.id)) self.assertEqual(lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) @@ -183,9 +221,13 @@ def assert_data_flow(self, data): self.assertEqual(ac_ex_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) # Check workflow output. + expected_value = wf_input["a1"] if six.PY3 else wf_input["a1"].decode("utf-8") + expected_output = { - "a5": wf_input["a1"] if six.PY3 else wf_input["a1"].decode("utf-8"), - "b5": wf_input["a1"] if six.PY3 else wf_input["a1"].decode("utf-8"), + "a6": expected_value, + "b6": expected_value, + "a7": expected_value, + "b7": expected_value, } self.assertDictEqual(wf_ex_db.output, expected_output) @@ -196,100 +238,34 @@ def assert_data_flow(self, data): self.assertDictEqual(lv_ac_db.result, expected_result) self.assertDictEqual(ac_ex_db.result, expected_result) - def test_string(self): - self.assert_data_flow("xyz") - - def test_unicode_string(self): - self.assert_data_flow("床前明月光 疑是地上霜 舉頭望明月 低頭思故鄉") - - def workflow_for_action_with_output_schema(self, data): - wf_meta = base.get_wf_fixture_meta_data(TEST_PACK_PATH, "secret-params-flow.yaml") - wf_input = {"a1": data} - lv_ac_db = lv_db_models.LiveActionDB( - action=wf_meta["name"], parameters=wf_input - ) - lv_ac_db, ac_ex_db = ac_svc.request(lv_ac_db) - - # Assert action execution is running. - lv_ac_db = lv_db_access.LiveAction.get_by_id(str(lv_ac_db.id)) - self.assertEqual( - lv_ac_db.status, ac_const.LIVEACTION_STATUS_RUNNING, lv_ac_db.result + # Assert expected output on conversion to API model + ac_ex_api = ex_api_models.ActionExecutionAPI.from_model( + ac_ex_db, mask_secrets=True ) - wf_ex_db = wf_db_access.WorkflowExecution.query( - action_execution=str(ac_ex_db.id) - )[0] - self.assertEqual(wf_ex_db.status, ac_const.LIVEACTION_STATUS_RUNNING) - - # Assert task1 is already completed. - query_filters = {"workflow_execution": str(wf_ex_db.id), "task_id": "task1"} - tk1_ex_db = wf_db_access.TaskExecution.query(**query_filters)[0] - tk1_ac_ex_db = ex_db_access.ActionExecution.query( - task_execution=str(tk1_ex_db.id) - )[0] - tk1_lv_ac_db = lv_db_access.LiveAction.get_by_id(tk1_ac_ex_db.liveaction["id"]) - self.assertEqual(tk1_lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) - - # Manually handle action execution completion. - wf_svc.handle_action_execution_completion(tk1_ac_ex_db) - # Assert task1 succeeded and workflow is still running. - tk1_ex_db = wf_db_access.TaskExecution.get_by_id(tk1_ex_db.id) - self.assertEqual(tk1_ex_db.status, wf_statuses.SUCCEEDED) - wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) - self.assertEqual(wf_ex_db.status, wf_statuses.RUNNING) + expected_masked_output = { + "a6": expected_value, + "b6": expected_value, + "a7": expected_value, + "b7": secrets_const.MASKED_ATTRIBUTE_VALUE, + } - # Assert task2 is already completed. - query_filters = {"workflow_execution": str(wf_ex_db.id), "task_id": "task2"} - tk2_ex_db = wf_db_access.TaskExecution.query(**query_filters)[0] - tk2_ac_ex_db = ex_db_access.ActionExecution.query( - task_execution=str(tk2_ex_db.id) - )[0] - tk2_lv_ac_db = lv_db_access.LiveAction.get_by_id(tk2_ac_ex_db.liveaction["id"]) - self.assertEqual(tk2_lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) + expected_masked_result = {"output": expected_masked_output} - # Manually handle action execution completion. - wf_svc.handle_action_execution_completion(tk2_ac_ex_db) + self.assertDictEqual(ac_ex_api.result, expected_masked_result) - # Assert task2 succeeded and workflow is still running. - tk2_ex_db = wf_db_access.TaskExecution.get_by_id(tk2_ex_db.id) - self.assertEqual(tk2_ex_db.status, wf_statuses.SUCCEEDED) - wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) - self.assertEqual(wf_ex_db.status, wf_statuses.RUNNING) + @mock.patch.object( + python_runner.PythonRunner, + "run", + mock.MagicMock(return_value=MOCK_PY_OUTPUT_1), + ) + def test_string(self): + self.assert_data_flow(TEST_1) - # Assert task3 is already completed. - query_filters = {"workflow_execution": str(wf_ex_db.id), "task_id": "task3"} - tk3_ex_db = wf_db_access.TaskExecution.query(**query_filters)[0] - # tk3_ac_ex_db = ex_db_access.ActionExecution.query( - # task_execution=str(tk3_ex_db.id) - # )[0] - # tk3_lv_ac_db = lv_db_access.LiveAction.get_by_id(tk3_ac_ex_db.liveaction["id"]) - # self.assertEqual(tk3_lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) - - # # Manually handle action execution completion. - # wf_svc.handle_action_execution_completion(tk3_ac_ex_db) - - # # Assert task3 succeeded and workflow is completed. - # tk3_ex_db = wf_db_access.TaskExecution.get_by_id(tk3_ex_db.id) - # self.assertEqual(tk3_ex_db.status, wf_statuses.SUCCEEDED) - # wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) - # self.assertEqual(wf_ex_db.status, wf_statuses.SUCCEEDED) - # lv_ac_db = lv_db_access.LiveAction.get_by_id(str(lv_ac_db.id)) - # self.assertEqual(lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) - # ac_ex_db = ex_db_access.ActionExecution.get_by_id(str(ac_ex_db.id)) - # self.assertEqual(ac_ex_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) - - # # Check workflow output. - # expected_output = { - # "secretParam": wf_input["secretParam"] if six.PY3 else wf_input["secretParam"].decode("utf-8"), - # } - - # self.assertDictEqual(wf_ex_db.output, expected_output) - - # # Check liveaction and action execution result. - # expected_result = {"output": expected_output} - - # self.assertDictEqual(lv_ac_db.result, expected_result) - # self.assertDictEqual(ac_ex_db.result, expected_result) - - def test_string_2(self): - self.workflow_for_action_with_output_schema("xyz") + @mock.patch.object( + python_runner.PythonRunner, + "run", + mock.MagicMock(return_value=MOCK_PY_OUTPUT_2), + ) + def test_unicode_string(self): + self.assert_data_flow(TEST_2) diff --git a/st2api/tests/unit/controllers/v1/test_executions.py b/st2api/tests/unit/controllers/v1/test_executions.py index 5ac2d407b6..5afcdbaeda 100644 --- a/st2api/tests/unit/controllers/v1/test_executions.py +++ b/st2api/tests/unit/controllers/v1/test_executions.py @@ -364,7 +364,7 @@ def setUpClass(cls): ) post_resp = cls.app.post_json("/v1/actions", cls.action_decrypt_secret_param) cls.action_decrypt_secret_param["id"] = post_resp.json["id"] - + cls.action_with_output_schema_secret_param = copy.deepcopy( ACTION_WITH_OUTPUT_SCHEMA_WITH_SECRET_PARAMS ) diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/data-flow.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/data-flow.yaml index b11dc84ab6..85105f435e 100644 --- a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/data-flow.yaml +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/data-flow.yaml @@ -10,10 +10,16 @@ parameters: required: true type: string output_schema: - a5: + a6: type: string required: true - b5: + b6: + type: string + required: true + a7: + type: string + required: true + b7: type: string required: true secret: true diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.py b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.py new file mode 100644 index 0000000000..3007056b91 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.py @@ -0,0 +1,22 @@ +# Copyright 2021 The StackStorm Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import absolute_import + +from st2common.runners.base_action import Action + + +class PyAction(Action): + def run(self, k1): + return {"k2": k1} diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.yaml new file mode 100644 index 0000000000..b6126e8ce1 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/py-secret-output.yaml @@ -0,0 +1,15 @@ +--- +name: py_secret_output +runner_type: "python-script" +description: A simple python action with an output schema for testing purposes. +enabled: true +entry_point: py-secret-output.py +parameters: + k1: + type: string + required: true +output_schema: + k2: + type: string + required: true + secret: true diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/data-flow.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/data-flow.yaml index 7660d02cb5..2aa03c0598 100644 --- a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/data-flow.yaml +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/data-flow.yaml @@ -11,8 +11,10 @@ vars: - b2: <% ctx().a2 %> output: - - a5: <% ctx().b4 %> - - b5: <% ctx().a5 %> + - a6: <% ctx().a4 %> + - b6: <% ctx().a6 %> + - a7: <% ctx().a5 %> + - b7: <% ctx().a7 %> tasks: task1: @@ -32,4 +34,10 @@ tasks: publish: a4=<% result().stdout %> b4=<% ctx().a4 %> do: task3 task3: + action: orquesta_tests.py_secret_output k1=<% ctx().b4 %> + next: + - when: <% succeeded() %> + publish: a5=<% result().result.k2 %> + do: task4 + task4: action: core.noop From 674f3c3e4d039c9e340052dad5a67c700c574301 Mon Sep 17 00:00:00 2001 From: W Chan Date: Thu, 20 May 2021 18:41:29 +0000 Subject: [PATCH 35/40] Minor fix to output schema unit test due to changes in a workflow --- st2actions/tests/unit/test_output_schema.py | 22 ++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/st2actions/tests/unit/test_output_schema.py b/st2actions/tests/unit/test_output_schema.py index ab5ce72244..330aa46860 100644 --- a/st2actions/tests/unit/test_output_schema.py +++ b/st2actions/tests/unit/test_output_schema.py @@ -67,7 +67,10 @@ None, ) -MOCK_ORQUESTA_ACTION_RESULT = {"errors": [], "output": {"a5": "foobar", "b5": "shhhh!"}} +MOCK_ORQUESTA_ACTION_RESULT = { + "errors": [], + "output": {"a6": "foobar", "b6": "foobar", "a7": "foobar", "b7": "shhhh!"}, +} MOCK_ORQUESTA_RUNNER_OUTPUT = ( ac_const.LIVEACTION_STATUS_SUCCEEDED, @@ -165,9 +168,11 @@ def test_http_action(self): mock.MagicMock(return_value=MOCK_ORQUESTA_RUNNER_OUTPUT), ) def test_orquesta_action(self): + wf_input = "foobar" + # Execute an orquesta action with output schema and secret lv_ac_db = lv_db_models.LiveActionDB( - action="orquesta_tests.data-flow", parameters={"a1": "foobar"} + action="orquesta_tests.data-flow", parameters={"a1": wf_input} ) lv_ac_db, ac_ex_db = action_service.request(lv_ac_db) ac_ex_db = self._wait_on_ac_ex_status( @@ -175,7 +180,12 @@ def test_orquesta_action(self): ) # Assert expected output written to the database - expected_output = {"a5": "foobar", "b5": "shhhh!"} + expected_output = { + "a6": wf_input, + "b6": wf_input, + "a7": wf_input, + "b7": "shhhh!", + } self.assertDictEqual(ac_ex_db.result["output"], expected_output) # Assert expected output on conversion to API model @@ -183,7 +193,9 @@ def test_orquesta_action(self): ac_ex_db, mask_secrets=True ) expected_masked_output = { - "a5": "foobar", - "b5": secrets_const.MASKED_ATTRIBUTE_VALUE, + "a6": wf_input, + "b6": wf_input, + "a7": wf_input, + "b7": secrets_const.MASKED_ATTRIBUTE_VALUE, } self.assertDictEqual(ac_ex_api.result["output"], expected_masked_output) From 914a566cb365e5a42417b3dc914ffdb79c4379e3 Mon Sep 17 00:00:00 2001 From: W Chan Date: Thu, 20 May 2021 18:44:16 +0000 Subject: [PATCH 36/40] Add entry to CHANGELOG for masking of secrets in execution output --- CHANGELOG.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5695a96466..fffafcec62 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -78,6 +78,11 @@ Added Contributed by @Kami. +* Mask secrets in output of an action execution in the API if the action has an output schema + defined and one or more output parameters are marked as secret. + + Contributed by @mahesh-orch. + Changed ~~~~~~~ From 386b252087687a58ce12cf12942dafec561d48be Mon Sep 17 00:00:00 2001 From: W Chan Date: Thu, 20 May 2021 18:46:49 +0000 Subject: [PATCH 37/40] Removed unused test fixtures --- .../actions/secret-params-flow.yaml | 11 ------ .../actions/workflows/secret-params-flow.yaml | 35 ------------------- 2 files changed, 46 deletions(-) delete mode 100644 st2tests/st2tests/fixtures/packs/orquesta_tests/actions/secret-params-flow.yaml delete mode 100644 st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/secret-params-flow.yaml diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/secret-params-flow.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/secret-params-flow.yaml deleted file mode 100644 index 5420fd79da..0000000000 --- a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/secret-params-flow.yaml +++ /dev/null @@ -1,11 +0,0 @@ ---- -name: secret-params-flow -description: A workflow containing action with output schema. -pack: orquesta_tests -runner_type: orquesta -entry_point: workflows/secret-params-flow.yaml -enabled: true -parameters: - a1: - required: true - type: string diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/secret-params-flow.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/secret-params-flow.yaml deleted file mode 100644 index e9ab6e2e53..0000000000 --- a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/secret-params-flow.yaml +++ /dev/null @@ -1,35 +0,0 @@ -version: 1.0 - -description: A workflow containing action with output schema. - -input: - - a1 - -vars: - - secretParam: null - -output: - - secretParam: <% ctx().secretParam %> - -tasks: - task1: - action: core.echo - input: - message: <% ctx().a1 %> - next: - - when: <% succeeded() %> - do: task2 - task2: - action: core.echo - input: - message: <% ctx().a1 %> - next: - - when: <% succeeded() %> - do: task3 - task3: - action: core.action_with_output_schema - input: - message: pqr - next: - - when: <% succeeded() %> - publish: secretParam=<% result().result.get(secretParam)%> From b8402b4c730a4d20797d84e584a8ba20e6e2684a Mon Sep 17 00:00:00 2001 From: W Chan Date: Thu, 20 May 2021 22:12:50 +0000 Subject: [PATCH 38/40] Fix masking of secret output for workflows Add the action.output_schema and runner.output_key to attributes to be included in the API query otherwise secrets are not masked in the output of workflow action. Update the output schema util to be more robust in terms of handling result that is either empty, wrong type, or missing output key. --- .../st2api/controllers/v1/actionexecutions.py | 24 +++++++++++ st2common/st2common/util/output_schema.py | 20 ++++++--- .../tests/unit/test_util_output_schema.py | 41 +++++++++++++++++++ 3 files changed, 80 insertions(+), 5 deletions(-) diff --git a/st2api/st2api/controllers/v1/actionexecutions.py b/st2api/st2api/controllers/v1/actionexecutions.py index f471889fb7..3644ca7cf4 100644 --- a/st2api/st2api/controllers/v1/actionexecutions.py +++ b/st2api/st2api/controllers/v1/actionexecutions.py @@ -86,7 +86,9 @@ class ActionExecutionsControllerMixin(BaseRestControllerMixin): # parameters mandatory_include_fields_retrieve = [ "action.parameters", + "action.output_schema", "runner.runner_parameters", + "runner.output_key", "parameters", # Attributes below are mandatory for RBAC installations "action.pack", @@ -276,6 +278,7 @@ def _get_children( ) mask_secrets = self._get_mask_secrets(requester_user, show_secrets=show_secrets) + return [ self.model.from_model(descendant, mask_secrets=mask_secrets) for descendant in descendants @@ -309,9 +312,19 @@ def get_one( :rtype: ``list`` """ + if not requester_user: + requester_user = UserDB(name=cfg.CONF.system_user.user) + + from_model_kwargs = { + "mask_secrets": self._get_mask_secrets( + requester_user, show_secrets=show_secrets + ) + } + execution_db = self._get_one_by_id( id=id, requester_user=requester_user, + from_model_kwargs=from_model_kwargs, permission_type=PermissionType.EXECUTION_VIEW, ) id = str(execution_db.id) @@ -468,6 +481,7 @@ def get_one( output_format="raw", existing_only=False, requester_user=None, + show_secrets=False, ): # Special case for id == "last" if id == "last": @@ -478,9 +492,19 @@ def get_one( id = str(execution_db.id) + if not requester_user: + requester_user = UserDB(name=cfg.CONF.system_user.user) + + from_model_kwargs = { + "mask_secrets": self._get_mask_secrets( + requester_user, show_secrets=False + ) + } + execution_db = self._get_one_by_id( id=id, requester_user=requester_user, + from_model_kwargs=from_model_kwargs, permission_type=PermissionType.EXECUTION_VIEW, ) execution_id = str(execution_db.id) diff --git a/st2common/st2common/util/output_schema.py b/st2common/st2common/util/output_schema.py index 0a77a95fc4..6070fe08a8 100644 --- a/st2common/st2common/util/output_schema.py +++ b/st2common/st2common/util/output_schema.py @@ -53,14 +53,24 @@ def _validate_action(action_schema, result, output_key): def mask_secret_output(ac_ex, output_value): + if not output_value: + return output_value + + if not isinstance(output_value, dict) and not isinstance(output_value, list): + return output_value + output_key = ac_ex["runner"].get("output_key") output_schema = ac_ex["action"].get("output_schema") - if output_key and output_schema: - if output_key in output_value: - for key, spec in output_schema.items(): - if key in output_value[output_key] and spec.get("secret", False): - output_value[output_key][key] = MASKED_ATTRIBUTE_VALUE + if not output_key or not output_schema: + return output_value + + if not output_value.get(output_key): + return output_value + + for key, spec in output_schema.items(): + if key in output_value[output_key] and spec.get("secret", False): + output_value[output_key][key] = MASKED_ATTRIBUTE_VALUE return output_value diff --git a/st2common/tests/unit/test_util_output_schema.py b/st2common/tests/unit/test_util_output_schema.py index a3dd6d028e..64ed13237e 100644 --- a/st2common/tests/unit/test_util_output_schema.py +++ b/st2common/tests/unit/test_util_output_schema.py @@ -190,3 +190,44 @@ def test_mask_secret_output_no_secret(self): ac_ex, copy.deepcopy(ACTION_RESULT) ) self.assertDictEqual(masked_output, expected_masked_output) + + def test_mask_secret_output_noop(self): + ac_ex = { + "action": { + "output_schema": ACTION_OUTPUT_SCHEMA_WITH_SECRET, + }, + "runner": { + "output_key": OUTPUT_KEY, + "output_schema": RUNNER_OUTPUT_SCHEMA, + }, + } + + # The result is type of None. + ac_ex_result = None + expected_masked_output = None + masked_output = output_schema.mask_secret_output(ac_ex, ac_ex_result) + self.assertEqual(masked_output, expected_masked_output) + + # The result is empty. + ac_ex_result = {} + expected_masked_output = {} + masked_output = output_schema.mask_secret_output(ac_ex, ac_ex_result) + self.assertDictEqual(masked_output, expected_masked_output) + + # The output is type of None. + ac_ex_result = {"output": None} + expected_masked_output = {"output": None} + masked_output = output_schema.mask_secret_output(ac_ex, ac_ex_result) + self.assertDictEqual(masked_output, expected_masked_output) + + # The output is not type of dict or list. + ac_ex_result = {"output": "foobar"} + expected_masked_output = {"output": "foobar"} + masked_output = output_schema.mask_secret_output(ac_ex, ac_ex_result) + self.assertDictEqual(masked_output, expected_masked_output) + + # The output key is missing. + ac_ex_result = {"output1": None} + expected_masked_output = {"output1": None} + masked_output = output_schema.mask_secret_output(ac_ex, ac_ex_result) + self.assertDictEqual(masked_output, expected_masked_output) From dc656013781a0f76d1deb51279039400180ba69d Mon Sep 17 00:00:00 2001 From: W Chan Date: Thu, 20 May 2021 22:19:41 +0000 Subject: [PATCH 39/40] Minor fix to show_secrets param in get_one of output controller --- st2api/st2api/controllers/v1/actionexecutions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2api/st2api/controllers/v1/actionexecutions.py b/st2api/st2api/controllers/v1/actionexecutions.py index 3644ca7cf4..cc20c61f52 100644 --- a/st2api/st2api/controllers/v1/actionexecutions.py +++ b/st2api/st2api/controllers/v1/actionexecutions.py @@ -497,7 +497,7 @@ def get_one( from_model_kwargs = { "mask_secrets": self._get_mask_secrets( - requester_user, show_secrets=False + requester_user, show_secrets=show_secrets ) } From 575eae947a6ecb31fbda4bddb16b9cb9818dcd8b Mon Sep 17 00:00:00 2001 From: W Chan Date: Thu, 20 May 2021 22:50:18 +0000 Subject: [PATCH 40/40] Add PR # to the changelog entry --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fffafcec62..3468fd6d0e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -79,7 +79,7 @@ Added Contributed by @Kami. * Mask secrets in output of an action execution in the API if the action has an output schema - defined and one or more output parameters are marked as secret. + defined and one or more output parameters are marked as secret. #5250 Contributed by @mahesh-orch.