From 6b9cf386d8c3f7793c1b3a4e9b28b3c56b31ca41 Mon Sep 17 00:00:00 2001 From: rebrowning Date: Thu, 21 Dec 2023 17:32:47 -0800 Subject: [PATCH 1/8] add a generic output schema to the local, remote, and winrm runners. St2 already appears to return the data in this format, this is simply formalizing the response so that validations can be added for commands leveraging these runners to better allow users of st2 to verify the integrity of the data their pipelines are processing --- .../local_runner/local_runner/runner.yaml | 40 +++++++++++++ .../remote_runner/remote_runner/runner.yaml | 40 +++++++++++++ .../winrm_runner/winrm_runner/runner.yaml | 60 +++++++++++++++++++ 3 files changed, 140 insertions(+) diff --git a/contrib/runners/local_runner/local_runner/runner.yaml b/contrib/runners/local_runner/local_runner/runner.yaml index 7463f46943..2a56edda99 100644 --- a/contrib/runners/local_runner/local_runner/runner.yaml +++ b/contrib/runners/local_runner/local_runner/runner.yaml @@ -33,6 +33,26 @@ description: Action timeout in seconds. Action will get killed if it doesn't finish in timeout seconds. type: integer + output_key: stdout + output_schema: + type: object + properties: + succeeded: + type: boolean + required: true + failed: + type: boolean + required: true + stdout: + anyOf: + - type: "object" + - type: "string" + required: true + stderr: + type: string + required: true + return_code: + type: integer - description: A runner to execute local actions as a fixed user. enabled: true name: local-shell-script @@ -74,3 +94,23 @@ description: Action timeout in seconds. Action will get killed if it doesn't finish in timeout seconds. type: integer + output_key: stdout + output_schema: + type: object + properties: + succeeded: + type: boolean + required: true + failed: + type: boolean + required: true + stdout: + anyOf: + - type: "object" + - type: "string" + required: true + stderr: + type: string + required: true + return_code: + type: integer \ No newline at end of file diff --git a/contrib/runners/remote_runner/remote_runner/runner.yaml b/contrib/runners/remote_runner/remote_runner/runner.yaml index 138973e218..3eaed94192 100644 --- a/contrib/runners/remote_runner/remote_runner/runner.yaml +++ b/contrib/runners/remote_runner/remote_runner/runner.yaml @@ -82,6 +82,26 @@ config is used. required: false type: string + output_key: stdout + output_schema: + type: object + properties: + succeeded: + type: boolean + required: true + failed: + type: boolean + required: true + stdout: + anyOf: + - type: "object" + - type: "string" + required: true + stderr: + type: string + required: true + return_code: + type: integer - description: A remote execution runner that executes actions as a fixed system user. enabled: true name: remote-shell-script @@ -162,3 +182,23 @@ config is used. required: false type: string + output_key: stdout + output_schema: + type: object + properties: + succeeded: + type: boolean + required: true + failed: + type: boolean + required: true + stdout: + anyOf: + - type: "object" + - type: "string" + required: true + stderr: + type: string + required: true + return_code: + type: integer \ No newline at end of file diff --git a/contrib/runners/winrm_runner/winrm_runner/runner.yaml b/contrib/runners/winrm_runner/winrm_runner/runner.yaml index 88a938c37f..7e1c47b475 100644 --- a/contrib/runners/winrm_runner/winrm_runner/runner.yaml +++ b/contrib/runners/winrm_runner/winrm_runner/runner.yaml @@ -66,6 +66,26 @@ 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: stdout + output_schema: + type: object + properties: + succeeded: + type: boolean + required: true + failed: + type: boolean + required: true + stdout: + anyOf: + - type: "object" + - type: "string" + required: true + stderr: + type: string + required: true + return_code: + type: integer - description: A remote execution runner that executes PowerShell commands via WinRM on a remote host enabled: true name: winrm-ps-cmd @@ -134,6 +154,26 @@ 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: stdout + output_schema: + type: object + properties: + succeeded: + type: boolean + required: true + failed: + type: boolean + required: true + stdout: + anyOf: + - type: "object" + - type: "string" + required: true + stderr: + type: string + required: true + return_code: + type: integer - 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 +239,23 @@ 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: stdout + output_schema: + type: object + properties: + succeeded: + type: boolean + required: true + failed: + type: boolean + required: true + stdout: + anyOf: + - type: "object" + - type: "string" + required: true + stderr: + type: string + required: true + return_code: + type: integer \ No newline at end of file From 74ef49ef72b554556e0db129f8d5e19223e5f06f Mon Sep 17 00:00:00 2001 From: rebrowning Date: Thu, 21 Dec 2023 22:19:27 -0800 Subject: [PATCH 2/8] updated several tests that were not returning a result that matches what st2 is currently outputting, so they were failing with the new schema being applied --- .../orquesta_runner/tests/unit/test_rerun.py | 2 +- .../tests/unit/test_with_items.py | 4 +- st2actions/tests/unit/test_policies.py | 37 +++++++++++++++++ .../unit/controllers/v1/test_executions.py | 40 +++++++++++-------- .../unit/services/test_workflow_rerun.py | 2 +- st2tests/st2tests/mocks/runners/runner.py | 18 ++++++++- 6 files changed, 80 insertions(+), 23 deletions(-) diff --git a/contrib/runners/orquesta_runner/tests/unit/test_rerun.py b/contrib/runners/orquesta_runner/tests/unit/test_rerun.py index 420b909e27..261f54812a 100644 --- a/contrib/runners/orquesta_runner/tests/unit/test_rerun.py +++ b/contrib/runners/orquesta_runner/tests/unit/test_rerun.py @@ -57,7 +57,7 @@ ) RUNNER_RESULT_SUCCEEDED = ( action_constants.LIVEACTION_STATUS_SUCCEEDED, - {"stdout": "foobar"}, + {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}, {}, ) diff --git a/contrib/runners/orquesta_runner/tests/unit/test_with_items.py b/contrib/runners/orquesta_runner/tests/unit/test_with_items.py index 8e8b67bd94..7a1d1ec549 100644 --- a/contrib/runners/orquesta_runner/tests/unit/test_with_items.py +++ b/contrib/runners/orquesta_runner/tests/unit/test_with_items.py @@ -56,13 +56,13 @@ RUNNER_RESULT_RUNNING = ( action_constants.LIVEACTION_STATUS_RUNNING, - {"stdout": "..."}, + {"stdout": "...", "stderr": ""}, {}, ) RUNNER_RESULT_SUCCEEDED = ( action_constants.LIVEACTION_STATUS_SUCCEEDED, - {"stdout": "..."}, + {"stdout": "...", "stderr": "", "succeeded": True, "failed": False}, {}, ) diff --git a/st2actions/tests/unit/test_policies.py b/st2actions/tests/unit/test_policies.py index a2c828b39b..29951d9cde 100644 --- a/st2actions/tests/unit/test_policies.py +++ b/st2actions/tests/unit/test_policies.py @@ -128,6 +128,43 @@ def test_apply(self): FakeConcurrencyApplicator.apply_after.assert_called_once_with(liveaction) RaiseExceptionApplicator.apply_after.assert_called_once_with(liveaction) + @mock.patch.object( + FakeConcurrencyApplicator, + "apply_before", + mock.MagicMock( + side_effect=FakeConcurrencyApplicator(None, None, threshold=3).apply_before + ), + ) + @mock.patch.object( + RaiseExceptionApplicator, + "apply_before", + mock.MagicMock(side_effect=RaiseExceptionApplicator(None, None).apply_before), + ) + @mock.patch.object( + FakeConcurrencyApplicator, + "apply_after", + mock.MagicMock( + side_effect=FakeConcurrencyApplicator(None, None, threshold=3).apply_after + ), + ) + @mock.patch.object( + RaiseExceptionApplicator, + "apply_after", + mock.MagicMock(side_effect=RaiseExceptionApplicator(None, None).apply_after), + ) + def test_apply_with_dict(self): + liveaction = LiveActionDB( + action="wolfpack.action-1", parameters={"actionstr": "dict_resp"} + ) + liveaction, _ = action_service.request(liveaction) + liveaction = self._wait_on_status( + liveaction, action_constants.LIVEACTION_STATUS_SUCCEEDED + ) + FakeConcurrencyApplicator.apply_before.assert_called_once_with(liveaction) + RaiseExceptionApplicator.apply_before.assert_called_once_with(liveaction) + FakeConcurrencyApplicator.apply_after.assert_called_once_with(liveaction) + RaiseExceptionApplicator.apply_after.assert_called_once_with(liveaction) + @mock.patch.object( FakeConcurrencyApplicator, "get_threshold", mock.MagicMock(return_value=0) ) diff --git a/st2api/tests/unit/controllers/v1/test_executions.py b/st2api/tests/unit/controllers/v1/test_executions.py index eb3face2ba..ae647c40d4 100644 --- a/st2api/tests/unit/controllers/v1/test_executions.py +++ b/st2api/tests/unit/controllers/v1/test_executions.py @@ -451,7 +451,7 @@ def test_get_one_max_result_size_query_parameter(self): # Update it with the result (this populates result and result size attributes) data = { - "result": {"fooo": "a" * 1000}, + "result": {"stdout": {"fooo": "a" * 1000}, "succeeded": True, "failed": False, "stderr": ""}, "status": "succeeded", } actual_result_size = len(json_encode(data["result"])) @@ -1281,16 +1281,16 @@ def test_put_status_and_result(self): self.assertEqual(post_resp.status_int, 201) execution_id = self._get_actionexecution_id(post_resp) - updates = {"status": "succeeded", "result": {"stdout": "foobar"}} + updates = {"status": "succeeded", "result": {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}} put_resp = self._do_put(execution_id, updates) self.assertEqual(put_resp.status_int, 200) self.assertEqual(put_resp.json["status"], "succeeded") - self.assertDictEqual(put_resp.json["result"], {"stdout": "foobar"}) + self.assertDictEqual(put_resp.json["result"], {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}) get_resp = self._do_get_one(execution_id) self.assertEqual(get_resp.status_int, 200) self.assertEqual(get_resp.json["status"], "succeeded") - self.assertDictEqual(get_resp.json["result"], {"stdout": "foobar"}) + self.assertDictEqual(get_resp.json["result"], {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}) def test_put_bad_state(self): post_resp = self._do_post(LIVE_ACTION_1) @@ -1329,11 +1329,11 @@ def test_put_status_to_completed_execution(self): self.assertEqual(post_resp.status_int, 201) execution_id = self._get_actionexecution_id(post_resp) - updates = {"status": "succeeded", "result": {"stdout": "foobar"}} + updates = {"status": "succeeded", "result": {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}} put_resp = self._do_put(execution_id, updates) self.assertEqual(put_resp.status_int, 200) self.assertEqual(put_resp.json["status"], "succeeded") - self.assertDictEqual(put_resp.json["result"], {"stdout": "foobar"}) + self.assertDictEqual(put_resp.json["result"], {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}) updates = {"status": "abandoned"} put_resp = self._do_put(execution_id, updates, expect_errors=True) @@ -1345,7 +1345,7 @@ def test_put_execution_missing_liveaction(self): self.assertEqual(post_resp.status_int, 201) execution_id = self._get_actionexecution_id(post_resp) - updates = {"status": "succeeded", "result": {"stdout": "foobar"}} + updates = {"status": "succeeded", "result": {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}} put_resp = self._do_put(execution_id, updates, expect_errors=True) self.assertEqual(put_resp.status_int, 500) @@ -1612,22 +1612,22 @@ def test_get_raw_result(self): execution_id = self._get_actionexecution_id(get_resp) updates = { "status": "succeeded", - "result": {"stdout": "foobar", "stderr": "barfoo"}, + "result": {"stdout": "foobar", "stderr": "barfoo", "succeeded": True, "failed": False}, } put_resp = self._do_put(execution_id, updates) self.assertEqual(put_resp.status_int, 200) self.assertEqual(put_resp.json["status"], "succeeded") self.assertDictEqual( - put_resp.json["result"], {"stdout": "foobar", "stderr": "barfoo"} + put_resp.json["result"], {"stdout": "foobar", "stderr": "barfoo", "succeeded": True, "failed": False} ) self.assertEqual( - put_resp.json["result_size"], len('{"stdout":"foobar","stderr":"barfoo"}') + put_resp.json["result_size"], len('{"stdout":"foobar","stderr":"barfoo","succeeded":true,"failed":false}') ) # 1. download=False, compress=False, pretty_format=False get_resp = self.app.get("/v1/executions/%s/result" % (execution_id)) self.assertEqual(get_resp.headers["Content-Type"], "text/json") - self.assertEqual(get_resp.body, b'{"stdout":"foobar","stderr":"barfoo"}') + self.assertEqual(get_resp.body, b'{"stdout":"foobar","stderr":"barfoo","succeeded":true,"failed":false}') # 2. download=False, compress=False, pretty_format=True get_resp = self.app.get( @@ -1636,7 +1636,9 @@ def test_get_raw_result(self): expected_result = b""" { "stdout": "foobar", - "stderr": "barfoo" + "stderr": "barfoo", + "succeeded": true, + "failed": false }""".strip() self.assertEqual(get_resp.headers["Content-Type"], "text/json") self.assertEqual(get_resp.body, expected_result) @@ -1645,7 +1647,7 @@ def test_get_raw_result(self): # NOTE: webtest auto decompresses the result get_resp = self.app.get("/v1/executions/%s/result?compress=1" % (execution_id)) self.assertEqual(get_resp.headers["Content-Type"], "application/x-gzip") - self.assertEqual(get_resp.body, b'{"stdout":"foobar","stderr":"barfoo"}') + self.assertEqual(get_resp.body, b'{"stdout":"foobar","stderr":"barfoo","succeeded":true,"failed":false}') # 4. download=True, compress=False, pretty_format=False get_resp = self.app.get("/v1/executions/%s/result?download=1" % (execution_id)) @@ -1654,7 +1656,7 @@ def test_get_raw_result(self): get_resp.headers["Content-Disposition"], "attachment; filename=execution_%s_result.json" % (execution_id), ) - self.assertEqual(get_resp.body, b'{"stdout":"foobar","stderr":"barfoo"}') + self.assertEqual(get_resp.body, b'{"stdout":"foobar","stderr":"barfoo","succeeded":true,"failed":false}') # 5. download=True, compress=False, pretty_format=True get_resp = self.app.get( @@ -1663,7 +1665,9 @@ def test_get_raw_result(self): expected_result = b""" { "stdout": "foobar", - "stderr": "barfoo" + "stderr": "barfoo", + "succeeded": true, + "failed": false }""".strip() self.assertEqual(get_resp.headers["Content-Type"], "text/json") @@ -1681,7 +1685,9 @@ def test_get_raw_result(self): expected_result = b""" { "stdout": "foobar", - "stderr": "barfoo" + "stderr": "barfoo", + "succeeded": true, + "failed": false }""".strip() self.assertEqual(get_resp.headers["Content-Type"], "application/x-gzip") @@ -1774,7 +1780,7 @@ def test_get_single_attribute_success(self): data = {} data["status"] = action_constants.LIVEACTION_STATUS_SUCCEEDED - data["result"] = {"foo": "bar"} + data["result"] = {"foo": "bar", "stdout": "hello world", "stderr": "", "succeeded": True, "failed": False} resp = self.app.put_json("/v1/executions/%s" % (exec_id), data) self.assertEqual(resp.status_int, 200) diff --git a/st2common/tests/unit/services/test_workflow_rerun.py b/st2common/tests/unit/services/test_workflow_rerun.py index 1fb10ace6f..a6188478c1 100644 --- a/st2common/tests/unit/services/test_workflow_rerun.py +++ b/st2common/tests/unit/services/test_workflow_rerun.py @@ -50,7 +50,7 @@ RUNNER_RESULT_FAILED = (action_constants.LIVEACTION_STATUS_FAILED, {}, {}) RUNNER_RESULT_SUCCEEDED = ( action_constants.LIVEACTION_STATUS_SUCCEEDED, - {"stdout": "foobar"}, + {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}, {}, ) diff --git a/st2tests/st2tests/mocks/runners/runner.py b/st2tests/st2tests/mocks/runners/runner.py index b89b75b712..a64cf02820 100644 --- a/st2tests/st2tests/mocks/runners/runner.py +++ b/st2tests/st2tests/mocks/runners/runner.py @@ -29,7 +29,6 @@ def get_runner(config=None): class MockActionRunner(ActionRunner): def __init__(self): super(MockActionRunner, self).__init__(runner_id="1") - self.pre_run_called = False self.run_called = False self.post_run_called = False @@ -45,7 +44,22 @@ def run(self, action_params): if self.runner_parameters.get("raise", False): raise Exception("Raise required.") - default_result = {"ran": True, "action_params": action_params} + default_result = { + "ran": True, + "action_params": action_params, + "failed": False, + "stdout": "res", + "stderr": "", + "succeeded": True + } + if action_params.get("actionstr", "") == "dict_resp": + default_result["stdout"] = { + "key": "value", + "key2": { + "sk1": "v1" + } + } + default_context = {"third_party_system": {"ref_id": "1234"}} status = self.runner_parameters.get("mock_status", LIVEACTION_STATUS_SUCCEEDED) From d8c4fd1a5ca26896312924e01dcc2c17fc6ea21e Mon Sep 17 00:00:00 2001 From: rebrowning Date: Thu, 21 Dec 2023 22:37:57 -0800 Subject: [PATCH 3/8] fix some linting tests also add a little bit more verbose output so it's easier to find the issues... That flag can be removed if necessary --- Makefile | 4 +- .../unit/controllers/v1/test_executions.py | 95 ++++++++++++++++--- st2tests/st2tests/mocks/runners/runner.py | 9 +- 3 files changed, 85 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index 7cdf9c60fc..5b3c7412f8 100644 --- a/Makefile +++ b/Makefile @@ -377,7 +377,7 @@ black: requirements .black-check echo "==========================================================="; \ echo "Running black on" $$component; \ echo "==========================================================="; \ - . $(VIRTUALENV_DIR)/bin/activate ; black --check --config pyproject.toml $$component/ || exit 1; \ + . $(VIRTUALENV_DIR)/bin/activate ; black --diff --check --config pyproject.toml $$component/ || exit 1; \ if [ -d "$$component/bin" ]; then \ . $(VIRTUALENV_DIR)/bin/activate ; black $$(grep -rl '^#!/.*python' $$component/bin) || exit 1; \ fi \ @@ -387,7 +387,7 @@ black: requirements .black-check echo "==========================================================="; \ echo "Running black on" $$component; \ echo "==========================================================="; \ - . $(VIRTUALENV_DIR)/bin/activate ; black --check --config pyproject.toml $$component/ || exit 1; \ + . $(VIRTUALENV_DIR)/bin/activate ; black --diff --check --config pyproject.toml $$component/ || exit 1; \ if [ -d "$$component/bin" ]; then \ . $(VIRTUALENV_DIR)/bin/activate ; black $$(grep -rl '^#!/.*python' $$component/bin) || exit 1; \ fi \ diff --git a/st2api/tests/unit/controllers/v1/test_executions.py b/st2api/tests/unit/controllers/v1/test_executions.py index ae647c40d4..e57e0cd688 100644 --- a/st2api/tests/unit/controllers/v1/test_executions.py +++ b/st2api/tests/unit/controllers/v1/test_executions.py @@ -451,7 +451,12 @@ def test_get_one_max_result_size_query_parameter(self): # Update it with the result (this populates result and result size attributes) data = { - "result": {"stdout": {"fooo": "a" * 1000}, "succeeded": True, "failed": False, "stderr": ""}, + "result": { + "stdout": {"fooo": "a" * 1000}, + "succeeded": True, + "failed": False, + "stderr": "", + }, "status": "succeeded", } actual_result_size = len(json_encode(data["result"])) @@ -1281,16 +1286,30 @@ def test_put_status_and_result(self): self.assertEqual(post_resp.status_int, 201) execution_id = self._get_actionexecution_id(post_resp) - updates = {"status": "succeeded", "result": {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}} + updates = { + "status": "succeeded", + "result": { + "stdout": "foobar", + "succeeded": True, + "failed": False, + "stderr": "", + }, + } put_resp = self._do_put(execution_id, updates) self.assertEqual(put_resp.status_int, 200) self.assertEqual(put_resp.json["status"], "succeeded") - self.assertDictEqual(put_resp.json["result"], {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}) + self.assertDictEqual( + put_resp.json["result"], + {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}, + ) get_resp = self._do_get_one(execution_id) self.assertEqual(get_resp.status_int, 200) self.assertEqual(get_resp.json["status"], "succeeded") - self.assertDictEqual(get_resp.json["result"], {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}) + self.assertDictEqual( + get_resp.json["result"], + {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}, + ) def test_put_bad_state(self): post_resp = self._do_post(LIVE_ACTION_1) @@ -1329,11 +1348,22 @@ def test_put_status_to_completed_execution(self): self.assertEqual(post_resp.status_int, 201) execution_id = self._get_actionexecution_id(post_resp) - updates = {"status": "succeeded", "result": {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}} + updates = { + "status": "succeeded", + "result": { + "stdout": "foobar", + "succeeded": True, + "failed": False, + "stderr": "", + }, + } put_resp = self._do_put(execution_id, updates) self.assertEqual(put_resp.status_int, 200) self.assertEqual(put_resp.json["status"], "succeeded") - self.assertDictEqual(put_resp.json["result"], {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}) + self.assertDictEqual( + put_resp.json["result"], + {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}, + ) updates = {"status": "abandoned"} put_resp = self._do_put(execution_id, updates, expect_errors=True) @@ -1345,7 +1375,15 @@ def test_put_execution_missing_liveaction(self): self.assertEqual(post_resp.status_int, 201) execution_id = self._get_actionexecution_id(post_resp) - updates = {"status": "succeeded", "result": {"stdout": "foobar", "succeeded": True, "failed": False, "stderr": ""}} + updates = { + "status": "succeeded", + "result": { + "stdout": "foobar", + "succeeded": True, + "failed": False, + "stderr": "", + }, + } put_resp = self._do_put(execution_id, updates, expect_errors=True) self.assertEqual(put_resp.status_int, 500) @@ -1612,22 +1650,39 @@ def test_get_raw_result(self): execution_id = self._get_actionexecution_id(get_resp) updates = { "status": "succeeded", - "result": {"stdout": "foobar", "stderr": "barfoo", "succeeded": True, "failed": False}, + "result": { + "stdout": "foobar", + "stderr": "barfoo", + "succeeded": True, + "failed": False, + }, } put_resp = self._do_put(execution_id, updates) self.assertEqual(put_resp.status_int, 200) self.assertEqual(put_resp.json["status"], "succeeded") self.assertDictEqual( - put_resp.json["result"], {"stdout": "foobar", "stderr": "barfoo", "succeeded": True, "failed": False} + put_resp.json["result"], + { + "stdout": "foobar", + "stderr": "barfoo", + "succeeded": True, + "failed": False, + }, ) self.assertEqual( - put_resp.json["result_size"], len('{"stdout":"foobar","stderr":"barfoo","succeeded":true,"failed":false}') + put_resp.json["result_size"], + len( + '{"stdout":"foobar","stderr":"barfoo","succeeded":true,"failed":false}' + ), ) # 1. download=False, compress=False, pretty_format=False get_resp = self.app.get("/v1/executions/%s/result" % (execution_id)) self.assertEqual(get_resp.headers["Content-Type"], "text/json") - self.assertEqual(get_resp.body, b'{"stdout":"foobar","stderr":"barfoo","succeeded":true,"failed":false}') + self.assertEqual( + get_resp.body, + b'{"stdout":"foobar","stderr":"barfoo","succeeded":true,"failed":false}', + ) # 2. download=False, compress=False, pretty_format=True get_resp = self.app.get( @@ -1647,7 +1702,10 @@ def test_get_raw_result(self): # NOTE: webtest auto decompresses the result get_resp = self.app.get("/v1/executions/%s/result?compress=1" % (execution_id)) self.assertEqual(get_resp.headers["Content-Type"], "application/x-gzip") - self.assertEqual(get_resp.body, b'{"stdout":"foobar","stderr":"barfoo","succeeded":true,"failed":false}') + self.assertEqual( + get_resp.body, + b'{"stdout":"foobar","stderr":"barfoo","succeeded":true,"failed":false}', + ) # 4. download=True, compress=False, pretty_format=False get_resp = self.app.get("/v1/executions/%s/result?download=1" % (execution_id)) @@ -1656,7 +1714,10 @@ def test_get_raw_result(self): get_resp.headers["Content-Disposition"], "attachment; filename=execution_%s_result.json" % (execution_id), ) - self.assertEqual(get_resp.body, b'{"stdout":"foobar","stderr":"barfoo","succeeded":true,"failed":false}') + self.assertEqual( + get_resp.body, + b'{"stdout":"foobar","stderr":"barfoo","succeeded":true,"failed":false}', + ) # 5. download=True, compress=False, pretty_format=True get_resp = self.app.get( @@ -1780,7 +1841,13 @@ def test_get_single_attribute_success(self): data = {} data["status"] = action_constants.LIVEACTION_STATUS_SUCCEEDED - data["result"] = {"foo": "bar", "stdout": "hello world", "stderr": "", "succeeded": True, "failed": False} + data["result"] = { + "foo": "bar", + "stdout": "hello world", + "stderr": "", + "succeeded": True, + "failed": False, + } resp = self.app.put_json("/v1/executions/%s" % (exec_id), data) self.assertEqual(resp.status_int, 200) diff --git a/st2tests/st2tests/mocks/runners/runner.py b/st2tests/st2tests/mocks/runners/runner.py index a64cf02820..0a6603c537 100644 --- a/st2tests/st2tests/mocks/runners/runner.py +++ b/st2tests/st2tests/mocks/runners/runner.py @@ -50,15 +50,10 @@ def run(self, action_params): "failed": False, "stdout": "res", "stderr": "", - "succeeded": True + "succeeded": True, } if action_params.get("actionstr", "") == "dict_resp": - default_result["stdout"] = { - "key": "value", - "key2": { - "sk1": "v1" - } - } + default_result["stdout"] = {"key": "value", "key2": {"sk1": "v1"}} default_context = {"third_party_system": {"ref_id": "1234"}} From 1bcd9e443eea82bdc05b669bcc531e118ba3131c Mon Sep 17 00:00:00 2001 From: rebrowning Date: Fri, 12 Jan 2024 10:39:42 -0800 Subject: [PATCH 4/8] update the cchangelog --- CHANGELOG.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4571dd0525..8daf25e519 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,6 +6,12 @@ in development Fixed ~~~~~ +Added +~~~~~ +* add a schema to shell and winrm runners to allow validations to be executed againt their payloads. This means that shell or winrm jobs + that return a json object will be parsed into a dict to allow for field validations to be performed. Non json string resonses are still + handled as normal #6102 + 3.8.1 - December 13, 2023 ------------------------- Fixed From 214cecff5815d6af5632ad21c95fe4a3642d42bb Mon Sep 17 00:00:00 2001 From: rebrowning Date: Fri, 12 Jan 2024 11:15:24 -0800 Subject: [PATCH 5/8] see if these additional options work to cover additional types in tests --- contrib/runners/local_runner/local_runner/runner.yaml | 4 ++++ contrib/runners/remote_runner/remote_runner/runner.yaml | 4 ++++ contrib/runners/winrm_runner/winrm_runner/runner.yaml | 6 ++++++ 3 files changed, 14 insertions(+) diff --git a/contrib/runners/local_runner/local_runner/runner.yaml b/contrib/runners/local_runner/local_runner/runner.yaml index 2a56edda99..e4c173ca44 100644 --- a/contrib/runners/local_runner/local_runner/runner.yaml +++ b/contrib/runners/local_runner/local_runner/runner.yaml @@ -47,6 +47,8 @@ anyOf: - type: "object" - type: "string" + - type: "list" + - type: "number" required: true stderr: type: string @@ -108,6 +110,8 @@ anyOf: - type: "object" - type: "string" + - type: "list" + - type: "number" required: true stderr: type: string diff --git a/contrib/runners/remote_runner/remote_runner/runner.yaml b/contrib/runners/remote_runner/remote_runner/runner.yaml index 3eaed94192..5cbe628d87 100644 --- a/contrib/runners/remote_runner/remote_runner/runner.yaml +++ b/contrib/runners/remote_runner/remote_runner/runner.yaml @@ -96,6 +96,8 @@ anyOf: - type: "object" - type: "string" + - type: "list" + - type: "number" required: true stderr: type: string @@ -196,6 +198,8 @@ anyOf: - type: "object" - type: "string" + - type: "list" + - type: "number" required: true stderr: type: string diff --git a/contrib/runners/winrm_runner/winrm_runner/runner.yaml b/contrib/runners/winrm_runner/winrm_runner/runner.yaml index 7e1c47b475..d8f6d2cb73 100644 --- a/contrib/runners/winrm_runner/winrm_runner/runner.yaml +++ b/contrib/runners/winrm_runner/winrm_runner/runner.yaml @@ -80,6 +80,8 @@ anyOf: - type: "object" - type: "string" + - type: "list" + - type: "number" required: true stderr: type: string @@ -168,6 +170,8 @@ anyOf: - type: "object" - type: "string" + - type: "list" + - type: "number" required: true stderr: type: string @@ -253,6 +257,8 @@ anyOf: - type: "object" - type: "string" + - type: "list" + - type: "number" required: true stderr: type: string From 989c8b49c7a750c1d3419acdf691ae2ed1dd5eb7 Mon Sep 17 00:00:00 2001 From: rebrowning Date: Fri, 12 Jan 2024 11:21:37 -0800 Subject: [PATCH 6/8] fix this type --- contrib/runners/local_runner/local_runner/runner.yaml | 4 ++-- contrib/runners/remote_runner/remote_runner/runner.yaml | 4 ++-- contrib/runners/winrm_runner/winrm_runner/runner.yaml | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/contrib/runners/local_runner/local_runner/runner.yaml b/contrib/runners/local_runner/local_runner/runner.yaml index e4c173ca44..2ec3f73c01 100644 --- a/contrib/runners/local_runner/local_runner/runner.yaml +++ b/contrib/runners/local_runner/local_runner/runner.yaml @@ -47,7 +47,7 @@ anyOf: - type: "object" - type: "string" - - type: "list" + - type: "array" - type: "number" required: true stderr: @@ -110,7 +110,7 @@ anyOf: - type: "object" - type: "string" - - type: "list" + - type: "array" - type: "number" required: true stderr: diff --git a/contrib/runners/remote_runner/remote_runner/runner.yaml b/contrib/runners/remote_runner/remote_runner/runner.yaml index 5cbe628d87..ab869385ab 100644 --- a/contrib/runners/remote_runner/remote_runner/runner.yaml +++ b/contrib/runners/remote_runner/remote_runner/runner.yaml @@ -96,7 +96,7 @@ anyOf: - type: "object" - type: "string" - - type: "list" + - type: "array" - type: "number" required: true stderr: @@ -198,7 +198,7 @@ anyOf: - type: "object" - type: "string" - - type: "list" + - type: "array" - type: "number" required: true stderr: diff --git a/contrib/runners/winrm_runner/winrm_runner/runner.yaml b/contrib/runners/winrm_runner/winrm_runner/runner.yaml index d8f6d2cb73..d560f973da 100644 --- a/contrib/runners/winrm_runner/winrm_runner/runner.yaml +++ b/contrib/runners/winrm_runner/winrm_runner/runner.yaml @@ -80,7 +80,7 @@ anyOf: - type: "object" - type: "string" - - type: "list" + - type: "array" - type: "number" required: true stderr: @@ -170,7 +170,7 @@ anyOf: - type: "object" - type: "string" - - type: "list" + - type: "array" - type: "number" required: true stderr: @@ -257,7 +257,7 @@ anyOf: - type: "object" - type: "string" - - type: "list" + - type: "array" - type: "number" required: true stderr: From 8e98f3126589318397f5896ee5cd1642f6e36ee8 Mon Sep 17 00:00:00 2001 From: rebrowning Date: Fri, 12 Jan 2024 11:36:44 -0800 Subject: [PATCH 7/8] adjust which pieces are required --- contrib/runners/local_runner/local_runner/runner.yaml | 6 ------ contrib/runners/remote_runner/remote_runner/runner.yaml | 6 ------ contrib/runners/winrm_runner/winrm_runner/runner.yaml | 9 --------- 3 files changed, 21 deletions(-) diff --git a/contrib/runners/local_runner/local_runner/runner.yaml b/contrib/runners/local_runner/local_runner/runner.yaml index 2ec3f73c01..25cd153edd 100644 --- a/contrib/runners/local_runner/local_runner/runner.yaml +++ b/contrib/runners/local_runner/local_runner/runner.yaml @@ -39,10 +39,8 @@ properties: succeeded: type: boolean - required: true failed: type: boolean - required: true stdout: anyOf: - type: "object" @@ -52,7 +50,6 @@ required: true stderr: type: string - required: true return_code: type: integer - description: A runner to execute local actions as a fixed user. @@ -102,10 +99,8 @@ properties: succeeded: type: boolean - required: true failed: type: boolean - required: true stdout: anyOf: - type: "object" @@ -115,6 +110,5 @@ required: true stderr: type: string - required: true return_code: type: integer \ No newline at end of file diff --git a/contrib/runners/remote_runner/remote_runner/runner.yaml b/contrib/runners/remote_runner/remote_runner/runner.yaml index ab869385ab..b4cdc0d5e1 100644 --- a/contrib/runners/remote_runner/remote_runner/runner.yaml +++ b/contrib/runners/remote_runner/remote_runner/runner.yaml @@ -88,10 +88,8 @@ properties: succeeded: type: boolean - required: true failed: type: boolean - required: true stdout: anyOf: - type: "object" @@ -101,7 +99,6 @@ required: true stderr: type: string - required: true return_code: type: integer - description: A remote execution runner that executes actions as a fixed system user. @@ -190,10 +187,8 @@ properties: succeeded: type: boolean - required: true failed: type: boolean - required: true stdout: anyOf: - type: "object" @@ -203,6 +198,5 @@ required: true stderr: type: string - required: true return_code: type: integer \ No newline at end of file diff --git a/contrib/runners/winrm_runner/winrm_runner/runner.yaml b/contrib/runners/winrm_runner/winrm_runner/runner.yaml index d560f973da..30dd722483 100644 --- a/contrib/runners/winrm_runner/winrm_runner/runner.yaml +++ b/contrib/runners/winrm_runner/winrm_runner/runner.yaml @@ -72,10 +72,8 @@ properties: succeeded: type: boolean - required: true failed: type: boolean - required: true stdout: anyOf: - type: "object" @@ -85,7 +83,6 @@ required: true stderr: type: string - required: true return_code: type: integer - description: A remote execution runner that executes PowerShell commands via WinRM on a remote host @@ -162,10 +159,8 @@ properties: succeeded: type: boolean - required: true failed: type: boolean - required: true stdout: anyOf: - type: "object" @@ -175,7 +170,6 @@ required: true stderr: type: string - required: true return_code: type: integer - description: A remote execution runner that executes PowerShell script via WinRM on a set of remote hosts @@ -249,10 +243,8 @@ properties: succeeded: type: boolean - required: true failed: type: boolean - required: true stdout: anyOf: - type: "object" @@ -262,6 +254,5 @@ required: true stderr: type: string - required: true return_code: type: integer \ No newline at end of file From 3f9db87f564ffe170eb6dcc3d56d1b7981ef4133 Mon Sep 17 00:00:00 2001 From: rebrowning Date: Sat, 13 Jan 2024 12:58:53 -0800 Subject: [PATCH 8/8] remove the required field --- contrib/runners/local_runner/local_runner/runner.yaml | 2 -- contrib/runners/remote_runner/remote_runner/runner.yaml | 2 -- contrib/runners/winrm_runner/winrm_runner/runner.yaml | 3 --- 3 files changed, 7 deletions(-) diff --git a/contrib/runners/local_runner/local_runner/runner.yaml b/contrib/runners/local_runner/local_runner/runner.yaml index 25cd153edd..f2eec20966 100644 --- a/contrib/runners/local_runner/local_runner/runner.yaml +++ b/contrib/runners/local_runner/local_runner/runner.yaml @@ -47,7 +47,6 @@ - type: "string" - type: "array" - type: "number" - required: true stderr: type: string return_code: @@ -107,7 +106,6 @@ - type: "string" - type: "array" - type: "number" - required: true stderr: type: string return_code: diff --git a/contrib/runners/remote_runner/remote_runner/runner.yaml b/contrib/runners/remote_runner/remote_runner/runner.yaml index b4cdc0d5e1..807ec27d67 100644 --- a/contrib/runners/remote_runner/remote_runner/runner.yaml +++ b/contrib/runners/remote_runner/remote_runner/runner.yaml @@ -96,7 +96,6 @@ - type: "string" - type: "array" - type: "number" - required: true stderr: type: string return_code: @@ -195,7 +194,6 @@ - type: "string" - type: "array" - type: "number" - required: true stderr: type: string return_code: diff --git a/contrib/runners/winrm_runner/winrm_runner/runner.yaml b/contrib/runners/winrm_runner/winrm_runner/runner.yaml index 30dd722483..5727c9457b 100644 --- a/contrib/runners/winrm_runner/winrm_runner/runner.yaml +++ b/contrib/runners/winrm_runner/winrm_runner/runner.yaml @@ -80,7 +80,6 @@ - type: "string" - type: "array" - type: "number" - required: true stderr: type: string return_code: @@ -167,7 +166,6 @@ - type: "string" - type: "array" - type: "number" - required: true stderr: type: string return_code: @@ -251,7 +249,6 @@ - type: "string" - type: "array" - type: "number" - required: true stderr: type: string return_code: