From 746f9612750130ca6e723cf747810cbef634567a Mon Sep 17 00:00:00 2001 From: bigmstone Date: Wed, 15 Aug 2018 22:35:23 -0500 Subject: [PATCH 01/22] Action and Runner Output Schema Model --- .../python_runner/python_runner/runner.yaml | 9 + st2common/st2common/models/api/action.py | 28 ++- st2common/st2common/models/db/action.py | 2 + st2common/st2common/models/db/runner.py | 2 + st2common/st2common/util/schema/__init__.py | 13 +- .../util/schema/action_output_schema.json | 160 ++++++++++++++++++ 6 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 st2common/st2common/util/schema/action_output_schema.json diff --git a/contrib/runners/python_runner/python_runner/runner.yaml b/contrib/runners/python_runner/python_runner/runner.yaml index fb8ca03de4..e7fddc2034 100644 --- a/contrib/runners/python_runner/python_runner/runner.yaml +++ b/contrib/runners/python_runner/python_runner/runner.yaml @@ -4,6 +4,15 @@ enabled: true name: python-script runner_module: python_runner + output_schema: + result: + type: object + stderr: + type: string + required: false + stdout: + type: string + required: false runner_parameters: debug: description: Enable runner debug mode. diff --git a/st2common/st2common/models/api/action.py b/st2common/st2common/models/api/action.py index d5ab250e56..60fdcf599c 100644 --- a/st2common/st2common/models/api/action.py +++ b/st2common/st2common/models/api/action.py @@ -109,7 +109,16 @@ class RunnerTypeAPI(BaseAPI): "^\w+$": util_schema.get_action_parameters_schema() }, 'additionalProperties': False - } + }, + "output_schema": { + "description": "Schema for the runner's output.", + "type": "object", + "patternProperties": { + "^\w+$": util_schema.get_action_output_schema() + }, + 'additionalProperties': False, + "default": {} + }, }, "additionalProperties": False } @@ -135,11 +144,12 @@ def to_model(cls, runner_type): runner_package = getattr(runner_type, 'runner_package', runner_type.runner_module) runner_module = str(runner_type.runner_module) runner_parameters = getattr(runner_type, 'runner_parameters', dict()) + output_schema = getattr(runner_type, 'output_schema', dict()) query_module = getattr(runner_type, 'query_module', None) model = cls.model(name=name, description=description, enabled=enabled, runner_package=runner_package, runner_module=runner_module, - runner_parameters=runner_parameters, + runner_parameters=runner_parameters, output_schema=output_schema, query_module=query_module) return model @@ -206,6 +216,15 @@ class ActionAPI(BaseAPI, APIUIDMixin): 'additionalProperties': False, "default": {} }, + "output_schema": { + "description": "Schema for the action's output.", + "type": "object", + "patternProperties": { + "^\w+$": util_schema.get_action_output_schema() + }, + 'additionalProperties': False, + "default": {} + }, "tags": { "description": "User associated metadata assigned to this object.", "type": "array", @@ -253,6 +272,7 @@ def to_model(cls, action): pack = str(action.pack) runner_type = {'name': str(action.runner_type)} parameters = getattr(action, 'parameters', dict()) + output_schema = getattr(action, 'output_schema', dict()) tags = TagsHelper.to_model(getattr(action, 'tags', [])) ref = ResourceReference.to_string_reference(pack=pack, name=name) @@ -267,8 +287,8 @@ def to_model(cls, action): model = cls.model(name=name, description=description, enabled=enabled, entry_point=entry_point, pack=pack, runner_type=runner_type, - tags=tags, parameters=parameters, notify=notify, - ref=ref) + tags=tags, parameters=parameters, output_schema=output_schema, + notify=notify, ref=ref) return model diff --git a/st2common/st2common/models/db/action.py b/st2common/st2common/models/db/action.py index aa219e2cb6..6cb455cd37 100644 --- a/st2common/st2common/models/db/action.py +++ b/st2common/st2common/models/db/action.py @@ -76,6 +76,8 @@ class ActionDB(stormbase.StormFoundationDB, stormbase.TagsMixin, help_text='The action runner to use for executing the action.') parameters = stormbase.EscapedDynamicField( help_text='The specification for parameters for the action.') + output_schema = stormbase.EscapedDynamicField( + help_text='The schema for output of the action.') notify = me.EmbeddedDocumentField(NotificationSchema) meta = { diff --git a/st2common/st2common/models/db/runner.py b/st2common/st2common/models/db/runner.py index 6b48ce624e..9ed74640d9 100644 --- a/st2common/st2common/models/db/runner.py +++ b/st2common/st2common/models/db/runner.py @@ -60,6 +60,8 @@ class RunnerTypeDB(stormbase.StormBaseDB, stormbase.UIDFieldMixin): help_text='The python module that implements the action runner for this type.') runner_parameters = me.DictField( help_text='The specification for parameters for the action runner.') + output_schema = me.DictField( + help_text='The schema for runner output.') query_module = me.StringField( required=False, help_text='The python module that implements the query module for this runner.') diff --git a/st2common/st2common/util/schema/__init__.py b/st2common/st2common/util/schema/__init__.py index b51e1c066c..500078bfec 100644 --- a/st2common/st2common/util/schema/__init__.py +++ b/st2common/st2common/util/schema/__init__.py @@ -53,7 +53,8 @@ 'custom': jsonify.load_file(os.path.join(PATH, 'custom.json')), # Custom schema for action params which doesn't allow parameter "type" attribute to be array - 'action_params': jsonify.load_file(os.path.join(PATH, 'action_params.json')) + 'action_params': jsonify.load_file(os.path.join(PATH, 'action_params.json')), + 'action_output_schema': jsonify.load_file(os.path.join(PATH, 'action_output_schema.json')) } SCHEMA_ANY_TYPE = { @@ -83,6 +84,16 @@ def get_draft_schema(version='custom', additional_properties=False): return schema +def get_action_output_schema(additional_properties=False): + """ + Return a generic schema which is used for validating action output. + """ + return get_draft_schema( + version='action_output_schema', + additional_properties=additional_properties + ) + + def get_action_parameters_schema(additional_properties=False): """ Return a generic schema which is used for validating action parameters definition. diff --git a/st2common/st2common/util/schema/action_output_schema.json b/st2common/st2common/util/schema/action_output_schema.json new file mode 100644 index 0000000000..3e92536cea --- /dev/null +++ b/st2common/st2common/util/schema/action_output_schema.json @@ -0,0 +1,160 @@ +{ + "id": "http://json-schema.org/draft-04/schema#", + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "Core schema meta-schema", + "definitions": { + "schemaArray": { + "type": "array", + "minItems": 1, + "items": { "$ref": "#" } + }, + "positiveInteger": { + "type": "integer", + "minimum": 0 + }, + "positiveIntegerDefault0": { + "allOf": [ { "$ref": "#/definitions/positiveInteger" }, { "default": 0 } ] + }, + "simpleTypes": { + "enum": [ "array", "boolean", "integer", "null", "number", "object", "string" ] + }, + "stringArray": { + "type": "array", + "items": { "type": "string" }, + "minItems": 1, + "uniqueItems": true + } + }, + "type": "object", + "properties": { + "id": { + "type": "string", + "format": "uri" + }, + "$schema": { + "type": "string", + "format": "uri" + }, + "title": { + "type": "string" + }, + "description": { + "type": "string" + }, + "default": {}, + "multipleOf": { + "type": "number", + "minimum": 0, + "exclusiveMinimum": true + }, + "maximum": { + "type": "number" + }, + "exclusiveMaximum": { + "type": "boolean", + "default": false + }, + "minimum": { + "type": "number" + }, + "exclusiveMinimum": { + "type": "boolean", + "default": false + }, + "maxLength": { "$ref": "#/definitions/positiveInteger" }, + "minLength": { "$ref": "#/definitions/positiveIntegerDefault0" }, + "pattern": { + "type": "string", + "format": "regex" + }, + "additionalItems": { + "anyOf": [ + { "type": "boolean" }, + { "$ref": "#" } + ], + "default": {} + }, + "items": { + "anyOf": [ + { "$ref": "#" }, + { "$ref": "#/definitions/schemaArray" } + ], + "default": {} + }, + "maxItems": { "$ref": "#/definitions/positiveInteger" }, + "minItems": { "$ref": "#/definitions/positiveIntegerDefault0" }, + "uniqueItems": { + "type": "boolean", + "default": false + }, + "maxProperties": { "$ref": "#/definitions/positiveInteger" }, + "minProperties": { "$ref": "#/definitions/positiveIntegerDefault0" }, + "required": { + "type": "boolean", + "default": false + }, + "secret": { + "type": "boolean", + "default": false + }, + "additionalProperties": { + "anyOf": [ + { "type": "boolean" }, + { "$ref": "#" } + ], + "default": {} + }, + "definitions": { + "type": "object", + "additionalProperties": { "$ref": "#" }, + "default": {} + }, + "properties": { + "type": "object", + "additionalProperties": { "$ref": "#" }, + "default": {} + }, + "patternProperties": { + "type": "object", + "additionalProperties": { "$ref": "#" }, + "default": {} + }, + "dependencies": { + "type": "object", + "additionalProperties": { + "anyOf": [ + { "$ref": "#" }, + { "$ref": "#/definitions/stringArray" } + ] + } + }, + "enum": { + "type": "array", + "minItems": 1, + "uniqueItems": true + }, + "type": { + "anyOf": [ + { "$ref": "#/definitions/simpleTypes" } + ] + }, + "position": { + "type": "number", + "minimum": 0 + }, + "immutable": { + "type": "boolean", + "default": false + }, + "allOf": { "$ref": "#/definitions/schemaArray" }, + "anyOf": { "$ref": "#/definitions/schemaArray" }, + "oneOf": { "$ref": "#/definitions/schemaArray" }, + "not": { "$ref": "#" } + }, + "dependencies": { + "exclusiveMaximum": [ "maximum" ], + "exclusiveMinimum": [ "minimum" ] + }, + "default": {}, + "additionalProperties": false +} From 838ca1ee229bf881c5a2937b1c4fe92e55fb52d6 Mon Sep 17 00:00:00 2001 From: bigmstone Date: Fri, 17 Aug 2018 23:48:16 -0500 Subject: [PATCH 02/22] Runner output schema enforcement --- .../runners/python_runner/python_runner/runner.yaml | 12 +++++++++--- st2actions/st2actions/container/base.py | 13 ++++++++++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/contrib/runners/python_runner/python_runner/runner.yaml b/contrib/runners/python_runner/python_runner/runner.yaml index e7fddc2034..bfdb339233 100644 --- a/contrib/runners/python_runner/python_runner/runner.yaml +++ b/contrib/runners/python_runner/python_runner/runner.yaml @@ -6,13 +6,19 @@ runner_module: python_runner output_schema: result: - type: object + anyOf: + - type: object + - type: string + - type: integer stderr: type: string - required: false + required: true stdout: type: string - required: false + required: true + exit_code: + type: integer + required: true runner_parameters: debug: description: Enable runner debug mode. diff --git a/st2actions/st2actions/container/base.py b/st2actions/st2actions/container/base.py index ee231a1a65..eac263c747 100644 --- a/st2actions/st2actions/container/base.py +++ b/st2actions/st2actions/container/base.py @@ -34,7 +34,7 @@ from st2common.util import param as param_utils from st2common.util.config_loader import ContentPackConfigLoader from st2common.metrics.base import CounterWithTimer -from st2common.util import jsonify +from st2common.util import jsonify, schema from st2common.runners.base import get_runner from st2common.runners.base import AsyncActionRunner, PollingAsyncActionRunner @@ -127,6 +127,17 @@ def _do_run(self, runner): (status, result, context) = runner.run(action_params) result = jsonify.try_loads(result) + try: + LOG.debug('Validating runner output: %s', runner.runner_type.output_schema) + runner_schema = { + "type": "object", + "properties": runner.runner_type.output_schema, + "additionalProperties": False + } + schema.validate(result, runner_schema, cls=schema.get_validator('custom')) + except AttributeError: + LOG.debug('No output schema') + action_completed = status in action_constants.LIVEACTION_COMPLETED_STATES if (isinstance(runner, PollingAsyncActionRunner) and From a647e973104c94610d57e346e4be46eef5141d1d Mon Sep 17 00:00:00 2001 From: bigmstone Date: Tue, 21 Aug 2018 11:46:12 -0500 Subject: [PATCH 03/22] Begin runner output_schema refactor --- .../action_chain_runner/runner.yaml | 5 ++++ .../announcement_runner.py | 7 +++++- .../announcement_runner/runner.yaml | 3 +++ .../cloudslang_runner/runner.yaml | 5 ++++ .../http_runner/http_runner/runner.yaml | 10 +++++++- .../local_runner/local_runner/runner.yaml | 22 ++++++++++++++++++ .../orquesta_runner/runner.yaml | 12 ++++++++++ .../python_runner/python_runner/runner.yaml | 10 +++++--- st2actions/st2actions/container/base.py | 23 +++++++++++++------ 9 files changed, 85 insertions(+), 12 deletions(-) diff --git a/contrib/runners/action_chain_runner/action_chain_runner/runner.yaml b/contrib/runners/action_chain_runner/action_chain_runner/runner.yaml index 0a70d75705..e3eb1b86b5 100644 --- a/contrib/runners/action_chain_runner/action_chain_runner/runner.yaml +++ b/contrib/runners/action_chain_runner/action_chain_runner/runner.yaml @@ -12,3 +12,8 @@ default: [] description: List of tasks to skip notifications for. type: array + output_schema: + published: + type: "object" + tasks: + type: "array" diff --git a/contrib/runners/announcement_runner/announcement_runner/announcement_runner.py b/contrib/runners/announcement_runner/announcement_runner/announcement_runner.py index 2629d9497e..96519c18d9 100644 --- a/contrib/runners/announcement_runner/announcement_runner/announcement_runner.py +++ b/contrib/runners/announcement_runner/announcement_runner/announcement_runner.py @@ -60,7 +60,12 @@ def run(self, action_parameters): self._dispatcher.dispatch(self._route, payload=action_parameters, trace_context=trace_context) - return (LIVEACTION_STATUS_SUCCEEDED, action_parameters, None) + + result = { + "output": action_parameters + } + + return (LIVEACTION_STATUS_SUCCEEDED, result, None) def get_runner(): diff --git a/contrib/runners/announcement_runner/announcement_runner/runner.yaml b/contrib/runners/announcement_runner/announcement_runner/runner.yaml index d84e6a4465..7a5442ff38 100644 --- a/contrib/runners/announcement_runner/announcement_runner/runner.yaml +++ b/contrib/runners/announcement_runner/announcement_runner/runner.yaml @@ -16,3 +16,6 @@ maxLength: 255 minLength: 1 type: string + output_schema: + output: + type: "object" diff --git a/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml b/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml index 16e29f96e0..7b9d31b1df 100644 --- a/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml +++ b/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml @@ -14,3 +14,8 @@ description: Action timeout in seconds. Action will get killed if it doesn't finish in timeout seconds. type: integer + output_schema: + stdout: + type: "string" + stdout: + type: "string" diff --git a/contrib/runners/http_runner/http_runner/runner.yaml b/contrib/runners/http_runner/http_runner/runner.yaml index 5352f6487e..12ef00b362 100644 --- a/contrib/runners/http_runner/http_runner/runner.yaml +++ b/contrib/runners/http_runner/http_runner/runner.yaml @@ -38,4 +38,12 @@ CA bundle which comes from Mozilla. Verification using a custom CA bundle is not yet supported. Set to False to skip verification. type: boolean - + output_schema: + status_code: + type: integer + body: + type: object + parsed: + type: object + headers: + type: object diff --git a/contrib/runners/local_runner/local_runner/runner.yaml b/contrib/runners/local_runner/local_runner/runner.yaml index 598193e5fe..7ab274d3cd 100644 --- a/contrib/runners/local_runner/local_runner/runner.yaml +++ b/contrib/runners/local_runner/local_runner/runner.yaml @@ -35,6 +35,17 @@ description: Action timeout in seconds. Action will get killed if it doesn't finish in timeout seconds. type: integer + output_schema: + succeeded: + type: "boolean" + failed: + type: "boolean" + return_code: + type: "integer" + stderr: + type: "string" + stdout: + type: "string" - aliases: - run-local-script description: A runner to execute local actions as a fixed user. @@ -78,3 +89,14 @@ description: Action timeout in seconds. Action will get killed if it doesn't finish in timeout seconds. type: integer + output_schema: + succeeded: + type: "boolean" + failed: + type: "boolean" + return_code: + type: "integer" + stderr: + type: "string" + stdout: + type: "string" diff --git a/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml b/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml index 094297a0df..b86c6662fc 100644 --- a/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml +++ b/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml @@ -4,3 +4,15 @@ enabled: true name: orquesta runner_module: orquesta_runner + output_schema: + errors: + type: "array" + output: + anyOf: + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" diff --git a/contrib/runners/python_runner/python_runner/runner.yaml b/contrib/runners/python_runner/python_runner/runner.yaml index bfdb339233..70207bec22 100644 --- a/contrib/runners/python_runner/python_runner/runner.yaml +++ b/contrib/runners/python_runner/python_runner/runner.yaml @@ -7,9 +7,13 @@ output_schema: result: anyOf: - - type: object - - type: string - - type: integer + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" stderr: type: string required: true diff --git a/st2actions/st2actions/container/base.py b/st2actions/st2actions/container/base.py index eac263c747..3322217f12 100644 --- a/st2actions/st2actions/container/base.py +++ b/st2actions/st2actions/container/base.py @@ -17,6 +17,7 @@ import sys import traceback +import copy from oslo_config import cfg @@ -127,16 +128,24 @@ def _do_run(self, runner): (status, result, context) = runner.run(action_params) result = jsonify.try_loads(result) - try: - LOG.debug('Validating runner output: %s', runner.runner_type.output_schema) - runner_schema = { + LOG.debug('Validating runner output: %s', runner.runner_type.output_schema) + runner_schema = { + "type": "object", + "properties": runner.runner_type.output_schema, + "additionalProperties": False + } + schema.validate(result, runner_schema, cls=schema.get_validator('custom')) + + if runner.action.output_schema: + output_schema = copy.deepcopy(runner.runner_type.output_schema) + output_schema.update(runner.action.output_schema) + action_schema = { "type": "object", - "properties": runner.runner_type.output_schema, + "properties": output_schema, "additionalProperties": False } - schema.validate(result, runner_schema, cls=schema.get_validator('custom')) - except AttributeError: - LOG.debug('No output schema') + LOG.debug('Validating action output: %s', action_schema) + schema.validate(result, action_schema, cls=schema.get_validator('custom')) action_completed = status in action_constants.LIVEACTION_COMPLETED_STATES From e633179bb3355aa09136568b64964d478bfdd258 Mon Sep 17 00:00:00 2001 From: bigmstone Date: Wed, 22 Aug 2018 22:39:07 -0500 Subject: [PATCH 04/22] Updating runners and definitions. Adding deprecation warning log message --- .../announcement_runner.py | 1 + .../announcement_runner/runner.yaml | 4 +-- .../http_runner/http_runner/runner.yaml | 11 ++++++-- .../inquirer_runner/runner.yaml | 11 ++++++++ .../local_runner/local_runner/runner.yaml | 20 +++++++-------- .../runners/mistral_v2/mistral_v2/runner.yaml | 3 +++ .../noop_runner/noop_runner/runner.yaml | 6 ++++- .../remote_runner/remote_command_runner.py | 1 + .../remote_runner/remote_runner/runner.yaml | 6 +++++ .../windows_runner/windows_runner/runner.yaml | 22 ++++++++++++++++ .../winrm_runner/winrm_runner/runner.yaml | 9 +++++++ st2actions/st2actions/container/base.py | 25 +++++++++++++++---- 12 files changed, 99 insertions(+), 20 deletions(-) diff --git a/contrib/runners/announcement_runner/announcement_runner/announcement_runner.py b/contrib/runners/announcement_runner/announcement_runner/announcement_runner.py index 96519c18d9..335483ec0f 100644 --- a/contrib/runners/announcement_runner/announcement_runner/announcement_runner.py +++ b/contrib/runners/announcement_runner/announcement_runner/announcement_runner.py @@ -64,6 +64,7 @@ def run(self, action_parameters): result = { "output": action_parameters } + result.update(action_parameters) return (LIVEACTION_STATUS_SUCCEEDED, result, None) diff --git a/contrib/runners/announcement_runner/announcement_runner/runner.yaml b/contrib/runners/announcement_runner/announcement_runner/runner.yaml index 7a5442ff38..937faa70cb 100644 --- a/contrib/runners/announcement_runner/announcement_runner/runner.yaml +++ b/contrib/runners/announcement_runner/announcement_runner/runner.yaml @@ -17,5 +17,5 @@ minLength: 1 type: string output_schema: - output: - type: "object" + unmodeled: + type: boolean diff --git a/contrib/runners/http_runner/http_runner/runner.yaml b/contrib/runners/http_runner/http_runner/runner.yaml index 12ef00b362..6b127bb326 100644 --- a/contrib/runners/http_runner/http_runner/runner.yaml +++ b/contrib/runners/http_runner/http_runner/runner.yaml @@ -42,8 +42,15 @@ status_code: type: integer body: - type: object + anyOf: + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" parsed: - type: object + type: boolean headers: type: object diff --git a/contrib/runners/inquirer_runner/inquirer_runner/runner.yaml b/contrib/runners/inquirer_runner/inquirer_runner/runner.yaml index e9819118a0..f115fda789 100644 --- a/contrib/runners/inquirer_runner/inquirer_runner/runner.yaml +++ b/contrib/runners/inquirer_runner/inquirer_runner/runner.yaml @@ -35,3 +35,14 @@ required: true description: Time (in minutes) to wait before timing out the inquiry if no response is received type: integer + output_schema: + schema: + type: object + roles: + type: array + users: + type: array + route: + type: string + ttl: + type: integer diff --git a/contrib/runners/local_runner/local_runner/runner.yaml b/contrib/runners/local_runner/local_runner/runner.yaml index 7ab274d3cd..9d2e7d9242 100644 --- a/contrib/runners/local_runner/local_runner/runner.yaml +++ b/contrib/runners/local_runner/local_runner/runner.yaml @@ -37,15 +37,15 @@ type: integer output_schema: succeeded: - type: "boolean" + type: boolean failed: - type: "boolean" + type: boolean return_code: - type: "integer" + type: integer stderr: - type: "string" + type: string stdout: - type: "string" + type: string - aliases: - run-local-script description: A runner to execute local actions as a fixed user. @@ -91,12 +91,12 @@ type: integer output_schema: succeeded: - type: "boolean" + type: boolean failed: - type: "boolean" + type: boolean return_code: - type: "integer" + type: integer stderr: - type: "string" + type: string stdout: - type: "string" + type: string diff --git a/contrib/runners/mistral_v2/mistral_v2/runner.yaml b/contrib/runners/mistral_v2/mistral_v2/runner.yaml index d2404b3507..9a4fe5d8c2 100644 --- a/contrib/runners/mistral_v2/mistral_v2/runner.yaml +++ b/contrib/runners/mistral_v2/mistral_v2/runner.yaml @@ -22,3 +22,6 @@ If entry point is a workflow or a workbook with a single workflow, the runner will identify the workflow automatically. type: string + output_schema: + unmodeled: + type: boolean diff --git a/contrib/runners/noop_runner/noop_runner/runner.yaml b/contrib/runners/noop_runner/noop_runner/runner.yaml index b5a30b4847..3f5bff0e6d 100644 --- a/contrib/runners/noop_runner/noop_runner/runner.yaml +++ b/contrib/runners/noop_runner/noop_runner/runner.yaml @@ -4,4 +4,8 @@ name: noop runner_module: noop_runner runner_parameters: {} - + output_schema: + stdout: + type: string + stderr: + type: string diff --git a/contrib/runners/remote_runner/remote_runner/remote_command_runner.py b/contrib/runners/remote_runner/remote_runner/remote_command_runner.py index 9b68e3d259..7f678e348d 100644 --- a/contrib/runners/remote_runner/remote_runner/remote_command_runner.py +++ b/contrib/runners/remote_runner/remote_runner/remote_command_runner.py @@ -49,6 +49,7 @@ def _run(self, remote_action): command = remote_action.get_full_command_string() return self._parallel_ssh_client.run(command, timeout=remote_action.get_timeout()) + def _get_remote_action(self, action_paramaters): # remote script actions with entry_point don't make sense, user probably wanted to use # "remote-shell-script" action diff --git a/contrib/runners/remote_runner/remote_runner/runner.yaml b/contrib/runners/remote_runner/remote_runner/runner.yaml index b690c3ad7a..dfa1b3bad2 100644 --- a/contrib/runners/remote_runner/remote_runner/runner.yaml +++ b/contrib/runners/remote_runner/remote_runner/runner.yaml @@ -85,6 +85,9 @@ config is used. required: false type: string + output_schema: + unmodeled: + type: boolean - aliases: - run-remote-script description: A remote execution runner that executes actions as a fixed system user. @@ -168,3 +171,6 @@ config is used. required: false type: string + output_schema: + unmodeled: + type: boolean diff --git a/contrib/runners/windows_runner/windows_runner/runner.yaml b/contrib/runners/windows_runner/windows_runner/runner.yaml index caff847bff..f7df25b1f1 100644 --- a/contrib/runners/windows_runner/windows_runner/runner.yaml +++ b/contrib/runners/windows_runner/windows_runner/runner.yaml @@ -28,6 +28,17 @@ description: Username used to log-in. required: true type: string + output_schema: + stdout: + type: string + stderr: + type: string + return_code: + type: integer + succeeded: + type: boolean + failed: + type: boolean - aliases: [] description: A remote execution runner that executes power shell scripts on Windows hosts. @@ -61,3 +72,14 @@ description: Username used to log-in. required: true type: string + output_schema: + stdout: + type: string + stderr: + type: string + return_code: + type: integer + succeeded: + type: boolean + failed: + type: boolean diff --git a/contrib/runners/winrm_runner/winrm_runner/runner.yaml b/contrib/runners/winrm_runner/winrm_runner/runner.yaml index 88a938c37f..887660f551 100644 --- a/contrib/runners/winrm_runner/winrm_runner/runner.yaml +++ b/contrib/runners/winrm_runner/winrm_runner/runner.yaml @@ -66,6 +66,9 @@ CA bundle which comes from Mozilla. Verification using a custom CA bundle is not yet supported. Set to False to skip verification. type: boolean + output_schema: + unmodeled: + type: boolean - description: A remote execution runner that executes PowerShell commands via WinRM on a remote host enabled: true name: winrm-ps-cmd @@ -134,6 +137,9 @@ CA bundle which comes from Mozilla. Verification using a custom CA bundle is not yet supported. Set to False to skip verification. type: boolean + output_schema: + unmodeled: + type: boolean - description: A remote execution runner that executes PowerShell script via WinRM on a set of remote hosts enabled: true name: winrm-ps-script @@ -199,3 +205,6 @@ CA bundle which comes from Mozilla. Verification using a custom CA bundle is not yet supported. Set to False to skip verification. type: boolean + output_schema: + unmodeled: + type: boolean diff --git a/st2actions/st2actions/container/base.py b/st2actions/st2actions/container/base.py index 3322217f12..67ef8a264b 100644 --- a/st2actions/st2actions/container/base.py +++ b/st2actions/st2actions/container/base.py @@ -129,11 +129,26 @@ def _do_run(self, runner): result = jsonify.try_loads(result) LOG.debug('Validating runner output: %s', runner.runner_type.output_schema) - runner_schema = { - "type": "object", - "properties": runner.runner_type.output_schema, - "additionalProperties": False - } + + if runner.runner_type.output_schema.get('unmodeled'): + runner.runner_type.output_schema.pop('unmodeled') + LOG.warn( + """Deprecation Notice: This runner has previously had unmodeled + output. In StackStorm 3.1 the output will be placed under the + `output` key.""" + ) + runner_schema = { + "type": "object", + "properties": runner.runner_type.output_schema, + "additionalProperties": True + } + else: + runner_schema = { + "type": "object", + "properties": runner.runner_type.output_schema, + "additionalProperties": False + } + schema.validate(result, runner_schema, cls=schema.get_validator('custom')) if runner.action.output_schema: From 8bb6a56a4f62caa93455d04262007af1bbe28214 Mon Sep 17 00:00:00 2001 From: bigmstone Date: Sun, 26 Aug 2018 14:37:42 -0500 Subject: [PATCH 05/22] Fixing tests --- .../local_runner/local_runner/runner.yaml | 36 ++++++++++++++++--- .../tests/unit/test_mistral_v2_callback.py | 4 +-- .../noop_runner/noop_runner/runner.yaml | 24 +++++++++++-- .../orquesta_runner/runner.yaml | 4 ++- .../remote_runner/remote_command_runner.py | 1 - .../windows_runner/windows_runner/runner.yaml | 36 ++++++++++++++++--- .../winrm_runner/winrm_runner/runner.yaml | 18 +++++----- st2actions/st2actions/container/base.py | 10 +++--- st2client/tests/unit/test_action.py | 8 +++-- .../fixtures/generic/runners/actionchain.yaml | 3 ++ .../fixtures/generic/runners/inquirer.yaml | 3 ++ .../fixtures/generic/runners/run-local.yaml | 11 ++++++ .../generic/runners/testasyncrunner1.yaml | 3 ++ .../generic/runners/testasyncrunner2.yaml | 3 ++ .../generic/runners/testfailingrunner1.yaml | 3 ++ .../fixtures/generic/runners/testrunner1.yaml | 3 ++ .../fixtures/generic/runners/testrunner2.yaml | 3 ++ .../fixtures/generic/runners/testrunner3.yaml | 3 ++ 18 files changed, 145 insertions(+), 31 deletions(-) diff --git a/contrib/runners/local_runner/local_runner/runner.yaml b/contrib/runners/local_runner/local_runner/runner.yaml index 9d2e7d9242..ee633d7c37 100644 --- a/contrib/runners/local_runner/local_runner/runner.yaml +++ b/contrib/runners/local_runner/local_runner/runner.yaml @@ -43,9 +43,23 @@ return_code: type: integer stderr: - type: string + anyOf: + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" stdout: - type: string + anyOf: + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" - aliases: - run-local-script description: A runner to execute local actions as a fixed user. @@ -97,6 +111,20 @@ return_code: type: integer stderr: - type: string + anyOf: + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" stdout: - type: string + anyOf: + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" diff --git a/contrib/runners/mistral_v2/tests/unit/test_mistral_v2_callback.py b/contrib/runners/mistral_v2/tests/unit/test_mistral_v2_callback.py index ef2fe09a80..01d2845e38 100644 --- a/contrib/runners/mistral_v2/tests/unit/test_mistral_v2_callback.py +++ b/contrib/runners/mistral_v2/tests/unit/test_mistral_v2_callback.py @@ -54,9 +54,9 @@ ] if six.PY2: - NON_EMPTY_RESULT = 'non-empty' + NON_EMPTY_RESULT = '{"stdout": "non-empty"}' else: - NON_EMPTY_RESULT = u'non-empty' + NON_EMPTY_RESULT = u'{"stdout": "non-empty"}' @mock.patch.object( diff --git a/contrib/runners/noop_runner/noop_runner/runner.yaml b/contrib/runners/noop_runner/noop_runner/runner.yaml index 3f5bff0e6d..6062550af9 100644 --- a/contrib/runners/noop_runner/noop_runner/runner.yaml +++ b/contrib/runners/noop_runner/noop_runner/runner.yaml @@ -6,6 +6,26 @@ runner_parameters: {} output_schema: stdout: - type: string + anyOf: + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" stderr: - type: string + anyOf: + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" + failed: + type: boolean + succeeded: + type: boolean + return_code: + type: integer diff --git a/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml b/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml index b86c6662fc..05c09cc249 100644 --- a/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml +++ b/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml @@ -6,7 +6,9 @@ runner_module: orquesta_runner output_schema: errors: - type: "array" + anyOf: + - type: "object" + - type: "array" output: anyOf: - type: "object" diff --git a/contrib/runners/remote_runner/remote_runner/remote_command_runner.py b/contrib/runners/remote_runner/remote_runner/remote_command_runner.py index 7f678e348d..9b68e3d259 100644 --- a/contrib/runners/remote_runner/remote_runner/remote_command_runner.py +++ b/contrib/runners/remote_runner/remote_runner/remote_command_runner.py @@ -49,7 +49,6 @@ def _run(self, remote_action): command = remote_action.get_full_command_string() return self._parallel_ssh_client.run(command, timeout=remote_action.get_timeout()) - def _get_remote_action(self, action_paramaters): # remote script actions with entry_point don't make sense, user probably wanted to use # "remote-shell-script" action diff --git a/contrib/runners/windows_runner/windows_runner/runner.yaml b/contrib/runners/windows_runner/windows_runner/runner.yaml index f7df25b1f1..b334027ba2 100644 --- a/contrib/runners/windows_runner/windows_runner/runner.yaml +++ b/contrib/runners/windows_runner/windows_runner/runner.yaml @@ -30,9 +30,23 @@ type: string output_schema: stdout: - type: string + anyOf: + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" stderr: - type: string + anyOf: + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" return_code: type: integer succeeded: @@ -74,9 +88,23 @@ type: string output_schema: stdout: - type: string + anyOf: + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" stderr: - type: string + anyOf: + - type: "object" + - type: "string" + - type: "integer" + - type: "number" + - type: "boolean" + - type: "array" + - type: "null" return_code: type: integer succeeded: diff --git a/contrib/runners/winrm_runner/winrm_runner/runner.yaml b/contrib/runners/winrm_runner/winrm_runner/runner.yaml index 887660f551..06f0e2ffa7 100644 --- a/contrib/runners/winrm_runner/winrm_runner/runner.yaml +++ b/contrib/runners/winrm_runner/winrm_runner/runner.yaml @@ -3,6 +3,9 @@ name: winrm-cmd runner_package: winrm_runner runner_module: winrm_command_runner + output_schema: + unmodeled: + type: boolean runner_parameters: cmd: description: Arbitrary Command Prompt command to be executed on the remote host. @@ -66,14 +69,14 @@ CA bundle which comes from Mozilla. Verification using a custom CA bundle is not yet supported. Set to False to skip verification. type: boolean - output_schema: - unmodeled: - type: boolean - description: A remote execution runner that executes PowerShell commands via WinRM on a remote host enabled: true name: winrm-ps-cmd runner_package: winrm_runner runner_module: winrm_ps_command_runner + output_schema: + unmodeled: + type: boolean runner_parameters: cmd: description: Arbitrary PowerShell command to be executed on the remote host. @@ -137,14 +140,14 @@ CA bundle which comes from Mozilla. Verification using a custom CA bundle is not yet supported. Set to False to skip verification. type: boolean - output_schema: - unmodeled: - type: boolean - description: A remote execution runner that executes PowerShell script via WinRM on a set of remote hosts enabled: true name: winrm-ps-script runner_package: winrm_runner runner_module: winrm_ps_script_runner + output_schema: + unmodeled: + type: boolean runner_parameters: cwd: description: Working directory where the command will be executed in @@ -205,6 +208,3 @@ CA bundle which comes from Mozilla. Verification using a custom CA bundle is not yet supported. Set to False to skip verification. type: boolean - output_schema: - unmodeled: - type: boolean diff --git a/st2actions/st2actions/container/base.py b/st2actions/st2actions/container/base.py index 67ef8a264b..9633d02fb2 100644 --- a/st2actions/st2actions/container/base.py +++ b/st2actions/st2actions/container/base.py @@ -133,13 +133,11 @@ def _do_run(self, runner): if runner.runner_type.output_schema.get('unmodeled'): runner.runner_type.output_schema.pop('unmodeled') LOG.warn( - """Deprecation Notice: This runner has previously had unmodeled - output. In StackStorm 3.1 the output will be placed under the - `output` key.""" + """Deprecation Notice: This runner has previously had unmodeled + output. In StackStorm 3.1 the output will be placed under the + `output` key.""" ) runner_schema = { - "type": "object", - "properties": runner.runner_type.output_schema, "additionalProperties": True } else: @@ -149,6 +147,8 @@ def _do_run(self, runner): "additionalProperties": False } + LOG.info("Action Result: %s", result) + schema.validate(result, runner_schema, cls=schema.get_validator('custom')) if runner.action.output_schema: diff --git a/st2client/tests/unit/test_action.py b/st2client/tests/unit/test_action.py index c648be951d..a6ad6a3796 100644 --- a/st2client/tests/unit/test_action.py +++ b/st2client/tests/unit/test_action.py @@ -36,7 +36,8 @@ "list": {"type": "array"}, "str": {"type": "string"} }, - "name": "mock-runner1" + "name": "mock-runner1", + "output_schema": {"unmodeled": {"type": "boolean"}} } ACTION1 = { @@ -52,7 +53,8 @@ RUNNER2 = { "enabled": True, "runner_parameters": {}, - "name": "mock-runner2" + "name": "mock-runner2", + "output_schema": {"unmodeled": {"type": "boolean"}} } ACTION2 = { @@ -75,7 +77,7 @@ LIVE_ACTION = { 'action': 'mockety.mock', 'status': 'complete', - 'result': 'non-empty' + 'result': {'stdout': 'non-empty'} } diff --git a/st2tests/st2tests/fixtures/generic/runners/actionchain.yaml b/st2tests/st2tests/fixtures/generic/runners/actionchain.yaml index 935115ec83..55ba56a0be 100644 --- a/st2tests/st2tests/fixtures/generic/runners/actionchain.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/actionchain.yaml @@ -4,3 +4,6 @@ enabled: true name: action-chain runner_module: action_chain_runner runner_parameters: {} +output_schema: + unmodeled: + type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/inquirer.yaml b/st2tests/st2tests/fixtures/generic/runners/inquirer.yaml index 5147d6ae32..89e856715f 100644 --- a/st2tests/st2tests/fixtures/generic/runners/inquirer.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/inquirer.yaml @@ -36,3 +36,6 @@ runner_parameters: required: true description: Time (in minutes) that an unacknowledged Inquiry is cleaned up type: integer +output_schema: + unmodeled: + type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/run-local.yaml b/st2tests/st2tests/fixtures/generic/runners/run-local.yaml index 74c82734b5..557dcb99fc 100644 --- a/st2tests/st2tests/fixtures/generic/runners/run-local.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/run-local.yaml @@ -14,3 +14,14 @@ runner_parameters: sudo: default: false type: boolean +output_schema: + succeeded: + type: boolean + failed: + type: boolean + return_code: + type: integer + stderr: + type: string + stdout: + type: string diff --git a/st2tests/st2tests/fixtures/generic/runners/testasyncrunner1.yaml b/st2tests/st2tests/fixtures/generic/runners/testasyncrunner1.yaml index cff11068c6..9ceacfd55e 100644 --- a/st2tests/st2tests/fixtures/generic/runners/testasyncrunner1.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/testasyncrunner1.yaml @@ -20,3 +20,6 @@ runner_parameters: default: defaultfoo description: Foo str param. type: string +output_schema: + unmodeled: + type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/testasyncrunner2.yaml b/st2tests/st2tests/fixtures/generic/runners/testasyncrunner2.yaml index 713328c365..cd7bc14659 100644 --- a/st2tests/st2tests/fixtures/generic/runners/testasyncrunner2.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/testasyncrunner2.yaml @@ -21,3 +21,6 @@ runner_parameters: default: defaultfoo description: Foo str param. type: string +output_schema: + unmodeled: + type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/testfailingrunner1.yaml b/st2tests/st2tests/fixtures/generic/runners/testfailingrunner1.yaml index c680773ba5..8c8d00c7f0 100644 --- a/st2tests/st2tests/fixtures/generic/runners/testfailingrunner1.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/testfailingrunner1.yaml @@ -9,3 +9,6 @@ runner_parameters: description: Foo str param. immutable: true type: boolean +output_schema: + unmodeled: + type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/testrunner1.yaml b/st2tests/st2tests/fixtures/generic/runners/testrunner1.yaml index 725b5c033e..be72ccfacf 100644 --- a/st2tests/st2tests/fixtures/generic/runners/testrunner1.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/testrunner1.yaml @@ -31,3 +31,6 @@ runner_parameters: default: defaultfoo description: Foo str param. type: string +output_schema: + unmodeled: + type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/testrunner2.yaml b/st2tests/st2tests/fixtures/generic/runners/testrunner2.yaml index 8810fda9b8..c1ea55f046 100644 --- a/st2tests/st2tests/fixtures/generic/runners/testrunner2.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/testrunner2.yaml @@ -4,3 +4,6 @@ enabled: true name: test-runner-2 runner_module: runner runner_parameters: {} +output_schema: + unmodeled: + type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/testrunner3.yaml b/st2tests/st2tests/fixtures/generic/runners/testrunner3.yaml index bb9db6555e..cfb5a64280 100644 --- a/st2tests/st2tests/fixtures/generic/runners/testrunner3.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/testrunner3.yaml @@ -12,3 +12,6 @@ runner_parameters: required: true k3: type: string +output_schema: + unmodeled: + type: boolean From bc9b9fe1c153489d001c4943a60b0d588c323d71 Mon Sep 17 00:00:00 2001 From: bigmstone Date: Mon, 27 Aug 2018 17:28:00 -0500 Subject: [PATCH 06/22] Adding st2client output_schema notification --- st2client/st2client/commands/action.py | 31 ++++++++++++++++ st2client/st2client/utils/schema.py | 51 ++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 st2client/st2client/utils/schema.py diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index b393e67bc9..0443414cc4 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -26,6 +26,8 @@ import six import sys +import yaml + from os.path import join as pjoin from six.moves import range @@ -40,6 +42,7 @@ from st2client.utils.date import format_isodate_for_user_timezone from st2client.utils.date import parse as parse_isotime from st2client.utils.color import format_status +from st2client.utils import schema LOG = logging.getLogger(__name__) @@ -319,6 +322,15 @@ def _add_common_options(self): return root_arg_grp + def _print_bordered(self, text): + lines = text.split('\n') + width = max(len(s) for s in lines) + 2 + res = ['+' + '-' * width + '+'] + for s in lines: + res.append('| ' + (s + ' ' * width)[:width-2] + ' |') + res.append('+' + '-' * width + '+') + print '\n'.join(res) + def _print_execution_details(self, execution, args, **kwargs): """ Print the execution detail to stdout. @@ -335,6 +347,24 @@ def _print_execution_details(self, execution, args, **kwargs): key = getattr(args, 'key', None) attr = getattr(args, 'attr', []) + rendered_schema = schema.render_output_schema_from_output( + execution.result + ) + + rendered_schema = { + 'output_schema': rendered_schema + } + + rendered_schema = yaml.safe_dump(rendered_schema, default_flow_style=False) + + if not execution.action['output_schema']: + self._print_bordered( + "This action does not have an output schema defined. Based " + "on the action output the following inferred schema was built:" + "\n\n" + "%s" % rendered_schema + ) + if show_tasks and not is_workflow_action: raise ValueError('--show-tasks option can only be used with workflow actions') @@ -1003,6 +1033,7 @@ def run(self, args, **kwargs): execution = self._get_execution_result(execution=execution, action_exec_mgr=action_exec_mgr, args=args, **kwargs) + return execution diff --git a/st2client/st2client/utils/schema.py b/st2client/st2client/utils/schema.py new file mode 100644 index 0000000000..fdc4263924 --- /dev/null +++ b/st2client/st2client/utils/schema.py @@ -0,0 +1,51 @@ +# Licensed to the StackStorm, Inc ('StackStorm') under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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 types import NoneType + + +TYPE_TABLE = { + dict: 'object', + list: 'array', + int: 'integer', + str: 'string', + unicode: 'string', + float: 'number', + bool: 'boolean', + NoneType: 'null', +} + + +def _dict_to_schema(item): + schema = {} + for key, value in item.iteritems(): + if isinstance(value, dict): + schema[key] = { + 'type': 'object', + 'parameters': _dict_to_schema(value) + } + else: + schema[key] = { + 'type': TYPE_TABLE[type(value)] + } + + return schema + + + +def render_output_schema_from_output(output): + """Given an action output produce a reasonable schema to match. + """ + return _dict_to_schema(output) From a833fbb4538c64972f5859709884033ce5ff763e Mon Sep 17 00:00:00 2001 From: bigmstone Date: Tue, 4 Sep 2018 14:55:07 -0500 Subject: [PATCH 07/22] Fixing st2client tests --- st2client/st2client/commands/action.py | 35 +++---------------- st2client/st2client/formatters/execution.py | 29 +++++++++++++++ st2client/st2client/utils/schema.py | 4 +-- .../fixtures/execution_get_attributes.txt | 2 ++ .../tests/fixtures/execution_get_default.txt | 2 ++ .../fixtures/execution_unescape_newline.txt | 2 ++ .../tests/fixtures/execution_unicode.txt | 2 ++ st2client/tests/unit/test_formatters.py | 1 + 8 files changed, 44 insertions(+), 33 deletions(-) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index 0443414cc4..b28aa6fc64 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -26,8 +26,6 @@ import six import sys -import yaml - from os.path import join as pjoin from six.moves import range @@ -42,7 +40,6 @@ from st2client.utils.date import format_isodate_for_user_timezone from st2client.utils.date import parse as parse_isotime from st2client.utils.color import format_status -from st2client.utils import schema LOG = logging.getLogger(__name__) @@ -280,6 +277,10 @@ def _add_common_options(self): # Display options task_list_arg_grp = root_arg_grp.add_argument_group() + task_list_arg_grp.add_argument('--with-schema', + default=False, action='store_true', + help=('Show schema_ouput suggestion with action.')) + task_list_arg_grp.add_argument('--raw', action='store_true', help='Raw output, don\'t show sub-tasks for workflows.') task_list_arg_grp.add_argument('--show-tasks', action='store_true', @@ -322,15 +323,6 @@ def _add_common_options(self): return root_arg_grp - def _print_bordered(self, text): - lines = text.split('\n') - width = max(len(s) for s in lines) + 2 - res = ['+' + '-' * width + '+'] - for s in lines: - res.append('| ' + (s + ' ' * width)[:width-2] + ' |') - res.append('+' + '-' * width + '+') - print '\n'.join(res) - def _print_execution_details(self, execution, args, **kwargs): """ Print the execution detail to stdout. @@ -347,24 +339,6 @@ def _print_execution_details(self, execution, args, **kwargs): key = getattr(args, 'key', None) attr = getattr(args, 'attr', []) - rendered_schema = schema.render_output_schema_from_output( - execution.result - ) - - rendered_schema = { - 'output_schema': rendered_schema - } - - rendered_schema = yaml.safe_dump(rendered_schema, default_flow_style=False) - - if not execution.action['output_schema']: - self._print_bordered( - "This action does not have an output schema defined. Based " - "on the action output the following inferred schema was built:" - "\n\n" - "%s" % rendered_schema - ) - if show_tasks and not is_workflow_action: raise ValueError('--show-tasks option can only be used with workflow actions') @@ -386,6 +360,7 @@ def _print_execution_details(self, execution, args, **kwargs): options = {'attributes': attr} options['json'] = args.json + options['with_schema'] = args.with_schema options['attribute_transform_functions'] = self.attribute_transform_functions self.print_output(instance, formatter, **options) diff --git a/st2client/st2client/formatters/execution.py b/st2client/st2client/formatters/execution.py index 0de0d708b3..0d786c9872 100644 --- a/st2client/st2client/formatters/execution.py +++ b/st2client/st2client/formatters/execution.py @@ -25,6 +25,7 @@ from st2client.utils import jsutil from st2client.utils import strutil from st2client.utils.color import DisplayColors +from st2client.utils import schema import six @@ -33,6 +34,16 @@ PLATFORM_MAXINT = 2 ** (struct.Struct('i').size * 8 - 1) - 1 +def _print_bordered(text): + lines = text.split('\n') + width = max(len(s) for s in lines) + 2 + res = ['\n+' + '-' * width + '+'] + for s in lines: + res.append('| ' + (s + ' ' * width)[:width - 2] + ' |') + res.append('+' + '-' * width + '+') + return '\n'.join(res) + + class ExecutionResult(formatters.Formatter): @classmethod @@ -76,6 +87,24 @@ def format(cls, entry, *args, **kwargs): output += ('\n' if output else '') + '%s: %s' % \ (DisplayColors.colorize(attr, DisplayColors.BLUE), value) + if not entry.get('action', {}).get('output_schema') and kwargs['with_schema']: + rendered_schema = { + 'output_schema': schema.render_output_schema_from_output(entry['result']) + } + + rendered_schema = yaml.safe_dump(rendered_schema, default_flow_style=False) + output += _print_bordered( + "This action does not have an output schema defined. Based " + "on the action output the following inferred schema was built:" + "\n\n" + "%s" % rendered_schema + ) + elif not entry.get('action', {}).get('output_schema'): + output += ( + "\n\n** This action does not have an output_schema. " + "Run again with --with-schema to see a suggested schema." + ) + if six.PY3: return strutil.unescape(str(output)) else: diff --git a/st2client/st2client/utils/schema.py b/st2client/st2client/utils/schema.py index fdc4263924..ce2451da94 100644 --- a/st2client/st2client/utils/schema.py +++ b/st2client/st2client/utils/schema.py @@ -13,7 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. # -from types import NoneType TYPE_TABLE = { @@ -24,7 +23,7 @@ unicode: 'string', float: 'number', bool: 'boolean', - NoneType: 'null', + type(None): 'null', } @@ -44,7 +43,6 @@ def _dict_to_schema(item): return schema - def render_output_schema_from_output(output): """Given an action output produce a reasonable schema to match. """ diff --git a/st2client/tests/fixtures/execution_get_attributes.txt b/st2client/tests/fixtures/execution_get_attributes.txt index 8f898c3906..1d7664aa77 100644 --- a/st2client/tests/fixtures/execution_get_attributes.txt +++ b/st2client/tests/fixtures/execution_get_attributes.txt @@ -1,2 +1,4 @@ status: succeeded (1s elapsed) end_timestamp: Tue, 02 Dec 2014 19:56:07 UTC + +** This action does not have an output_schema. Run again with --with-schema to see a suggested schema. diff --git a/st2client/tests/fixtures/execution_get_default.txt b/st2client/tests/fixtures/execution_get_default.txt index 2613b61f2c..79ef717eea 100644 --- a/st2client/tests/fixtures/execution_get_default.txt +++ b/st2client/tests/fixtures/execution_get_default.txt @@ -16,3 +16,5 @@ result: 3 packets transmitted, 3 received, 0% packet loss, time 1998ms rtt min/avg/max/mdev = 0.015/0.023/0.030/0.006 ms' succeeded: true + +** This action does not have an output_schema. Run again with --with-schema to see a suggested schema. diff --git a/st2client/tests/fixtures/execution_unescape_newline.txt b/st2client/tests/fixtures/execution_unescape_newline.txt index 40ab18deca..9bb2865cca 100644 --- a/st2client/tests/fixtures/execution_unescape_newline.txt +++ b/st2client/tests/fixtures/execution_unescape_newline.txt @@ -11,3 +11,5 @@ Traceback (most recent call last): stdout: 'FOOBAR2 ' succeeded: true + +** This action does not have an output_schema. Run again with --with-schema to see a suggested schema. diff --git a/st2client/tests/fixtures/execution_unicode.txt b/st2client/tests/fixtures/execution_unicode.txt index f7209afaa9..5aedcde8f9 100644 --- a/st2client/tests/fixtures/execution_unicode.txt +++ b/st2client/tests/fixtures/execution_unicode.txt @@ -8,3 +8,5 @@ result: stderr: '' stdout: "‡" succeeded: true + +** This action does not have an output_schema. Run again with --with-schema to see a suggested schema. diff --git a/st2client/tests/unit/test_formatters.py b/st2client/tests/unit/test_formatters.py index bfe8c0b8e4..3562ca660a 100644 --- a/st2client/tests/unit/test_formatters.py +++ b/st2client/tests/unit/test_formatters.py @@ -59,6 +59,7 @@ FIXTURES = loader.load_fixtures(fixtures_dict=FIXTURES_MANIFEST) EXECUTION = FIXTURES['executions']['execution.json'] UNICODE = FIXTURES['executions']['execution_unicode.json'] +OUTPUT_SCHEMA = FIXTURES['executions']['execution_with_schema.json'] NEWLINE = FIXTURES['executions']['execution_with_stack_trace.json'] HAS_CARRIAGE_RETURN = FIXTURES['executions']['execution_result_has_carriage_return.json'] From b4e68023cdc9e4d81b67f78c389db07280e591b0 Mon Sep 17 00:00:00 2001 From: bigmstone Date: Tue, 4 Sep 2018 18:01:34 -0500 Subject: [PATCH 08/22] Adding unit test to st2client for ouput_schema present --- st2client/st2client/formatters/execution.py | 6 ++-- st2client/st2client/utils/schema.py | 5 +++- .../fixtures/execution_get_has_schema.txt | 18 +++++++++++ .../tests/fixtures/execution_unicode_py3.txt | 2 ++ .../tests/fixtures/execution_with_schema.json | 30 +++++++++++++++++++ st2client/tests/unit/test_formatters.py | 22 ++++++++++++-- 6 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 st2client/tests/fixtures/execution_get_has_schema.txt create mode 100644 st2client/tests/fixtures/execution_with_schema.json diff --git a/st2client/st2client/formatters/execution.py b/st2client/st2client/formatters/execution.py index 0d786c9872..ea2ae583c2 100644 --- a/st2client/st2client/formatters/execution.py +++ b/st2client/st2client/formatters/execution.py @@ -87,7 +87,9 @@ def format(cls, entry, *args, **kwargs): output += ('\n' if output else '') + '%s: %s' % \ (DisplayColors.colorize(attr, DisplayColors.BLUE), value) - if not entry.get('action', {}).get('output_schema') and kwargs['with_schema']: + output_schema = entry.get('action', {}).get('output_schema') + + if not output_schema and kwargs.get('with_schema'): rendered_schema = { 'output_schema': schema.render_output_schema_from_output(entry['result']) } @@ -99,7 +101,7 @@ def format(cls, entry, *args, **kwargs): "\n\n" "%s" % rendered_schema ) - elif not entry.get('action', {}).get('output_schema'): + elif not output_schema: output += ( "\n\n** This action does not have an output_schema. " "Run again with --with-schema to see a suggested schema." diff --git a/st2client/st2client/utils/schema.py b/st2client/st2client/utils/schema.py index ce2451da94..3b38c34f86 100644 --- a/st2client/st2client/utils/schema.py +++ b/st2client/st2client/utils/schema.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # +import sys TYPE_TABLE = { @@ -20,12 +21,14 @@ list: 'array', int: 'integer', str: 'string', - unicode: 'string', float: 'number', bool: 'boolean', type(None): 'null', } +if sys.version_info[0] < 3: + TYPE_TABLE[unicode] = 'string' + def _dict_to_schema(item): schema = {} diff --git a/st2client/tests/fixtures/execution_get_has_schema.txt b/st2client/tests/fixtures/execution_get_has_schema.txt new file mode 100644 index 0000000000..2613b61f2c --- /dev/null +++ b/st2client/tests/fixtures/execution_get_has_schema.txt @@ -0,0 +1,18 @@ +id: 547e19561e2e2417d3dde398 +status: succeeded (1s elapsed) +parameters: + cmd: 127.0.0.1 3 +result: + localhost: + failed: false + return_code: 0 + stderr: '' + stdout: 'PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data. + 64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.015 ms + 64 bytes from 127.0.0.1: icmp_seq=2 ttl=64 time=0.024 ms + 64 bytes from 127.0.0.1: icmp_seq=3 ttl=64 time=0.030 ms + + --- 127.0.0.1 ping statistics --- + 3 packets transmitted, 3 received, 0% packet loss, time 1998ms + rtt min/avg/max/mdev = 0.015/0.023/0.030/0.006 ms' + succeeded: true diff --git a/st2client/tests/fixtures/execution_unicode_py3.txt b/st2client/tests/fixtures/execution_unicode_py3.txt index ba96a99346..9550d14f28 100644 --- a/st2client/tests/fixtures/execution_unicode_py3.txt +++ b/st2client/tests/fixtures/execution_unicode_py3.txt @@ -8,3 +8,5 @@ result: stderr: '' stdout: "\u2021" succeeded: true + +** This action does not have an output_schema. Run again with --with-schema to see a suggested schema. diff --git a/st2client/tests/fixtures/execution_with_schema.json b/st2client/tests/fixtures/execution_with_schema.json new file mode 100644 index 0000000000..a9c0b99169 --- /dev/null +++ b/st2client/tests/fixtures/execution_with_schema.json @@ -0,0 +1,30 @@ +{ + "id": "547e19561e2e2417d3dde398", + "parameters": { + "cmd": "127.0.0.1 3" + }, + "callback": {}, + "context": { + "user": "stanley" + }, + "result": { + "localhost": { + "failed": false, + "stderr": "", + "return_code": 0, + "succeeded": true, + "stdout": "PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.\n64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.015 ms\n64 bytes from 127.0.0.1: icmp_seq=2 ttl=64 time=0.024 ms\n64 bytes from 127.0.0.1: icmp_seq=3 ttl=64 time=0.030 ms\n\n--- 127.0.0.1 ping statistics ---\n3 packets transmitted, 3 received, 0% packet loss, time 1998ms\nrtt min/avg/max/mdev = 0.015/0.023/0.030/0.006 ms" + } + }, + "status": "succeeded", + "start_timestamp": "2014-12-02T19:56:06.900000Z", + "end_timestamp": "2014-12-02T19:56:07.000000Z", + "action": { + "output_schema": {"result": {"type": "object", "properties": {"test": {"type": "object", "properties": {"item": {"type": "string"}}}}}}, + "ref": "core.ping" + }, + "liveaction": { + "callback": {}, + "id": "1" + } +} diff --git a/st2client/tests/unit/test_formatters.py b/st2client/tests/unit/test_formatters.py index 3562ca660a..43315ca504 100644 --- a/st2client/tests/unit/test_formatters.py +++ b/st2client/tests/unit/test_formatters.py @@ -42,7 +42,8 @@ 'executions': ['execution.json', 'execution_result_has_carriage_return.json', 'execution_unicode.json', - 'execution_with_stack_trace.json'], + 'execution_with_stack_trace.json', + 'execution_with_schema.json'], 'results': ['execution_get_default.txt', 'execution_get_detail.txt', 'execution_get_result_by_key.txt', @@ -53,7 +54,8 @@ 'execution_list_empty_response_start_timestamp_attr.txt', 'execution_unescape_newline.txt', 'execution_unicode.txt', - 'execution_unicode_py3.txt'] + 'execution_unicode_py3.txt', + 'execution_get_has_schema.txt'] } FIXTURES = loader.load_fixtures(fixtures_dict=FIXTURES_MANIFEST) @@ -119,6 +121,11 @@ def test_execution_get_detail(self): content = self._get_execution(argv) self.assertEqual(content, FIXTURES['results']['execution_get_detail.txt']) + def test_execution_with_schema(self): + argv = ['execution', 'get', OUTPUT_SCHEMA['id']] + content = self._get_schema_execution(argv) + self.assertEqual(content, FIXTURES['results']['execution_get_has_schema.txt']) + @mock.patch.object( httpclient.HTTPClient, 'get', mock.MagicMock(return_value=base.FakeResponse(json.dumps(NEWLINE), 200, 'OK', {}))) @@ -233,6 +240,17 @@ def _get_execution(self, argv): return content + @mock.patch.object( + httpclient.HTTPClient, 'get', + mock.MagicMock(return_value=base.FakeResponse(json.dumps(OUTPUT_SCHEMA), 200, 'OK', {}))) + def _get_schema_execution(self, argv): + self.assertEqual(self.shell.run(argv), 0) + self._undo_console_redirect() + with open(self.path, 'r') as fd: + content = fd.read() + + return content + def test_SinlgeRowTable_notebox_one(self): with mock.patch('sys.stderr', new=StringIO()) as fackety_fake: expected = "Note: Only one action execution is displayed. Use -n/--last flag for " \ From 3c13f9b6b007c061d9bc3a9082c339cf41e5b2ca Mon Sep 17 00:00:00 2001 From: bigmstone Date: Wed, 5 Sep 2018 16:03:46 -0500 Subject: [PATCH 09/22] Fixing output_schema for cloudslang runner --- contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml b/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml index 7b9d31b1df..7dbc23831b 100644 --- a/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml +++ b/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml @@ -17,5 +17,5 @@ output_schema: stdout: type: "string" - stdout: + stderr: type: "string" From 7dfc7eed5f9b77aa3f03db9e28430761b084c58f Mon Sep 17 00:00:00 2001 From: bigmstone Date: Mon, 10 Sep 2018 16:03:30 -0500 Subject: [PATCH 10/22] Incorporating feedback. Adding test for failed JSON schema. --- st2actions/st2actions/container/base.py | 19 +++++++++-- .../tests/unit/test_runner_container.py | 34 ++++++++++++++++++- .../actions/action-with-output-schema.yaml | 19 +++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 st2tests/st2tests/fixtures/generic/actions/action-with-output-schema.yaml diff --git a/st2actions/st2actions/container/base.py b/st2actions/st2actions/container/base.py index 9633d02fb2..1399c390cd 100644 --- a/st2actions/st2actions/container/base.py +++ b/st2actions/st2actions/container/base.py @@ -17,7 +17,7 @@ import sys import traceback -import copy +import jsonschema from oslo_config import cfg @@ -36,6 +36,7 @@ from st2common.util.config_loader import ContentPackConfigLoader from st2common.metrics.base import CounterWithTimer from st2common.util import jsonify, schema +from st2common.util.ujson import fast_deepcopy from st2common.runners.base import get_runner from st2common.runners.base import AsyncActionRunner, PollingAsyncActionRunner @@ -147,12 +148,12 @@ def _do_run(self, runner): "additionalProperties": False } - LOG.info("Action Result: %s", result) + LOG.debug("Action Result: %s", result) schema.validate(result, runner_schema, cls=schema.get_validator('custom')) if runner.action.output_schema: - output_schema = copy.deepcopy(runner.runner_type.output_schema) + output_schema = fast_deepcopy(runner.runner_type.output_schema) output_schema.update(runner.action.output_schema) action_schema = { "type": "object", @@ -167,6 +168,18 @@ def _do_run(self, runner): if (isinstance(runner, PollingAsyncActionRunner) and runner.is_polling_enabled() and not action_completed): queries.setup_query(runner.liveaction.id, runner.runner_type, context) + except jsonschema.ValidationError as e: + LOG.exception('Failed to validate output.') + _, ex, tb = sys.exc_info() + # mark execution as failed. + status = action_constants.LIVEACTION_STATUS_FAILED + # include the error message and traceback to try and provide some hints. + result = { + 'error': str(ex), + 'traceback': ''.join(traceback.format_tb(tb, 20)), + 'message': 'Error validating output. See traceback for specific error.', + } + context = None except: LOG.exception('Failed to run action.') _, ex, tb = sys.exc_info() diff --git a/st2actions/tests/unit/test_runner_container.py b/st2actions/tests/unit/test_runner_container.py index ce0cfc63e8..003c5a4418 100644 --- a/st2actions/tests/unit/test_runner_container.py +++ b/st2actions/tests/unit/test_runner_container.py @@ -56,7 +56,8 @@ 'action1.yaml', 'async_action1.yaml', 'async_action2.yaml', - 'action-invalid-runner.yaml' + 'action-invalid-runner.yaml', + 'action-with-output-schema.yaml' ] } @@ -90,6 +91,9 @@ def setUpClass(cls): RunnerContainerTest.async_action_db = models['actions']['async_action1.yaml'] RunnerContainerTest.polling_async_action_db = models['actions']['async_action2.yaml'] RunnerContainerTest.failingaction_db = models['actions']['action-invalid-runner.yaml'] + RunnerContainerTest.schema_output_action_db = models['actions'][ + 'action-with-output-schema.yaml' + ] @classmethod def tearDownClass(cls): @@ -307,6 +311,21 @@ def test_dispatch_runner_failure(self): self.assertTrue('error' in liveaction_db.result) self.assertTrue('traceback' in liveaction_db.result) + def test_output_schema_failure(self): + runner_container = get_runner_container() + params = { + 'cmd': 'echo "test"' + } + liveaction_db = self._get_output_schema_exec_db_model(params) + liveaction_db = LiveAction.add_or_update(liveaction_db) + executions.create_execution_object(liveaction_db) + runner_container.dispatch(liveaction_db) + # pickup updated liveaction_db + liveaction_db = LiveAction.get_by_id(liveaction_db.id) + self.assertTrue('error' in liveaction_db.result) + self.assertTrue('traceback' in liveaction_db.result) + self.assertTrue('message' in liveaction_db.result) + def test_dispatch_override_default_action_params(self): runner_container = get_runner_container() params = { @@ -425,3 +444,16 @@ def _get_failingaction_exec_db_model(self, params): action=action_ref, parameters=parameters, context=context) return liveaction_db + + def _get_output_schema_exec_db_model(self, params): + status = action_constants.LIVEACTION_STATUS_REQUESTED + start_timestamp = date_utils.get_datetime_utc_now() + action_ref = ResourceReference( + name=RunnerContainerTest.schema_output_action_db.name, + pack=RunnerContainerTest.schema_output_action_db.pack).ref + parameters = params + context = {'user': cfg.CONF.system_user.user} + liveaction_db = LiveActionDB(status=status, start_timestamp=start_timestamp, + action=action_ref, parameters=parameters, + context=context) + return liveaction_db diff --git a/st2tests/st2tests/fixtures/generic/actions/action-with-output-schema.yaml b/st2tests/st2tests/fixtures/generic/actions/action-with-output-schema.yaml new file mode 100644 index 0000000000..808243064b --- /dev/null +++ b/st2tests/st2tests/fixtures/generic/actions/action-with-output-schema.yaml @@ -0,0 +1,19 @@ +--- +description: Action that executes an arbitrary Linux command on the localhost. +enabled: true +entry_point: '' +name: local_schema +pack: core +parameters: + cmd: + required: true + sudo: + immutable: true + type: boolean +runner_type: "local-shell-cmd" +output_schema: + stdout: + parameters: + bob: + type: string + type: object From 13955a090e8759ca4689544f33f24f02bd7073a1 Mon Sep 17 00:00:00 2001 From: bigmstone Date: Tue, 11 Sep 2018 17:22:33 -0500 Subject: [PATCH 11/22] Moving schema validation to database update to accomidate various forms of result update. --- conf/st2.conf.sample | 2 + st2actions/st2actions/container/base.py | 50 +---------- .../tests/unit/test_runner_container.py | 15 ---- st2common/st2common/config.py | 5 +- st2common/st2common/util/action_db.py | 9 +- st2common/st2common/util/action_output.py | 86 +++++++++++++++++++ st2common/tests/unit/test_action_db_utils.py | 38 ++++++++ 7 files changed, 139 insertions(+), 66 deletions(-) create mode 100644 st2common/st2common/util/action_output.py diff --git a/conf/st2.conf.sample b/conf/st2.conf.sample index 8d090c4c0f..ffbe232edd 100644 --- a/conf/st2.conf.sample +++ b/conf/st2.conf.sample @@ -309,6 +309,8 @@ facility = local7 debug = False # True to validate parameters for non-system trigger types when creatinga rule. By default, only parameters for system triggers are validated. validate_trigger_parameters = True +# True to validate action and runner output against schema. +validate_output_schema = False # True to validate payload for non-system trigger types when dispatching a trigger inside the sensor. By default, only payload for system triggers is validated. validate_trigger_payload = True # Base path to all st2 artifacts. diff --git a/st2actions/st2actions/container/base.py b/st2actions/st2actions/container/base.py index 1399c390cd..ee231a1a65 100644 --- a/st2actions/st2actions/container/base.py +++ b/st2actions/st2actions/container/base.py @@ -17,7 +17,6 @@ import sys import traceback -import jsonschema from oslo_config import cfg @@ -35,8 +34,7 @@ from st2common.util import param as param_utils from st2common.util.config_loader import ContentPackConfigLoader from st2common.metrics.base import CounterWithTimer -from st2common.util import jsonify, schema -from st2common.util.ujson import fast_deepcopy +from st2common.util import jsonify from st2common.runners.base import get_runner from st2common.runners.base import AsyncActionRunner, PollingAsyncActionRunner @@ -129,57 +127,11 @@ def _do_run(self, runner): (status, result, context) = runner.run(action_params) result = jsonify.try_loads(result) - LOG.debug('Validating runner output: %s', runner.runner_type.output_schema) - - if runner.runner_type.output_schema.get('unmodeled'): - runner.runner_type.output_schema.pop('unmodeled') - LOG.warn( - """Deprecation Notice: This runner has previously had unmodeled - output. In StackStorm 3.1 the output will be placed under the - `output` key.""" - ) - runner_schema = { - "additionalProperties": True - } - else: - runner_schema = { - "type": "object", - "properties": runner.runner_type.output_schema, - "additionalProperties": False - } - - LOG.debug("Action Result: %s", result) - - schema.validate(result, runner_schema, cls=schema.get_validator('custom')) - - if runner.action.output_schema: - output_schema = fast_deepcopy(runner.runner_type.output_schema) - output_schema.update(runner.action.output_schema) - action_schema = { - "type": "object", - "properties": output_schema, - "additionalProperties": False - } - LOG.debug('Validating action output: %s', action_schema) - schema.validate(result, action_schema, cls=schema.get_validator('custom')) - action_completed = status in action_constants.LIVEACTION_COMPLETED_STATES if (isinstance(runner, PollingAsyncActionRunner) and runner.is_polling_enabled() and not action_completed): queries.setup_query(runner.liveaction.id, runner.runner_type, context) - except jsonschema.ValidationError as e: - LOG.exception('Failed to validate output.') - _, ex, tb = sys.exc_info() - # mark execution as failed. - status = action_constants.LIVEACTION_STATUS_FAILED - # include the error message and traceback to try and provide some hints. - result = { - 'error': str(ex), - 'traceback': ''.join(traceback.format_tb(tb, 20)), - 'message': 'Error validating output. See traceback for specific error.', - } - context = None except: LOG.exception('Failed to run action.') _, ex, tb = sys.exc_info() diff --git a/st2actions/tests/unit/test_runner_container.py b/st2actions/tests/unit/test_runner_container.py index 003c5a4418..18888c15aa 100644 --- a/st2actions/tests/unit/test_runner_container.py +++ b/st2actions/tests/unit/test_runner_container.py @@ -311,21 +311,6 @@ def test_dispatch_runner_failure(self): self.assertTrue('error' in liveaction_db.result) self.assertTrue('traceback' in liveaction_db.result) - def test_output_schema_failure(self): - runner_container = get_runner_container() - params = { - 'cmd': 'echo "test"' - } - liveaction_db = self._get_output_schema_exec_db_model(params) - liveaction_db = LiveAction.add_or_update(liveaction_db) - executions.create_execution_object(liveaction_db) - runner_container.dispatch(liveaction_db) - # pickup updated liveaction_db - liveaction_db = LiveAction.get_by_id(liveaction_db.id) - self.assertTrue('error' in liveaction_db.result) - self.assertTrue('traceback' in liveaction_db.result) - self.assertTrue('message' in liveaction_db.result) - def test_dispatch_override_default_action_params(self): runner_container = get_runner_container() params = { diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index c9c1a23fe2..53e23ef52f 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -111,7 +111,10 @@ def register_opts(ignore_errors=False): cfg.BoolOpt( 'validate_trigger_payload', default=True, help='True to validate payload for non-system trigger types when dispatching a trigger ' - 'inside the sensor. By default, only payload for system triggers is validated.') + 'inside the sensor. By default, only payload for system triggers is validated.'), + cfg.BoolOpt( + 'validate_output_schema', default=False, + help='True to validate action and runner output against schema.') ] do_register_opts(system_opts, 'system', ignore_errors) diff --git a/st2common/st2common/util/action_db.py b/st2common/st2common/util/action_db.py index 94cc817a4e..1fa3d3209b 100644 --- a/st2common/st2common/util/action_db.py +++ b/st2common/st2common/util/action_db.py @@ -21,8 +21,9 @@ from collections import OrderedDict -from mongoengine import ValidationError +from oslo_config import cfg import six +from mongoengine import ValidationError from st2common import log as logging from st2common.constants.action import LIVEACTION_STATUSES, LIVEACTION_STATUS_CANCELED @@ -31,6 +32,7 @@ from st2common.persistence.liveaction import LiveAction from st2common.persistence.runner import RunnerType from st2common.metrics.base import get_driver +from st2common.util import action_output LOG = logging.getLogger(__name__) @@ -194,6 +196,11 @@ def update_liveaction_status(status=None, result=None, context=None, end_timesta 'to unknown status string. Unknown status is "%s"', liveaction_db, status) + if result and cfg.CONF.system.validate_output_schema: + action_db = get_action_by_ref(liveaction_db.action) + runner_db = get_runnertype_by_name(action_db.runner_type['name']) + result, status = action_output.validate_output(runner_db, action_db, result, status) + # If liveaction_db status is set then we need to decrement the counter # because it is transitioning to a new state if liveaction_db.status: diff --git a/st2common/st2common/util/action_output.py b/st2common/st2common/util/action_output.py new file mode 100644 index 0000000000..e069bb3d82 --- /dev/null +++ b/st2common/st2common/util/action_output.py @@ -0,0 +1,86 @@ +# Licensed to the StackStorm, Inc ('StackStorm') under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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. +import sys +import traceback +import logging + +import jsonschema + +from st2common.util import schema +from st2common.util.ujson import fast_deepcopy +from st2common.constants import action as action_constants + + +LOG = logging.getLogger(__name__) + + +def _validate_runner(runner, result): + LOG.debug('Validating runner output: %s', runner.output_schema) + + if runner.output_schema.get('unmodeled'): + runner.output_schema.pop('unmodeled') + LOG.warn( + """Deprecation Notice: This runner has previously had unmodeled + output. In StackStorm 3.1 the output will be placed under the + `output` key.""" + ) + runner_schema = { + "additionalProperties": True + } + else: + runner_schema = { + "type": "object", + "properties": runner.output_schema, + "additionalProperties": False + } + + schema.validate(result, runner_schema, cls=schema.get_validator('custom')) + + +def _validate_action(runner_db, action_db, result): + output_schema = fast_deepcopy(runner_db.output_schema) + output_schema.update(action_db.output_schema) + action_schema = { + "type": "object", + "properties": output_schema, + "additionalProperties": False + } + LOG.debug('Validating action output: %s', action_schema) + schema.validate(result, action_schema, cls=schema.get_validator('custom')) + + +def validate_output(runner_db, action_db, result, status): + """ Validate output of action with runner and action schema. + """ + try: + LOG.debug('Validating action output: %s', result) + _validate_runner(runner_db, result) + + if action_db.output_schema: + _validate_action(runner_db, action_db, result) + + except jsonschema.ValidationError as _: + LOG.exception('Failed to validate output.') + _, ex, _ = sys.exc_info() + # mark execution as failed. + status = action_constants.LIVEACTION_STATUS_FAILED + # include the error message and traceback to try and provide some hints. + result = { + 'error': str(ex), + 'message': 'Error validating output. See error output for more details.', + } + return (result, status) + + return (result, status) diff --git a/st2common/tests/unit/test_action_db_utils.py b/st2common/tests/unit/test_action_db_utils.py index 76c298d7af..3132a2ebd7 100644 --- a/st2common/tests/unit/test_action_db_utils.py +++ b/st2common/tests/unit/test_action_db_utils.py @@ -89,6 +89,44 @@ def test_get_actionexec_existing(self): liveaction = action_db_utils.get_liveaction_by_id(ActionDBUtilsTestCase.liveaction_db.id) self.assertEqual(liveaction, ActionDBUtilsTestCase.liveaction_db) + @mock.patch.object(LiveActionPublisher, 'publish_state', mock.MagicMock()) + def test_update_liveaction_with_incorrect_output_schema(self): + liveaction_db = LiveActionDB() + liveaction_db.status = 'initializing' + liveaction_db.start_timestamp = get_datetime_utc_now() + liveaction_db.action = ResourceReference( + name=ActionDBUtilsTestCase.action_db.name, + pack=ActionDBUtilsTestCase.action_db.pack).ref + params = { + 'actionstr': 'foo', + 'some_key_that_aint_exist_in_action_or_runner': 'bar', + 'runnerint': 555 + } + liveaction_db.parameters = params + runner = mock.MagicMock() + runner.output_schema = { + "notaparam": { + "type": "boolean" + } + } + liveaction_db.runner = runner + liveaction_db = LiveAction.add_or_update(liveaction_db) + origliveaction_db = copy.copy(liveaction_db) + + now = get_datetime_utc_now() + status = 'succeeded' + result = 'Work is done.' + context = {'third_party_id': uuid.uuid4().hex} + newliveaction_db = action_db_utils.update_liveaction_status( + status=status, result=result, context=context, end_timestamp=now, + liveaction_id=liveaction_db.id) + + self.assertEqual(origliveaction_db.id, newliveaction_db.id) + self.assertEqual(newliveaction_db.status, status) + self.assertEqual(newliveaction_db.result, result) + self.assertDictEqual(newliveaction_db.context, context) + self.assertEqual(newliveaction_db.end_timestamp, now) + @mock.patch.object(LiveActionPublisher, 'publish_state', mock.MagicMock()) def test_update_liveaction_status(self): liveaction_db = LiveActionDB() From b54f1650e248753e5fb5e31b387ba12b6b4fc566 Mon Sep 17 00:00:00 2001 From: bigmstone Date: Thu, 13 Sep 2018 23:25:58 -0500 Subject: [PATCH 12/22] Adding output_path to runners. Adding testing for action schema. --- conf/st2.dev.conf | 1 + .../action_chain_runner/runner.yaml | 2 + .../announcement_runner/runner.yaml | 1 + .../cloudslang_runner/runner.yaml | 2 + .../http_runner/http_runner/runner.yaml | 2 + .../inquirer_runner/runner.yaml | 1 + .../local_runner/local_runner/runner.yaml | 4 + .../runners/mistral_v2/mistral_v2/runner.yaml | 1 + .../noop_runner/noop_runner/runner.yaml | 2 + .../orquesta_runner/runner.yaml | 2 + .../python_runner/python_runner/runner.yaml | 2 + .../remote_runner/remote_runner/runner.yaml | 2 + .../windows_runner/windows_runner/runner.yaml | 4 + .../winrm_runner/winrm_runner/runner.yaml | 3 + st2common/st2common/util/action_db.py | 17 ++- .../{action_output.py => output_schema.py} | 48 ++++--- st2common/st2common/util/schema/__init__.py | 2 +- .../util/schema/action_output_schema.json | 5 +- .../tests/unit/test_util_output_schema.py | 132 ++++++++++++++++++ 19 files changed, 211 insertions(+), 22 deletions(-) rename st2common/st2common/util/{action_output.py => output_schema.py} (62%) create mode 100644 st2common/tests/unit/test_util_output_schema.py diff --git a/conf/st2.dev.conf b/conf/st2.dev.conf index 21c5afc984..b082d10fb0 100644 --- a/conf/st2.dev.conf +++ b/conf/st2.dev.conf @@ -63,6 +63,7 @@ api_url = http://127.0.0.1:9101/ debug = True base_path = /opt/stackstorm validate_trigger_parameters = True +validate_output_schema = True [garbagecollector] logging = st2reactor/conf/logging.garbagecollector.conf diff --git a/contrib/runners/action_chain_runner/action_chain_runner/runner.yaml b/contrib/runners/action_chain_runner/action_chain_runner/runner.yaml index e3eb1b86b5..b0420cb3a1 100644 --- a/contrib/runners/action_chain_runner/action_chain_runner/runner.yaml +++ b/contrib/runners/action_chain_runner/action_chain_runner/runner.yaml @@ -13,6 +13,8 @@ description: List of tasks to skip notifications for. type: array output_schema: + output_path: + - published published: type: "object" tasks: diff --git a/contrib/runners/announcement_runner/announcement_runner/runner.yaml b/contrib/runners/announcement_runner/announcement_runner/runner.yaml index 937faa70cb..f446021a6b 100644 --- a/contrib/runners/announcement_runner/announcement_runner/runner.yaml +++ b/contrib/runners/announcement_runner/announcement_runner/runner.yaml @@ -17,5 +17,6 @@ minLength: 1 type: string output_schema: + output_path: [] unmodeled: type: boolean diff --git a/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml b/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml index 7dbc23831b..e1f334157c 100644 --- a/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml +++ b/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml @@ -15,6 +15,8 @@ finish in timeout seconds. type: integer output_schema: + output_path: + - stdout stdout: type: "string" stderr: diff --git a/contrib/runners/http_runner/http_runner/runner.yaml b/contrib/runners/http_runner/http_runner/runner.yaml index 6b127bb326..d27ae4be4a 100644 --- a/contrib/runners/http_runner/http_runner/runner.yaml +++ b/contrib/runners/http_runner/http_runner/runner.yaml @@ -39,6 +39,8 @@ is not yet supported. Set to False to skip verification. type: boolean output_schema: + output_path: + - body status_code: type: integer body: diff --git a/contrib/runners/inquirer_runner/inquirer_runner/runner.yaml b/contrib/runners/inquirer_runner/inquirer_runner/runner.yaml index f115fda789..96cdac266d 100644 --- a/contrib/runners/inquirer_runner/inquirer_runner/runner.yaml +++ b/contrib/runners/inquirer_runner/inquirer_runner/runner.yaml @@ -36,6 +36,7 @@ description: Time (in minutes) to wait before timing out the inquiry if no response is received type: integer output_schema: + output_path: [] schema: type: object roles: diff --git a/contrib/runners/local_runner/local_runner/runner.yaml b/contrib/runners/local_runner/local_runner/runner.yaml index ee633d7c37..3b4fc30bab 100644 --- a/contrib/runners/local_runner/local_runner/runner.yaml +++ b/contrib/runners/local_runner/local_runner/runner.yaml @@ -36,6 +36,8 @@ finish in timeout seconds. type: integer output_schema: + output_path: + - stdout succeeded: type: boolean failed: @@ -104,6 +106,8 @@ finish in timeout seconds. type: integer output_schema: + output_path: + - stdout succeeded: type: boolean failed: diff --git a/contrib/runners/mistral_v2/mistral_v2/runner.yaml b/contrib/runners/mistral_v2/mistral_v2/runner.yaml index 9a4fe5d8c2..ad58a9c5bd 100644 --- a/contrib/runners/mistral_v2/mistral_v2/runner.yaml +++ b/contrib/runners/mistral_v2/mistral_v2/runner.yaml @@ -23,5 +23,6 @@ will identify the workflow automatically. type: string output_schema: + output_path: [] unmodeled: type: boolean diff --git a/contrib/runners/noop_runner/noop_runner/runner.yaml b/contrib/runners/noop_runner/noop_runner/runner.yaml index 6062550af9..1d2b6d333b 100644 --- a/contrib/runners/noop_runner/noop_runner/runner.yaml +++ b/contrib/runners/noop_runner/noop_runner/runner.yaml @@ -5,6 +5,8 @@ runner_module: noop_runner runner_parameters: {} output_schema: + output_path: + - stdout stdout: anyOf: - type: "object" diff --git a/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml b/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml index 05c09cc249..1c66ca5f93 100644 --- a/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml +++ b/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml @@ -5,6 +5,8 @@ name: orquesta runner_module: orquesta_runner output_schema: + output_path: + - output errors: anyOf: - type: "object" diff --git a/contrib/runners/python_runner/python_runner/runner.yaml b/contrib/runners/python_runner/python_runner/runner.yaml index 70207bec22..6bc9842044 100644 --- a/contrib/runners/python_runner/python_runner/runner.yaml +++ b/contrib/runners/python_runner/python_runner/runner.yaml @@ -5,6 +5,8 @@ name: python-script runner_module: python_runner output_schema: + output_path: + - result result: anyOf: - type: "object" diff --git a/contrib/runners/remote_runner/remote_runner/runner.yaml b/contrib/runners/remote_runner/remote_runner/runner.yaml index dfa1b3bad2..7effa7a244 100644 --- a/contrib/runners/remote_runner/remote_runner/runner.yaml +++ b/contrib/runners/remote_runner/remote_runner/runner.yaml @@ -86,6 +86,7 @@ required: false type: string output_schema: + output_path: [] unmodeled: type: boolean - aliases: @@ -172,5 +173,6 @@ required: false type: string output_schema: + output_path: [] unmodeled: type: boolean diff --git a/contrib/runners/windows_runner/windows_runner/runner.yaml b/contrib/runners/windows_runner/windows_runner/runner.yaml index b334027ba2..61f4cf668e 100644 --- a/contrib/runners/windows_runner/windows_runner/runner.yaml +++ b/contrib/runners/windows_runner/windows_runner/runner.yaml @@ -29,6 +29,8 @@ required: true type: string output_schema: + output_path: + - stdout stdout: anyOf: - type: "object" @@ -87,6 +89,8 @@ required: true type: string output_schema: + output_path: + - stdout stdout: anyOf: - type: "object" diff --git a/contrib/runners/winrm_runner/winrm_runner/runner.yaml b/contrib/runners/winrm_runner/winrm_runner/runner.yaml index 06f0e2ffa7..04e14710c1 100644 --- a/contrib/runners/winrm_runner/winrm_runner/runner.yaml +++ b/contrib/runners/winrm_runner/winrm_runner/runner.yaml @@ -4,6 +4,7 @@ runner_package: winrm_runner runner_module: winrm_command_runner output_schema: + output_path: [] unmodeled: type: boolean runner_parameters: @@ -75,6 +76,7 @@ runner_package: winrm_runner runner_module: winrm_ps_command_runner output_schema: + output_path: [] unmodeled: type: boolean runner_parameters: @@ -146,6 +148,7 @@ runner_package: winrm_runner runner_module: winrm_ps_script_runner output_schema: + output_path: [] unmodeled: type: boolean runner_parameters: diff --git a/st2common/st2common/util/action_db.py b/st2common/st2common/util/action_db.py index 1fa3d3209b..432a6bc37a 100644 --- a/st2common/st2common/util/action_db.py +++ b/st2common/st2common/util/action_db.py @@ -26,13 +26,17 @@ from mongoengine import ValidationError from st2common import log as logging -from st2common.constants.action import LIVEACTION_STATUSES, LIVEACTION_STATUS_CANCELED +from st2common.constants.action import ( + LIVEACTION_STATUSES, + LIVEACTION_STATUS_CANCELED, + LIVEACTION_STATUS_SUCCEEDED, +) from st2common.exceptions.db import StackStormDBObjectNotFoundError from st2common.persistence.action import Action from st2common.persistence.liveaction import LiveAction from st2common.persistence.runner import RunnerType from st2common.metrics.base import get_driver -from st2common.util import action_output +from st2common.util import output_schema LOG = logging.getLogger(__name__) @@ -196,10 +200,15 @@ def update_liveaction_status(status=None, result=None, context=None, end_timesta 'to unknown status string. Unknown status is "%s"', liveaction_db, status) - if result and cfg.CONF.system.validate_output_schema: + if result and cfg.CONF.system.validate_output_schema and status == LIVEACTION_STATUS_SUCCEEDED: action_db = get_action_by_ref(liveaction_db.action) runner_db = get_runnertype_by_name(action_db.runner_type['name']) - result, status = action_output.validate_output(runner_db, action_db, result, status) + result, status = output_schema.validate_output( + runner_db.output_schema, + action_db.output_schema, + result, + status + ) # If liveaction_db status is set then we need to decrement the counter # because it is transitioning to a new state diff --git a/st2common/st2common/util/action_output.py b/st2common/st2common/util/output_schema.py similarity index 62% rename from st2common/st2common/util/action_output.py rename to st2common/st2common/util/output_schema.py index e069bb3d82..c98fb350df 100644 --- a/st2common/st2common/util/action_output.py +++ b/st2common/st2common/util/output_schema.py @@ -13,24 +13,23 @@ # See the License for the specific language governing permissions and # limitations under the License. import sys -import traceback import logging import jsonschema from st2common.util import schema -from st2common.util.ujson import fast_deepcopy from st2common.constants import action as action_constants LOG = logging.getLogger(__name__) +PATH_KEY = 'output_path' -def _validate_runner(runner, result): - LOG.debug('Validating runner output: %s', runner.output_schema) +def _validate_runner(runner_schema, result): + LOG.debug('Validating runner output: %s', runner_schema) - if runner.output_schema.get('unmodeled'): - runner.output_schema.pop('unmodeled') + if runner_schema.get('unmodeled'): + runner_schema.pop('unmodeled') LOG.warn( """Deprecation Notice: This runner has previously had unmodeled output. In StackStorm 3.1 the output will be placed under the @@ -42,34 +41,39 @@ def _validate_runner(runner, result): else: runner_schema = { "type": "object", - "properties": runner.output_schema, + "properties": runner_schema, "additionalProperties": False } schema.validate(result, runner_schema, cls=schema.get_validator('custom')) -def _validate_action(runner_db, action_db, result): - output_schema = fast_deepcopy(runner_db.output_schema) - output_schema.update(action_db.output_schema) +def _validate_action(action_schema, result, output_path): + final_result = result + output_path = action_schema.pop(PATH_KEY, output_path) + + for key in output_path: + final_result = final_result[key] + action_schema = { "type": "object", - "properties": output_schema, + "properties": action_schema, "additionalProperties": False } LOG.debug('Validating action output: %s', action_schema) - schema.validate(result, action_schema, cls=schema.get_validator('custom')) + schema.validate(final_result, action_schema, cls=schema.get_validator('custom')) -def validate_output(runner_db, action_db, result, status): +def validate_output(runner_schema, action_schema, result, status): """ Validate output of action with runner and action schema. """ try: LOG.debug('Validating action output: %s', result) - _validate_runner(runner_db, result) + output_path = runner_schema.pop(PATH_KEY, []) + _validate_runner(runner_schema, result) - if action_db.output_schema: - _validate_action(runner_db, action_db, result) + if action_schema: + _validate_action(action_schema, result, output_path) except jsonschema.ValidationError as _: LOG.exception('Failed to validate output.') @@ -82,5 +86,17 @@ def validate_output(runner_db, action_db, result, status): 'message': 'Error validating output. See error output for more details.', } return (result, status) + except: + LOG.exception('Failed to validate output.') + _, ex, tb = sys.exc_info() + # mark execution as failed. + status = action_constants.LIVEACTION_STATUS_FAILED + # include the error message and traceback to try and provide some hints. + result = { + 'traceback': str(tb), + 'error': str(ex), + 'message': 'Error validating output. See error output for more details.', + } + return (result, status) return (result, status) diff --git a/st2common/st2common/util/schema/__init__.py b/st2common/st2common/util/schema/__init__.py index 500078bfec..07cf08a70c 100644 --- a/st2common/st2common/util/schema/__init__.py +++ b/st2common/st2common/util/schema/__init__.py @@ -84,7 +84,7 @@ def get_draft_schema(version='custom', additional_properties=False): return schema -def get_action_output_schema(additional_properties=False): +def get_action_output_schema(additional_properties=True): """ Return a generic schema which is used for validating action output. """ diff --git a/st2common/st2common/util/schema/action_output_schema.json b/st2common/st2common/util/schema/action_output_schema.json index 3e92536cea..823e70e1dc 100644 --- a/st2common/st2common/util/schema/action_output_schema.json +++ b/st2common/st2common/util/schema/action_output_schema.json @@ -25,7 +25,10 @@ "uniqueItems": true } }, - "type": "object", + "anyOf": [ + { "type": "object" }, + { "type": "array" } + ], "properties": { "id": { "type": "string", diff --git a/st2common/tests/unit/test_util_output_schema.py b/st2common/tests/unit/test_util_output_schema.py new file mode 100644 index 0000000000..995fcfb313 --- /dev/null +++ b/st2common/tests/unit/test_util_output_schema.py @@ -0,0 +1,132 @@ +# Licensed to the StackStorm, Inc ('StackStorm') under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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. +import copy +import unittest2 + +from st2common.util import output_schema + +from st2common.constants.action import ( + LIVEACTION_STATUS_SUCCEEDED, + LIVEACTION_STATUS_FAILED +) + +ACTION_RESULT = { + 'output': { + 'output_1': 'Bobby', + 'output_2': 5, + 'deep_output': { + 'deep_item_1': 'Jindal', + }, + } +} + +RUNNER_SCHEMA = { + 'output_path': [ + 'output' + ], + 'output': { + 'type': 'object' + }, + 'error': { + 'type': 'array' + }, +} + +ACTION_SCHEMA = { + 'output_1': { + 'type': 'string' + }, + 'output_2': { + 'type': 'integer' + }, + 'deep_output': { + 'type': 'object', + 'parameters': { + 'deep_item_1': { + 'type': 'string', + }, + }, + }, +} + +RUNNER_SCHEMA_FAIL = { + 'not_a_key_you_have': { + 'type': 'string' + }, +} + +ACTION_SCHEMA_FAIL = { + 'not_a_key_you_have': { + 'type': 'string' + }, +} + + +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(ACTION_RESULT), + LIVEACTION_STATUS_SUCCEEDED + ) + + self.assertEqual(result, ACTION_RESULT) + self.assertEqual(status, LIVEACTION_STATUS_SUCCEEDED) + + def test_invalid_runner_schema(self): + result, status = output_schema.validate_output( + copy.deepcopy(RUNNER_SCHEMA_FAIL), + copy.deepcopy(ACTION_SCHEMA), + copy.deepcopy(ACTION_RESULT), + LIVEACTION_STATUS_SUCCEEDED + ) + + 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}}" + ), + 'message': 'Error validating output. See error output for more details.' + } + + self.assertEqual(result, expected_result) + self.assertEqual(status, LIVEACTION_STATUS_FAILED) + + def test_invalid_action_schema(self): + result, status = output_schema.validate_output( + copy.deepcopy(RUNNER_SCHEMA), + copy.deepcopy(ACTION_SCHEMA_FAIL), + copy.deepcopy(ACTION_RESULT), + LIVEACTION_STATUS_SUCCEEDED + ) + + expected_result = { + 'error': + "Additional properties are not allowed ('deep_output', 'output_2', " + "'output_1' were unexpected)\n\nFailed validating 'additionalProper" + "ties' in schema:\n {'additionalProperties': False,\n 'prope" + "rties': {'not_a_key_you_have': {'type': 'string'}},\n 'type': " + "'object'}\n\nOn instance:\n {'deep_output': {'deep_item_1': 'Ji" + "ndal'},\n 'output_1': 'Bobby',\n 'output_2': 5}", + 'message': 'Error validating output. See error output for more details.' + } + + self.assertEqual(result, expected_result) + self.assertEqual(status, LIVEACTION_STATUS_FAILED) From 39c6164730a8afeeb4fa41e6edddf98ef4b444cc Mon Sep 17 00:00:00 2001 From: bigmstone Date: Fri, 14 Sep 2018 08:30:59 -0500 Subject: [PATCH 13/22] Removing unmodeled key in favor of complete absense of output schema in runner --- .../announcement_runner/runner.yaml | 4 --- .../runners/mistral_v2/mistral_v2/runner.yaml | 4 --- .../remote_runner/remote_runner/runner.yaml | 8 ------ .../winrm_runner/winrm_runner/runner.yaml | 12 -------- st2client/tests/unit/test_action.py | 2 -- st2common/st2common/util/output_schema.py | 28 ++++++------------- .../fixtures/generic/runners/actionchain.yaml | 3 -- .../fixtures/generic/runners/inquirer.yaml | 3 -- .../generic/runners/testasyncrunner1.yaml | 3 -- .../generic/runners/testasyncrunner2.yaml | 3 -- .../generic/runners/testfailingrunner1.yaml | 3 -- .../fixtures/generic/runners/testrunner1.yaml | 3 -- .../fixtures/generic/runners/testrunner2.yaml | 3 -- .../fixtures/generic/runners/testrunner3.yaml | 3 -- 14 files changed, 9 insertions(+), 73 deletions(-) diff --git a/contrib/runners/announcement_runner/announcement_runner/runner.yaml b/contrib/runners/announcement_runner/announcement_runner/runner.yaml index f446021a6b..d84e6a4465 100644 --- a/contrib/runners/announcement_runner/announcement_runner/runner.yaml +++ b/contrib/runners/announcement_runner/announcement_runner/runner.yaml @@ -16,7 +16,3 @@ maxLength: 255 minLength: 1 type: string - output_schema: - output_path: [] - unmodeled: - type: boolean diff --git a/contrib/runners/mistral_v2/mistral_v2/runner.yaml b/contrib/runners/mistral_v2/mistral_v2/runner.yaml index ad58a9c5bd..d2404b3507 100644 --- a/contrib/runners/mistral_v2/mistral_v2/runner.yaml +++ b/contrib/runners/mistral_v2/mistral_v2/runner.yaml @@ -22,7 +22,3 @@ If entry point is a workflow or a workbook with a single workflow, the runner will identify the workflow automatically. type: string - output_schema: - output_path: [] - unmodeled: - type: boolean diff --git a/contrib/runners/remote_runner/remote_runner/runner.yaml b/contrib/runners/remote_runner/remote_runner/runner.yaml index 7effa7a244..b690c3ad7a 100644 --- a/contrib/runners/remote_runner/remote_runner/runner.yaml +++ b/contrib/runners/remote_runner/remote_runner/runner.yaml @@ -85,10 +85,6 @@ config is used. required: false type: string - output_schema: - output_path: [] - unmodeled: - type: boolean - aliases: - run-remote-script description: A remote execution runner that executes actions as a fixed system user. @@ -172,7 +168,3 @@ config is used. required: false type: string - output_schema: - output_path: [] - unmodeled: - type: boolean diff --git a/contrib/runners/winrm_runner/winrm_runner/runner.yaml b/contrib/runners/winrm_runner/winrm_runner/runner.yaml index 04e14710c1..88a938c37f 100644 --- a/contrib/runners/winrm_runner/winrm_runner/runner.yaml +++ b/contrib/runners/winrm_runner/winrm_runner/runner.yaml @@ -3,10 +3,6 @@ name: winrm-cmd runner_package: winrm_runner runner_module: winrm_command_runner - output_schema: - output_path: [] - unmodeled: - type: boolean runner_parameters: cmd: description: Arbitrary Command Prompt command to be executed on the remote host. @@ -75,10 +71,6 @@ name: winrm-ps-cmd runner_package: winrm_runner runner_module: winrm_ps_command_runner - output_schema: - output_path: [] - unmodeled: - type: boolean runner_parameters: cmd: description: Arbitrary PowerShell command to be executed on the remote host. @@ -147,10 +139,6 @@ name: winrm-ps-script runner_package: winrm_runner runner_module: winrm_ps_script_runner - output_schema: - output_path: [] - unmodeled: - type: boolean runner_parameters: cwd: description: Working directory where the command will be executed in diff --git a/st2client/tests/unit/test_action.py b/st2client/tests/unit/test_action.py index a6ad6a3796..ea2bc7db33 100644 --- a/st2client/tests/unit/test_action.py +++ b/st2client/tests/unit/test_action.py @@ -37,7 +37,6 @@ "str": {"type": "string"} }, "name": "mock-runner1", - "output_schema": {"unmodeled": {"type": "boolean"}} } ACTION1 = { @@ -54,7 +53,6 @@ "enabled": True, "runner_parameters": {}, "name": "mock-runner2", - "output_schema": {"unmodeled": {"type": "boolean"}} } ACTION2 = { diff --git a/st2common/st2common/util/output_schema.py b/st2common/st2common/util/output_schema.py index c98fb350df..896bcbfa6e 100644 --- a/st2common/st2common/util/output_schema.py +++ b/st2common/st2common/util/output_schema.py @@ -28,27 +28,17 @@ def _validate_runner(runner_schema, result): LOG.debug('Validating runner output: %s', runner_schema) - if runner_schema.get('unmodeled'): - runner_schema.pop('unmodeled') - LOG.warn( - """Deprecation Notice: This runner has previously had unmodeled - output. In StackStorm 3.1 the output will be placed under the - `output` key.""" - ) - runner_schema = { - "additionalProperties": True - } - else: - runner_schema = { - "type": "object", - "properties": runner_schema, - "additionalProperties": False - } + runner_schema = { + "type": "object", + "properties": runner_schema, + "additionalProperties": False + } schema.validate(result, runner_schema, cls=schema.get_validator('custom')) def _validate_action(action_schema, result, output_path): + LOG.debug('Validating action output: %s', action_schema) final_result = result output_path = action_schema.pop(PATH_KEY, output_path) @@ -60,7 +50,6 @@ def _validate_action(action_schema, result, output_path): "properties": action_schema, "additionalProperties": False } - LOG.debug('Validating action output: %s', action_schema) schema.validate(final_result, action_schema, cls=schema.get_validator('custom')) @@ -69,8 +58,9 @@ def validate_output(runner_schema, action_schema, result, status): """ try: LOG.debug('Validating action output: %s', result) - output_path = runner_schema.pop(PATH_KEY, []) - _validate_runner(runner_schema, result) + if runner_schema: + output_path = runner_schema.pop(PATH_KEY, []) + _validate_runner(runner_schema, result) if action_schema: _validate_action(action_schema, result, output_path) diff --git a/st2tests/st2tests/fixtures/generic/runners/actionchain.yaml b/st2tests/st2tests/fixtures/generic/runners/actionchain.yaml index 55ba56a0be..935115ec83 100644 --- a/st2tests/st2tests/fixtures/generic/runners/actionchain.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/actionchain.yaml @@ -4,6 +4,3 @@ enabled: true name: action-chain runner_module: action_chain_runner runner_parameters: {} -output_schema: - unmodeled: - type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/inquirer.yaml b/st2tests/st2tests/fixtures/generic/runners/inquirer.yaml index 89e856715f..5147d6ae32 100644 --- a/st2tests/st2tests/fixtures/generic/runners/inquirer.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/inquirer.yaml @@ -36,6 +36,3 @@ runner_parameters: required: true description: Time (in minutes) that an unacknowledged Inquiry is cleaned up type: integer -output_schema: - unmodeled: - type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/testasyncrunner1.yaml b/st2tests/st2tests/fixtures/generic/runners/testasyncrunner1.yaml index 9ceacfd55e..cff11068c6 100644 --- a/st2tests/st2tests/fixtures/generic/runners/testasyncrunner1.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/testasyncrunner1.yaml @@ -20,6 +20,3 @@ runner_parameters: default: defaultfoo description: Foo str param. type: string -output_schema: - unmodeled: - type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/testasyncrunner2.yaml b/st2tests/st2tests/fixtures/generic/runners/testasyncrunner2.yaml index cd7bc14659..713328c365 100644 --- a/st2tests/st2tests/fixtures/generic/runners/testasyncrunner2.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/testasyncrunner2.yaml @@ -21,6 +21,3 @@ runner_parameters: default: defaultfoo description: Foo str param. type: string -output_schema: - unmodeled: - type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/testfailingrunner1.yaml b/st2tests/st2tests/fixtures/generic/runners/testfailingrunner1.yaml index 8c8d00c7f0..c680773ba5 100644 --- a/st2tests/st2tests/fixtures/generic/runners/testfailingrunner1.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/testfailingrunner1.yaml @@ -9,6 +9,3 @@ runner_parameters: description: Foo str param. immutable: true type: boolean -output_schema: - unmodeled: - type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/testrunner1.yaml b/st2tests/st2tests/fixtures/generic/runners/testrunner1.yaml index be72ccfacf..725b5c033e 100644 --- a/st2tests/st2tests/fixtures/generic/runners/testrunner1.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/testrunner1.yaml @@ -31,6 +31,3 @@ runner_parameters: default: defaultfoo description: Foo str param. type: string -output_schema: - unmodeled: - type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/testrunner2.yaml b/st2tests/st2tests/fixtures/generic/runners/testrunner2.yaml index c1ea55f046..8810fda9b8 100644 --- a/st2tests/st2tests/fixtures/generic/runners/testrunner2.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/testrunner2.yaml @@ -4,6 +4,3 @@ enabled: true name: test-runner-2 runner_module: runner runner_parameters: {} -output_schema: - unmodeled: - type: boolean diff --git a/st2tests/st2tests/fixtures/generic/runners/testrunner3.yaml b/st2tests/st2tests/fixtures/generic/runners/testrunner3.yaml index cfb5a64280..bb9db6555e 100644 --- a/st2tests/st2tests/fixtures/generic/runners/testrunner3.yaml +++ b/st2tests/st2tests/fixtures/generic/runners/testrunner3.yaml @@ -12,6 +12,3 @@ runner_parameters: required: true k3: type: string -output_schema: - unmodeled: - type: boolean From 8b1fa7d11121ec171fbbd5fad790f69c755e37bd Mon Sep 17 00:00:00 2001 From: bigmstone Date: Fri, 14 Sep 2018 22:47:31 -0500 Subject: [PATCH 14/22] Removing output path in favor of output key --- .../action_chain_runner/runner.yaml | 3 +- .../cloudslang_runner/runner.yaml | 3 +- .../http_runner/http_runner/runner.yaml | 3 +- .../inquirer_runner/runner.yaml | 12 ----- .../local_runner/local_runner/runner.yaml | 54 ------------------- .../noop_runner/noop_runner/runner.yaml | 27 ---------- .../orquesta_runner/runner.yaml | 3 +- .../python_runner/python_runner/runner.yaml | 3 +- .../windows_runner/windows_runner/runner.yaml | 54 ------------------- st2client/st2client/formatters/execution.py | 9 +++- st2common/st2common/models/api/action.py | 8 ++- st2common/st2common/models/db/runner.py | 2 + st2common/st2common/util/action_db.py | 3 +- st2common/st2common/util/output_schema.py | 20 ++++--- .../util/schema/action_output_schema.json | 5 +- .../tests/unit/test_util_output_schema.py | 14 ++--- 16 files changed, 42 insertions(+), 181 deletions(-) diff --git a/contrib/runners/action_chain_runner/action_chain_runner/runner.yaml b/contrib/runners/action_chain_runner/action_chain_runner/runner.yaml index b0420cb3a1..e30470a148 100644 --- a/contrib/runners/action_chain_runner/action_chain_runner/runner.yaml +++ b/contrib/runners/action_chain_runner/action_chain_runner/runner.yaml @@ -12,9 +12,8 @@ default: [] description: List of tasks to skip notifications for. type: array + output_key: published output_schema: - output_path: - - published published: type: "object" tasks: diff --git a/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml b/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml index e1f334157c..7bcd86e3db 100644 --- a/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml +++ b/contrib/runners/cloudslang_runner/cloudslang_runner/runner.yaml @@ -14,9 +14,8 @@ description: Action timeout in seconds. Action will get killed if it doesn't finish in timeout seconds. type: integer + output_key: stdout output_schema: - output_path: - - stdout stdout: type: "string" stderr: diff --git a/contrib/runners/http_runner/http_runner/runner.yaml b/contrib/runners/http_runner/http_runner/runner.yaml index d27ae4be4a..5fde87895a 100644 --- a/contrib/runners/http_runner/http_runner/runner.yaml +++ b/contrib/runners/http_runner/http_runner/runner.yaml @@ -38,9 +38,8 @@ CA bundle which comes from Mozilla. Verification using a custom CA bundle is not yet supported. Set to False to skip verification. type: boolean + output_key: body output_schema: - output_path: - - body status_code: type: integer body: diff --git a/contrib/runners/inquirer_runner/inquirer_runner/runner.yaml b/contrib/runners/inquirer_runner/inquirer_runner/runner.yaml index 96cdac266d..e9819118a0 100644 --- a/contrib/runners/inquirer_runner/inquirer_runner/runner.yaml +++ b/contrib/runners/inquirer_runner/inquirer_runner/runner.yaml @@ -35,15 +35,3 @@ required: true description: Time (in minutes) to wait before timing out the inquiry if no response is received type: integer - output_schema: - output_path: [] - schema: - type: object - roles: - type: array - users: - type: array - route: - type: string - ttl: - type: integer diff --git a/contrib/runners/local_runner/local_runner/runner.yaml b/contrib/runners/local_runner/local_runner/runner.yaml index 3b4fc30bab..598193e5fe 100644 --- a/contrib/runners/local_runner/local_runner/runner.yaml +++ b/contrib/runners/local_runner/local_runner/runner.yaml @@ -35,33 +35,6 @@ description: Action timeout in seconds. Action will get killed if it doesn't finish in timeout seconds. type: integer - output_schema: - output_path: - - stdout - succeeded: - type: boolean - failed: - type: boolean - return_code: - type: integer - stderr: - anyOf: - - type: "object" - - type: "string" - - type: "integer" - - type: "number" - - type: "boolean" - - type: "array" - - type: "null" - stdout: - anyOf: - - type: "object" - - type: "string" - - type: "integer" - - type: "number" - - type: "boolean" - - type: "array" - - type: "null" - aliases: - run-local-script description: A runner to execute local actions as a fixed user. @@ -105,30 +78,3 @@ description: Action timeout in seconds. Action will get killed if it doesn't finish in timeout seconds. type: integer - output_schema: - output_path: - - stdout - succeeded: - type: boolean - failed: - type: boolean - return_code: - type: integer - stderr: - anyOf: - - type: "object" - - type: "string" - - type: "integer" - - type: "number" - - type: "boolean" - - type: "array" - - type: "null" - stdout: - anyOf: - - type: "object" - - type: "string" - - type: "integer" - - type: "number" - - type: "boolean" - - type: "array" - - type: "null" diff --git a/contrib/runners/noop_runner/noop_runner/runner.yaml b/contrib/runners/noop_runner/noop_runner/runner.yaml index 1d2b6d333b..eeeed20eeb 100644 --- a/contrib/runners/noop_runner/noop_runner/runner.yaml +++ b/contrib/runners/noop_runner/noop_runner/runner.yaml @@ -4,30 +4,3 @@ name: noop runner_module: noop_runner runner_parameters: {} - output_schema: - output_path: - - stdout - stdout: - anyOf: - - type: "object" - - type: "string" - - type: "integer" - - type: "number" - - type: "boolean" - - type: "array" - - type: "null" - stderr: - anyOf: - - type: "object" - - type: "string" - - type: "integer" - - type: "number" - - type: "boolean" - - type: "array" - - type: "null" - failed: - type: boolean - succeeded: - type: boolean - return_code: - type: integer diff --git a/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml b/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml index 1c66ca5f93..6ea34033c8 100644 --- a/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml +++ b/contrib/runners/orquesta_runner/orquesta_runner/runner.yaml @@ -4,9 +4,8 @@ enabled: true name: orquesta runner_module: orquesta_runner + output_key: output output_schema: - output_path: - - output errors: anyOf: - type: "object" diff --git a/contrib/runners/python_runner/python_runner/runner.yaml b/contrib/runners/python_runner/python_runner/runner.yaml index 6bc9842044..1dfd16e680 100644 --- a/contrib/runners/python_runner/python_runner/runner.yaml +++ b/contrib/runners/python_runner/python_runner/runner.yaml @@ -4,9 +4,8 @@ enabled: true name: python-script runner_module: python_runner + output_key: result output_schema: - output_path: - - result result: anyOf: - type: "object" diff --git a/contrib/runners/windows_runner/windows_runner/runner.yaml b/contrib/runners/windows_runner/windows_runner/runner.yaml index 61f4cf668e..caff847bff 100644 --- a/contrib/runners/windows_runner/windows_runner/runner.yaml +++ b/contrib/runners/windows_runner/windows_runner/runner.yaml @@ -28,33 +28,6 @@ description: Username used to log-in. required: true type: string - output_schema: - output_path: - - stdout - stdout: - anyOf: - - type: "object" - - type: "string" - - type: "integer" - - type: "number" - - type: "boolean" - - type: "array" - - type: "null" - stderr: - anyOf: - - type: "object" - - type: "string" - - type: "integer" - - type: "number" - - type: "boolean" - - type: "array" - - type: "null" - return_code: - type: integer - succeeded: - type: boolean - failed: - type: boolean - aliases: [] description: A remote execution runner that executes power shell scripts on Windows hosts. @@ -88,30 +61,3 @@ description: Username used to log-in. required: true type: string - output_schema: - output_path: - - stdout - stdout: - anyOf: - - type: "object" - - type: "string" - - type: "integer" - - type: "number" - - type: "boolean" - - type: "array" - - type: "null" - stderr: - anyOf: - - type: "object" - - type: "string" - - type: "integer" - - type: "number" - - type: "boolean" - - type: "array" - - type: "null" - return_code: - type: integer - succeeded: - type: boolean - failed: - type: boolean diff --git a/st2client/st2client/formatters/execution.py b/st2client/st2client/formatters/execution.py index ea2ae583c2..fb00d2058e 100644 --- a/st2client/st2client/formatters/execution.py +++ b/st2client/st2client/formatters/execution.py @@ -60,9 +60,16 @@ def format(cls, entry, *args, **kwargs): for attr in attrs: value = jsutil.get_value(entry, attr) value = strutil.strip_carriage_returns(strutil.unescape(value)) + # TODO: This check is inherently flawed since it will crash st2client + # if the leading character is objectish start and last character is objectish + # end but the string isn't supposed to be a object. Try/Except will catch + # this for now, but this should be improved. if (isinstance(value, six.string_types) and len(value) > 0 and value[0] in ['{', '['] and value[len(value) - 1] in ['}', ']']): - new_value = ast.literal_eval(value) + try: + new_value = ast.literal_eval(value) + except: + new_value = value if type(new_value) in [dict, list]: value = new_value if type(value) in [dict, list]: diff --git a/st2common/st2common/models/api/action.py b/st2common/st2common/models/api/action.py index 60fdcf599c..2c523a5070 100644 --- a/st2common/st2common/models/api/action.py +++ b/st2common/st2common/models/api/action.py @@ -110,6 +110,11 @@ class RunnerTypeAPI(BaseAPI): }, 'additionalProperties': False }, + "output_key": { + "description": "Default key to expect results to be published to.", + "type": "string", + "required": False + }, "output_schema": { "description": "Schema for the runner's output.", "type": "object", @@ -144,13 +149,14 @@ def to_model(cls, runner_type): runner_package = getattr(runner_type, 'runner_package', runner_type.runner_module) runner_module = str(runner_type.runner_module) runner_parameters = getattr(runner_type, 'runner_parameters', dict()) + output_key = getattr(runner_type, 'output_key', None) output_schema = getattr(runner_type, 'output_schema', dict()) query_module = getattr(runner_type, 'query_module', None) model = cls.model(name=name, description=description, enabled=enabled, runner_package=runner_package, runner_module=runner_module, runner_parameters=runner_parameters, output_schema=output_schema, - query_module=query_module) + query_module=query_module, output_key=output_key) return model diff --git a/st2common/st2common/models/db/runner.py b/st2common/st2common/models/db/runner.py index 9ed74640d9..9308cc0346 100644 --- a/st2common/st2common/models/db/runner.py +++ b/st2common/st2common/models/db/runner.py @@ -60,6 +60,8 @@ class RunnerTypeDB(stormbase.StormBaseDB, stormbase.UIDFieldMixin): help_text='The python module that implements the action runner for this type.') runner_parameters = me.DictField( help_text='The specification for parameters for the action runner.') + output_key = me.StringField( + help_text='Default key to expect results to be published to.') output_schema = me.DictField( help_text='The schema for runner output.') query_module = me.StringField( diff --git a/st2common/st2common/util/action_db.py b/st2common/st2common/util/action_db.py index 432a6bc37a..415fdb4e5f 100644 --- a/st2common/st2common/util/action_db.py +++ b/st2common/st2common/util/action_db.py @@ -207,7 +207,8 @@ def update_liveaction_status(status=None, result=None, context=None, end_timesta runner_db.output_schema, action_db.output_schema, result, - status + status, + runner_db.output_key, ) # If liveaction_db status is set then we need to decrement the counter diff --git a/st2common/st2common/util/output_schema.py b/st2common/st2common/util/output_schema.py index 896bcbfa6e..2ae04f9b22 100644 --- a/st2common/st2common/util/output_schema.py +++ b/st2common/st2common/util/output_schema.py @@ -16,13 +16,13 @@ import logging import jsonschema +import traceback from st2common.util import schema from st2common.constants import action as action_constants LOG = logging.getLogger(__name__) -PATH_KEY = 'output_path' def _validate_runner(runner_schema, result): @@ -37,33 +37,31 @@ def _validate_runner(runner_schema, result): schema.validate(result, runner_schema, cls=schema.get_validator('custom')) -def _validate_action(action_schema, result, output_path): +def _validate_action(action_schema, result, output_key): LOG.debug('Validating action output: %s', action_schema) - final_result = result - output_path = action_schema.pop(PATH_KEY, output_path) - for key in output_path: - final_result = final_result[key] + final_result = result[output_key] action_schema = { "type": "object", "properties": action_schema, "additionalProperties": False } + schema.validate(final_result, action_schema, cls=schema.get_validator('custom')) -def validate_output(runner_schema, action_schema, result, status): +def validate_output(runner_schema, action_schema, result, status, output_key): """ Validate output of action with runner and action schema. """ try: LOG.debug('Validating action output: %s', result) + LOG.debug('Output Key: %s', output_key) if runner_schema: - output_path = runner_schema.pop(PATH_KEY, []) _validate_runner(runner_schema, result) - if action_schema: - _validate_action(action_schema, result, output_path) + if action_schema: + _validate_action(action_schema, result, output_key) except jsonschema.ValidationError as _: LOG.exception('Failed to validate output.') @@ -83,7 +81,7 @@ def validate_output(runner_schema, action_schema, result, status): status = action_constants.LIVEACTION_STATUS_FAILED # include the error message and traceback to try and provide some hints. result = { - 'traceback': str(tb), + 'traceback': ''.join(traceback.format_tb(tb, 20)), 'error': str(ex), 'message': 'Error validating output. See error output for more details.', } diff --git a/st2common/st2common/util/schema/action_output_schema.json b/st2common/st2common/util/schema/action_output_schema.json index 823e70e1dc..3e92536cea 100644 --- a/st2common/st2common/util/schema/action_output_schema.json +++ b/st2common/st2common/util/schema/action_output_schema.json @@ -25,10 +25,7 @@ "uniqueItems": true } }, - "anyOf": [ - { "type": "object" }, - { "type": "array" } - ], + "type": "object", "properties": { "id": { "type": "string", diff --git a/st2common/tests/unit/test_util_output_schema.py b/st2common/tests/unit/test_util_output_schema.py index 995fcfb313..58bc6e54fd 100644 --- a/st2common/tests/unit/test_util_output_schema.py +++ b/st2common/tests/unit/test_util_output_schema.py @@ -33,9 +33,6 @@ } RUNNER_SCHEMA = { - 'output_path': [ - 'output' - ], 'output': { 'type': 'object' }, @@ -73,6 +70,8 @@ }, } +OUTPUT_KEY = 'output' + class OutputSchemaTestCase(unittest2.TestCase): def test_valid_schema(self): @@ -80,7 +79,8 @@ def test_valid_schema(self): copy.deepcopy(RUNNER_SCHEMA), copy.deepcopy(ACTION_SCHEMA), copy.deepcopy(ACTION_RESULT), - LIVEACTION_STATUS_SUCCEEDED + LIVEACTION_STATUS_SUCCEEDED, + OUTPUT_KEY, ) self.assertEqual(result, ACTION_RESULT) @@ -91,7 +91,8 @@ def test_invalid_runner_schema(self): copy.deepcopy(RUNNER_SCHEMA_FAIL), copy.deepcopy(ACTION_SCHEMA), copy.deepcopy(ACTION_RESULT), - LIVEACTION_STATUS_SUCCEEDED + LIVEACTION_STATUS_SUCCEEDED, + OUTPUT_KEY, ) expected_result = { @@ -114,7 +115,8 @@ def test_invalid_action_schema(self): copy.deepcopy(RUNNER_SCHEMA), copy.deepcopy(ACTION_SCHEMA_FAIL), copy.deepcopy(ACTION_RESULT), - LIVEACTION_STATUS_SUCCEEDED + LIVEACTION_STATUS_SUCCEEDED, + OUTPUT_KEY, ) expected_result = { From 9a774603f57716152e5a84bd3b736a5b7eec4b01 Mon Sep 17 00:00:00 2001 From: bigmstone Date: Sat, 15 Sep 2018 20:19:01 -0500 Subject: [PATCH 15/22] Adding output schema check for python and orquesta runner --- .../tests/unit/test_output_schema.py | 175 ++++++++++++++++++ .../tests/unit/test_output_schema.py | 115 ++++++++++++ .../tests/unit/test_runner_container.py | 6 +- st2common/st2common/util/output_schema.py | 2 +- .../tests/unit/test_util_output_schema.py | 15 +- st2tests/st2tests/base.py | 19 +- st2tests/st2tests/config.py | 1 + .../actions/action-with-output-schema.yaml | 19 -- .../sequential_with_broken_schema.yaml | 15 ++ .../actions/sequential_with_schema.yaml | 15 ++ 10 files changed, 342 insertions(+), 40 deletions(-) create mode 100644 contrib/runners/orquesta_runner/tests/unit/test_output_schema.py create mode 100644 contrib/runners/python_runner/tests/unit/test_output_schema.py delete mode 100644 st2tests/st2tests/fixtures/generic/actions/action-with-output-schema.yaml create mode 100644 st2tests/st2tests/fixtures/packs/orquesta_tests/actions/sequential_with_broken_schema.yaml create mode 100644 st2tests/st2tests/fixtures/packs/orquesta_tests/actions/sequential_with_schema.yaml diff --git a/contrib/runners/orquesta_runner/tests/unit/test_output_schema.py b/contrib/runners/orquesta_runner/tests/unit/test_output_schema.py new file mode 100644 index 0000000000..af666c9909 --- /dev/null +++ b/contrib/runners/orquesta_runner/tests/unit/test_output_schema.py @@ -0,0 +1,175 @@ +# Licensed to the StackStorm, Inc ('StackStorm') under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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 os + +import mock + +import st2tests + +# XXX: actionsensor import depends on config being setup. +import st2tests.config as tests_config +tests_config.parse_args() + +from tests.unit import base + +from st2common.bootstrap import actionsregistrar +from st2common.bootstrap import runnersregistrar +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 +from st2common.persistence import workflow as wf_db_access +from st2common.runners import base as runners +from st2common.services import action as ac_svc +from st2common.services import workflows as wf_svc +from st2common.transport import liveaction as lv_ac_xport +from st2common.transport import workflow as wf_ex_xport +from st2common.transport import publishers +from st2common.constants import action as ac_const +from st2tests.mocks import liveaction as mock_lv_ac_xport +from st2tests.mocks import workflow as mock_wf_ex_xport +from st2tests.base import RunnerTestCase + +BASE_DIR = os.path.dirname(os.path.abspath(__file__)) + +TEST_PACK = 'orquesta_tests' +TEST_PACK_PATH = st2tests.fixturesloader.get_fixtures_packs_base_path() + '/' + TEST_PACK + +PACKS = [ + TEST_PACK_PATH, + st2tests.fixturesloader.get_fixtures_packs_base_path() + '/core' +] + +FAIL_SCHEMA = { + "notvalid": { + "type": "string", + }, +} + + +@mock.patch.object( + publishers.CUDPublisher, + 'publish_update', + mock.MagicMock(return_value=None)) +@mock.patch.object( + lv_ac_xport.LiveActionPublisher, + '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)) +@mock.patch.object( + wf_ex_xport.WorkflowExecutionPublisher, + 'publish_create', + mock.MagicMock(side_effect=mock_wf_ex_xport.MockWorkflowExecutionPublisher.publish_create)) +@mock.patch.object( + wf_ex_xport.WorkflowExecutionPublisher, + 'publish_state', + mock.MagicMock(side_effect=mock_wf_ex_xport.MockWorkflowExecutionPublisher.publish_state)) +class OrquestaRunnerTest(RunnerTestCase, st2tests.DbTestCase): + @classmethod + def setUpClass(cls): + super(OrquestaRunnerTest, 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) + + @classmethod + def get_runner_class(cls, runner_name): + return runners.get_runner(runner_name, runner_name).__class__ + + def test_ahearence_to_output_schema(self): + wf_meta = base.get_wf_fixture_meta_data(TEST_PACK_PATH, 'sequential_with_schema.yaml') + wf_input = {'who': 'Thanos'} + 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) + lv_ac_db = lv_db_access.LiveAction.get_by_id(str(lv_ac_db.id)) + wf_ex_dbs = wf_db_access.WorkflowExecution.query(action_execution=str(ac_ex_db.id)) + wf_ex_db = wf_ex_dbs[0] + 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] + wf_svc.handle_action_execution_completion(tk1_ac_ex_db) + tk1_ex_db = wf_db_access.TaskExecution.get_by_id(tk1_ex_db.id) + wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) + 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] + wf_svc.handle_action_execution_completion(tk2_ac_ex_db) + tk2_ex_db = wf_db_access.TaskExecution.get_by_id(tk2_ex_db.id) + wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) + 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] + wf_svc.handle_action_execution_completion(tk3_ac_ex_db) + wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) + lv_ac_db = lv_db_access.LiveAction.get_by_id(str(lv_ac_db.id)) + ac_ex_db = ex_db_access.ActionExecution.get_by_id(str(ac_ex_db.id)) + + self.assertEqual(lv_ac_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) + self.assertEqual(ac_ex_db.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) + + def test_fail_incorrect_output_schema(self): + wf_meta = base.get_wf_fixture_meta_data( + TEST_PACK_PATH, + 'sequential_with_broken_schema.yaml' + ) + wf_input = {'who': 'Thanos'} + 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) + lv_ac_db = lv_db_access.LiveAction.get_by_id(str(lv_ac_db.id)) + wf_ex_dbs = wf_db_access.WorkflowExecution.query(action_execution=str(ac_ex_db.id)) + wf_ex_db = wf_ex_dbs[0] + 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] + wf_svc.handle_action_execution_completion(tk1_ac_ex_db) + tk1_ex_db = wf_db_access.TaskExecution.get_by_id(tk1_ex_db.id) + wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) + 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] + wf_svc.handle_action_execution_completion(tk2_ac_ex_db) + tk2_ex_db = wf_db_access.TaskExecution.get_by_id(tk2_ex_db.id) + wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) + 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] + wf_svc.handle_action_execution_completion(tk3_ac_ex_db) + wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) + lv_ac_db = lv_db_access.LiveAction.get_by_id(str(lv_ac_db.id)) + ac_ex_db = ex_db_access.ActionExecution.get_by_id(str(ac_ex_db.id)) + + self.assertEqual(lv_ac_db.status, ac_const.LIVEACTION_STATUS_FAILED) + self.assertEqual(ac_ex_db.status, ac_const.LIVEACTION_STATUS_FAILED) + + expected_result = { + 'error': "Additional properties are not allowed", + 'message': 'Error validating output. See error output for more details.' + } + + self.assertIn(expected_result['error'], ac_ex_db.result['error']) + self.assertEqual(expected_result['message'], ac_ex_db.result['message']) diff --git a/contrib/runners/python_runner/tests/unit/test_output_schema.py b/contrib/runners/python_runner/tests/unit/test_output_schema.py new file mode 100644 index 0000000000..cca134a548 --- /dev/null +++ b/contrib/runners/python_runner/tests/unit/test_output_schema.py @@ -0,0 +1,115 @@ +# Licensed to the StackStorm, Inc ('StackStorm') under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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 os +import sys + +import mock +import jsonschema + +from python_runner import python_runner +from st2common.constants.action import LIVEACTION_STATUS_SUCCEEDED +from st2common.constants.pack import SYSTEM_PACK_NAME +from st2common.util import output_schema +from st2tests.base import RunnerTestCase +from st2tests.base import CleanDbTestCase +from st2tests.fixturesloader import assert_submodules_are_checked_out +import st2tests.base as tests_base + + +BASE_DIR = os.path.dirname(os.path.abspath(__file__)) + +PASCAL_ROW_ACTION_PATH = os.path.join(tests_base.get_resources_path(), 'packs', + 'pythonactions/actions/pascal_row.py') + +MOCK_SYS = mock.Mock() +MOCK_SYS.argv = [] +MOCK_SYS.executable = sys.executable + +MOCK_EXECUTION = mock.Mock() +MOCK_EXECUTION.id = '598dbf0c0640fd54bffc688b' + +FAIL_SCHEMA = { + "notvalid": { + "type": "string", + }, +} + + +@mock.patch('python_runner.python_runner.sys', MOCK_SYS) +class PythonRunnerTestCase(RunnerTestCase, CleanDbTestCase): + register_packs = True + register_pack_configs = True + + @classmethod + def setUpClass(cls): + super(PythonRunnerTestCase, cls).setUpClass() + assert_submodules_are_checked_out() + + def test_ahearence_to_output_schema(self): + config = self.loader(os.path.join(BASE_DIR, '../../runner.yaml')) + runner = self._get_mock_runner_obj() + runner.entry_point = PASCAL_ROW_ACTION_PATH + runner.pre_run() + (status, output, _) = runner.run({'row_index': 5}) + output_schema._validate_runner( + config[0]['output_schema'], + output + ) + self.assertEqual(status, LIVEACTION_STATUS_SUCCEEDED) + self.assertTrue(output is not None) + self.assertEqual(output['result'], [1, 5, 10, 10, 5, 1]) + + def test_fail_incorrect_output_schema(self): + runner = self._get_mock_runner_obj() + runner.entry_point = PASCAL_ROW_ACTION_PATH + runner.pre_run() + (status, output, _) = runner.run({'row_index': 5}) + with self.assertRaises(jsonschema.ValidationError): + output_schema._validate_runner( + FAIL_SCHEMA, + output + ) + + def _get_mock_runner_obj(self, pack=None, sandbox=None): + runner = python_runner.get_runner() + runner.execution = MOCK_EXECUTION + runner.action = self._get_mock_action_obj() + runner.runner_parameters = {} + + if pack: + runner.action.pack = pack + + if sandbox is not None: + runner._sandbox = sandbox + + return runner + + def _get_mock_action_obj(self): + """ + Return mock action object. + + Pack gets set to the system pack so the action doesn't require a separate virtualenv. + """ + action = mock.Mock() + action.ref = 'dummy.action' + action.pack = SYSTEM_PACK_NAME + action.entry_point = 'foo.py' + action.runner_type = { + 'name': 'python-script' + } + return action diff --git a/st2actions/tests/unit/test_runner_container.py b/st2actions/tests/unit/test_runner_container.py index 18888c15aa..5c046ababa 100644 --- a/st2actions/tests/unit/test_runner_container.py +++ b/st2actions/tests/unit/test_runner_container.py @@ -56,8 +56,7 @@ 'action1.yaml', 'async_action1.yaml', 'async_action2.yaml', - 'action-invalid-runner.yaml', - 'action-with-output-schema.yaml' + 'action-invalid-runner.yaml' ] } @@ -91,9 +90,6 @@ def setUpClass(cls): RunnerContainerTest.async_action_db = models['actions']['async_action1.yaml'] RunnerContainerTest.polling_async_action_db = models['actions']['async_action2.yaml'] RunnerContainerTest.failingaction_db = models['actions']['action-invalid-runner.yaml'] - RunnerContainerTest.schema_output_action_db = models['actions'][ - 'action-with-output-schema.yaml' - ] @classmethod def tearDownClass(cls): diff --git a/st2common/st2common/util/output_schema.py b/st2common/st2common/util/output_schema.py index 2ae04f9b22..1f3b851e2f 100644 --- a/st2common/st2common/util/output_schema.py +++ b/st2common/st2common/util/output_schema.py @@ -15,8 +15,8 @@ import sys import logging -import jsonschema import traceback +import jsonschema from st2common.util import schema from st2common.constants import action as action_constants diff --git a/st2common/tests/unit/test_util_output_schema.py b/st2common/tests/unit/test_util_output_schema.py index 58bc6e54fd..1321a36d1b 100644 --- a/st2common/tests/unit/test_util_output_schema.py +++ b/st2common/tests/unit/test_util_output_schema.py @@ -120,15 +120,12 @@ def test_invalid_action_schema(self): ) expected_result = { - 'error': - "Additional properties are not allowed ('deep_output', 'output_2', " - "'output_1' were unexpected)\n\nFailed validating 'additionalProper" - "ties' in schema:\n {'additionalProperties': False,\n 'prope" - "rties': {'not_a_key_you_have': {'type': 'string'}},\n 'type': " - "'object'}\n\nOn instance:\n {'deep_output': {'deep_item_1': 'Ji" - "ndal'},\n 'output_1': 'Bobby',\n 'output_2': 5}", - 'message': 'Error validating output. See error output for more details.' + 'error': "Additional properties are not allowed", + 'message': u'Error validating output. See error output for more details.' } - self.assertEqual(result, expected_result) + # To avoid random failures (especially in python3) this assert cant be + # exact since the parameters can be ordered differently per execution. + self.assertIn(expected_result['error'], result['error']) + self.assertEqual(result['message'], expected_result['message']) self.assertEqual(status, LIVEACTION_STATUS_FAILED) diff --git a/st2tests/st2tests/base.py b/st2tests/st2tests/base.py index 62d3a1436c..a6b4480e75 100644 --- a/st2tests/st2tests/base.py +++ b/st2tests/st2tests/base.py @@ -49,7 +49,13 @@ from st2common.bootstrap.base import ResourceRegistrar from st2common.bootstrap.configsregistrar import ConfigsRegistrar from st2common.content.utils import get_packs_base_paths +from st2common.content.loader import MetaLoader from st2common.exceptions.db import StackStormDBObjectNotFoundError +from st2common.persistence import execution as ex_db_access +from st2common.persistence import workflow as wf_db_access +from st2common.services import workflows as wf_svc +from st2common.util import api as api_util +from st2common.util import loader import st2common.models.db.rule as rule_model import st2common.models.db.rule_enforcement as rule_enforcement_model import st2common.models.db.sensor as sensor_model @@ -62,11 +68,6 @@ import st2common.models.db.liveaction as liveaction_model import st2common.models.db.actionalias as actionalias_model import st2common.models.db.policy as policy_model -from st2common.persistence import execution as ex_db_access -from st2common.persistence import workflow as wf_db_access -from st2common.services import workflows as wf_svc -from st2common.util import api as api_util -from st2common.util import loader import st2tests.config # Imports for backward compatibility (those classes have been moved to standalone modules) @@ -118,6 +119,8 @@ class RunnerTestCase(unittest2.TestCase): + meta_loader = MetaLoader() + def assertCommonSt2EnvVarsAvailableInEnv(self, env): """ Method which asserts that the common ST2 environment variables are present in the provided @@ -125,10 +128,14 @@ def assertCommonSt2EnvVarsAvailableInEnv(self, env): """ for var_name in COMMON_ACTION_ENV_VARIABLES: self.assertTrue(var_name in env) - self.assertEqual(env['ST2_ACTION_API_URL'], get_full_public_api_url()) self.assertTrue(env[AUTH_TOKEN_ENV_VARIABLE_NAME] is not None) + def loader(self, path): + """ Load the runner config + """ + return self.meta_loader.load(path) + class BaseTestCase(TestCase): diff --git a/st2tests/st2tests/config.py b/st2tests/st2tests/config.py index 99f4acbda4..d54eb64c37 100644 --- a/st2tests/st2tests/config.py +++ b/st2tests/st2tests/config.py @@ -75,6 +75,7 @@ def _override_common_opts(): packs_base_path = get_fixtures_packs_base_path() runners_base_path = get_fixtures_runners_base_path() CONF.set_override(name='base_path', override=packs_base_path, group='system') + CONF.set_override(name='validate_output_schema', override=True, group='system') CONF.set_override(name='system_packs_base_path', override=packs_base_path, group='content') CONF.set_override(name='packs_base_paths', override=packs_base_path, group='content') CONF.set_override(name='system_runners_base_path', override=runners_base_path, group='content') diff --git a/st2tests/st2tests/fixtures/generic/actions/action-with-output-schema.yaml b/st2tests/st2tests/fixtures/generic/actions/action-with-output-schema.yaml deleted file mode 100644 index 808243064b..0000000000 --- a/st2tests/st2tests/fixtures/generic/actions/action-with-output-schema.yaml +++ /dev/null @@ -1,19 +0,0 @@ ---- -description: Action that executes an arbitrary Linux command on the localhost. -enabled: true -entry_point: '' -name: local_schema -pack: core -parameters: - cmd: - required: true - sudo: - immutable: true - type: boolean -runner_type: "local-shell-cmd" -output_schema: - stdout: - parameters: - bob: - type: string - type: object diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/sequential_with_broken_schema.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/sequential_with_broken_schema.yaml new file mode 100644 index 0000000000..81b3a38289 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/sequential_with_broken_schema.yaml @@ -0,0 +1,15 @@ +--- +name: sequential_with_broken_schema +description: A basic sequential workflow. +pack: orquesta_tests +runner_type: orquesta +entry_point: workflows/sequential.yaml +enabled: true +parameters: + who: + required: true + type: string + default: Stanley +output_schema: + notakey: + type: boolean diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/sequential_with_schema.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/sequential_with_schema.yaml new file mode 100644 index 0000000000..cd0d5b8b34 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/sequential_with_schema.yaml @@ -0,0 +1,15 @@ +--- +name: sequential_with_schema +description: A basic sequential workflow. +pack: orquesta_tests +runner_type: orquesta +entry_point: workflows/sequential.yaml +enabled: true +parameters: + who: + required: true + type: string + default: Stanley +output_schema: + msg: + type: string From bb0153242877978699fa09e80d739e933071768b Mon Sep 17 00:00:00 2001 From: stanley Date: Sun, 16 Sep 2018 20:11:16 +0000 Subject: [PATCH 16/22] Update changelog info for release - 2.9.0 --- CHANGELOG.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 91203b95db..be6691b726 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,6 +4,10 @@ Changelog in development -------------- + +2.9.0 - September 16, 2018 +-------------------------- + Added ~~~~~ From 667b301f93c0480679f978b2a5ea72fc8f778eb5 Mon Sep 17 00:00:00 2001 From: Lindsay Hill Date: Mon, 17 Sep 2018 16:46:12 +1000 Subject: [PATCH 17/22] WIP Option to skip Mistral & Orquesta tests --- st2common/bin/st2-self-check | 50 ++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/st2common/bin/st2-self-check b/st2common/bin/st2-self-check index be3d0fdad2..fd51af7ee6 100755 --- a/st2common/bin/st2-self-check +++ b/st2common/bin/st2-self-check @@ -19,21 +19,31 @@ function usage() { echo "Usage: $0" echo "" echo "Options:" + echo " -o Skip Orquesta tests" + echo " -m Skip Mistral tests" echo " -w Run Windows tests" echo " -b Which branch of st2tests to use (defaults to master)" echo "" >&2 } +RUN_ORQUESTA_TESTS=true +RUN_MISTRAL_TESTS=true RUN_WINDOWS_TESTS=false ST2_TESTS_BRANCH="master" -while getopts "b:w" o +while getopts "b:wom" o do case "${o}" in b) ST2_TESTS_BRANCH=${OPTARG} ;; + o) + RUN_ORQUESTA_TESTS=false + ;; + m) + RUN_MISTRAL_TESTS=false + ;; w) RUN_WINDOWS_TESTS=true ;; @@ -114,6 +124,12 @@ do continue fi + # Skip Mistral Inquiry test if we're not running Mistral tests + if [ ${RUN_MISTRAL_TESTS} = "false" ] && [ ${TEST} = "tests.test_inquiry_mistral" ]; then + echo "Skipping ${TEST}..." + continue + fi + echo -n "Attempting Test ${TEST}..." st2 run ${TEST} protocol=${PROTOCOL} token=${ST2_AUTH_TOKEN} | grep "status" | grep -q "succeeded" if [ $? -ne 0 ]; then @@ -124,22 +140,30 @@ do fi done -echo -n "Attempting Example examples.mistral_examples..." -st2 run examples.mistral_examples | grep "status" | grep -q "succeeded" -if [ $? -ne 0 ]; then - echo -e "ERROR!" - ((ERRORS++)) +if [ ${RUN_MISTRAL_TESTS} = "true" ]; then + echo -n "Attempting Example examples.mistral_examples..." + st2 run examples.mistral_examples | grep "status" | grep -q "succeeded" + if [ $? -ne 0 ]; then + echo -e "ERROR!" + ((ERRORS++)) + else + echo "OK!" + fi else - echo "OK!" + echo "Skipping examples.mistral_examples..." fi -echo -n "Attempting Example examples.orquesta-examples..." -st2 run examples.orquesta-examples | grep "status" | grep -q "succeeded" -if [ $? -ne 0 ]; then - echo -e "ERROR!" - ((ERRORS++)) +if [ ${RUN_ORQUESTA_TESTS} = "true" ]; then + echo -n "Attempting Example examples.orquesta-examples..." + st2 run examples.orquesta-examples | grep "status" | grep -q "succeeded" + if [ $? -ne 0 ]; then + echo -e "ERROR!" + ((ERRORS++)) + else + echo "OK!" + fi else - echo "OK!" + echo "Skipping examples.orquesta-examples..." fi if [ $ERRORS -ne 0 ]; then From 365645d9a97d209b1afed3ee91e0275b6cacc321 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 17 Sep 2018 18:13:42 +0200 Subject: [PATCH 18/22] Include line break before the table. --- st2client/st2client/formatters/execution.py | 1 + 1 file changed, 1 insertion(+) diff --git a/st2client/st2client/formatters/execution.py b/st2client/st2client/formatters/execution.py index fb00d2058e..ca65d32d54 100644 --- a/st2client/st2client/formatters/execution.py +++ b/st2client/st2client/formatters/execution.py @@ -102,6 +102,7 @@ def format(cls, entry, *args, **kwargs): } rendered_schema = yaml.safe_dump(rendered_schema, default_flow_style=False) + output += '\n' output += _print_bordered( "This action does not have an output schema defined. Based " "on the action output the following inferred schema was built:" From 5595c0a3b010ec13e19a412f3e2807a9e25dcd53 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 17 Sep 2018 18:17:27 +0200 Subject: [PATCH 19/22] Pass yaml kwargs to result formatter and don't print "no output_schema" warning when --yaml flag is used (use wants machine readable format). --- st2client/st2client/commands/action.py | 1 + 1 file changed, 1 insertion(+) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index b28aa6fc64..51b6a58953 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -360,6 +360,7 @@ def _print_execution_details(self, execution, args, **kwargs): options = {'attributes': attr} options['json'] = args.json + options['yaml'] = args.yaml options['with_schema'] = args.with_schema options['attribute_transform_functions'] = self.attribute_transform_functions self.print_output(instance, formatter, **options) From b12122d89a1fe5e4dc1ea262d4bc20eaa4838d28 Mon Sep 17 00:00:00 2001 From: bigmstone Date: Mon, 17 Sep 2018 17:27:26 -0500 Subject: [PATCH 20/22] Adding option to ignore schema output warning in st2client and defaulting to silent for 2.9 --- st2client/st2client/config_parser.py | 5 +++++ st2client/st2client/formatters/execution.py | 8 ++++---- st2client/tests/fixtures/execution_get_attributes.txt | 2 -- st2client/tests/fixtures/execution_get_default.txt | 2 -- st2client/tests/fixtures/execution_unescape_newline.txt | 2 -- st2client/tests/fixtures/execution_unicode.txt | 2 -- st2client/tests/fixtures/execution_unicode_py3.txt | 2 -- st2client/tests/unit/test_config_parser.py | 3 ++- 8 files changed, 11 insertions(+), 15 deletions(-) diff --git a/st2client/st2client/config_parser.py b/st2client/st2client/config_parser.py index c627c68a9a..46afda564d 100644 --- a/st2client/st2client/config_parser.py +++ b/st2client/st2client/config_parser.py @@ -29,6 +29,7 @@ import six from six.moves.configparser import ConfigParser + __all__ = [ 'CLIConfigParser', @@ -60,6 +61,10 @@ 'silence_ssl_warnings': { 'type': 'bool', 'default': False + }, + 'silence_schema_output': { + 'type': 'bool', + 'default': True } }, 'cli': { diff --git a/st2client/st2client/formatters/execution.py b/st2client/st2client/formatters/execution.py index ca65d32d54..ecd5f1dd9d 100644 --- a/st2client/st2client/formatters/execution.py +++ b/st2client/st2client/formatters/execution.py @@ -22,6 +22,7 @@ import yaml from st2client import formatters +from st2client.config import get_config from st2client.utils import jsutil from st2client.utils import strutil from st2client.utils.color import DisplayColors @@ -95,7 +96,7 @@ def format(cls, entry, *args, **kwargs): (DisplayColors.colorize(attr, DisplayColors.BLUE), value) output_schema = entry.get('action', {}).get('output_schema') - + schema_check = get_config()['general']['silence_schema_output'] if not output_schema and kwargs.get('with_schema'): rendered_schema = { 'output_schema': schema.render_output_schema_from_output(entry['result']) @@ -104,12 +105,11 @@ def format(cls, entry, *args, **kwargs): rendered_schema = yaml.safe_dump(rendered_schema, default_flow_style=False) output += '\n' output += _print_bordered( - "This action does not have an output schema defined. Based " - "on the action output the following inferred schema was built:" + "Based on the action output the following inferred schema was built:" "\n\n" "%s" % rendered_schema ) - elif not output_schema: + elif not output_schema and not schema_check: output += ( "\n\n** This action does not have an output_schema. " "Run again with --with-schema to see a suggested schema." diff --git a/st2client/tests/fixtures/execution_get_attributes.txt b/st2client/tests/fixtures/execution_get_attributes.txt index 1d7664aa77..8f898c3906 100644 --- a/st2client/tests/fixtures/execution_get_attributes.txt +++ b/st2client/tests/fixtures/execution_get_attributes.txt @@ -1,4 +1,2 @@ status: succeeded (1s elapsed) end_timestamp: Tue, 02 Dec 2014 19:56:07 UTC - -** This action does not have an output_schema. Run again with --with-schema to see a suggested schema. diff --git a/st2client/tests/fixtures/execution_get_default.txt b/st2client/tests/fixtures/execution_get_default.txt index 79ef717eea..2613b61f2c 100644 --- a/st2client/tests/fixtures/execution_get_default.txt +++ b/st2client/tests/fixtures/execution_get_default.txt @@ -16,5 +16,3 @@ result: 3 packets transmitted, 3 received, 0% packet loss, time 1998ms rtt min/avg/max/mdev = 0.015/0.023/0.030/0.006 ms' succeeded: true - -** This action does not have an output_schema. Run again with --with-schema to see a suggested schema. diff --git a/st2client/tests/fixtures/execution_unescape_newline.txt b/st2client/tests/fixtures/execution_unescape_newline.txt index 9bb2865cca..40ab18deca 100644 --- a/st2client/tests/fixtures/execution_unescape_newline.txt +++ b/st2client/tests/fixtures/execution_unescape_newline.txt @@ -11,5 +11,3 @@ Traceback (most recent call last): stdout: 'FOOBAR2 ' succeeded: true - -** This action does not have an output_schema. Run again with --with-schema to see a suggested schema. diff --git a/st2client/tests/fixtures/execution_unicode.txt b/st2client/tests/fixtures/execution_unicode.txt index 5aedcde8f9..f7209afaa9 100644 --- a/st2client/tests/fixtures/execution_unicode.txt +++ b/st2client/tests/fixtures/execution_unicode.txt @@ -8,5 +8,3 @@ result: stderr: '' stdout: "‡" succeeded: true - -** This action does not have an output_schema. Run again with --with-schema to see a suggested schema. diff --git a/st2client/tests/fixtures/execution_unicode_py3.txt b/st2client/tests/fixtures/execution_unicode_py3.txt index 9550d14f28..ba96a99346 100644 --- a/st2client/tests/fixtures/execution_unicode_py3.txt +++ b/st2client/tests/fixtures/execution_unicode_py3.txt @@ -8,5 +8,3 @@ result: stderr: '' stdout: "\u2021" succeeded: true - -** This action does not have an output_schema. Run again with --with-schema to see a suggested schema. diff --git a/st2client/tests/unit/test_config_parser.py b/st2client/tests/unit/test_config_parser.py index 3d11c6b5f8..9bbc8ccc59 100644 --- a/st2client/tests/unit/test_config_parser.py +++ b/st2client/tests/unit/test_config_parser.py @@ -52,7 +52,8 @@ def test_parse(self): 'base_url': 'http://127.0.0.1', 'api_version': 'v1', 'cacert': 'cacartpath', - 'silence_ssl_warnings': False + 'silence_ssl_warnings': False, + 'silence_schema_output': True }, 'cli': { 'debug': True, From 537e4f03fd75b9f9bc5d8ef1d48dc4227595f842 Mon Sep 17 00:00:00 2001 From: Lindsay Hill Date: Tue, 18 Sep 2018 13:52:23 +1000 Subject: [PATCH 21/22] Added changelog entry --- CHANGELOG.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index be6691b726..4784935548 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,6 +4,12 @@ Changelog in development -------------- +Added +~~~~~ + +* Added ``-o`` and ``-m`` CLI options to ``st2-self-check`` script, to skip Orquesta and/or Mistral + tests (#4347) + 2.9.0 - September 16, 2018 -------------------------- From 7e18ae55d16ef080ed2010e495c3e0bff59558c0 Mon Sep 17 00:00:00 2001 From: Lindsay Hill Date: Fri, 21 Sep 2018 11:56:25 +1000 Subject: [PATCH 22/22] Remove outdated st2actions README --- st2actions/README.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 st2actions/README.md diff --git a/st2actions/README.md b/st2actions/README.md deleted file mode 100644 index e7113501c8..0000000000 --- a/st2actions/README.md +++ /dev/null @@ -1,5 +0,0 @@ -Action Runner -============= - -See: https://stackstorm.atlassian.net/wiki/display/STORM/Demo+-+ActionRunner - https://stackstorm.atlassian.net/wiki/display/STORM/API+Design