From 3680e0bd3669a58e9b993b832bf783e215c83caa Mon Sep 17 00:00:00 2001 From: Michal Gershenzon Date: Wed, 18 May 2016 14:46:47 +0000 Subject: [PATCH] Validate ad-hoc action via cli Expose the new mistral feature of validating ad-hoc actions. Change-Id: Ib9eaa43eea193e22b149fc20ffee140607631dc6 Implements: blueprint validate-ad-hoc-action-cli Depends-On: Ibbb949ef38befae1ef83a2a56cda4c817ceb41d4 --- mistralclient/api/v2/actions.py | 18 ++++ mistralclient/commands/v2/actions.py | 34 +++++++ mistralclient/commands/v2/workbooks.py | 6 +- mistralclient/commands/v2/workflows.py | 6 +- mistralclient/shell.py | 1 + mistralclient/tests/unit/v2/test_actions.py | 94 +++++++++++++++++++ .../tests/unit/v2/test_cli_actions.py | 19 ++++ mistralclient/tests/unit/v2/test_workbooks.py | 26 +++-- 8 files changed, 185 insertions(+), 19 deletions(-) diff --git a/mistralclient/api/v2/actions.py b/mistralclient/api/v2/actions.py index 18c866ca..d722467f 100644 --- a/mistralclient/api/v2/actions.py +++ b/mistralclient/api/v2/actions.py @@ -97,3 +97,21 @@ class ActionManager(base.ResourceManager): self._ensure_not_empty(name=name) self._delete('/actions/%s' % name) + + def validate(self, definition): + self._ensure_not_empty(definition=definition) + + # If the specified definition is actually a file, read in the + # definition file + definition = utils.get_contents_if_file(definition) + + resp = self.client.http_client.post( + '/actions/validate', + definition, + headers={'content-type': 'text/plain'} + ) + + if resp.status_code != 200: + self._raise_api_exception(resp) + + return base.extract_json(resp, None) diff --git a/mistralclient/commands/v2/actions.py b/mistralclient/commands/v2/actions.py index af68cf6b..5cbd42a0 100644 --- a/mistralclient/commands/v2/actions.py +++ b/mistralclient/commands/v2/actions.py @@ -201,3 +201,37 @@ class GetDefinition(command.Command): definition = mistral_client.actions.get(parsed_args.name).definition self.app.stdout.write(definition or "\n") + + +class Validate(show.ShowOne): + """Validate action.""" + + def _format(self, result=None): + columns = ('Valid', 'Error') + + if result: + data = (result.get('valid'), result.get('error')) + else: + data = (tuple('' for _ in range(len(columns))),) + + return columns, data + + def get_parser(self, prog_name): + parser = super(Validate, self).get_parser(prog_name) + + parser.add_argument( + 'definition', + type=argparse.FileType('r'), + help='action definition file' + ) + + return parser + + def take_action(self, parsed_args): + mistral_client = self.app.client_manager.workflow_engine + + result = mistral_client.actions.validate( + parsed_args.definition.read() + ) + + return self._format(result) diff --git a/mistralclient/commands/v2/workbooks.py b/mistralclient/commands/v2/workbooks.py index 315173e8..af8f8f04 100644 --- a/mistralclient/commands/v2/workbooks.py +++ b/mistralclient/commands/v2/workbooks.py @@ -173,11 +173,7 @@ class Validate(show.ShowOne): columns = ('Valid', 'Error') if result: - data = (result.get('valid'),) - if not result.get('error'): - data += (None,) - else: - data += (result.get('error'),) + data = (result.get('valid'), result.get('error'),) else: data = (tuple('' for _ in range(len(columns))),) diff --git a/mistralclient/commands/v2/workflows.py b/mistralclient/commands/v2/workflows.py index f90bb906..1a5c3464 100644 --- a/mistralclient/commands/v2/workflows.py +++ b/mistralclient/commands/v2/workflows.py @@ -210,11 +210,7 @@ class Validate(show.ShowOne): columns = ('Valid', 'Error') if result: - data = (result.get('valid'),) - if not result.get('error'): - data += (None,) - else: - data += (result.get('error'),) + data = (result.get('valid'), result.get('error'),) else: data = (tuple('' for _ in range(len(columns))),) diff --git a/mistralclient/shell.py b/mistralclient/shell.py index 04d4bec6..a86c0ec7 100644 --- a/mistralclient/shell.py +++ b/mistralclient/shell.py @@ -403,6 +403,7 @@ class MistralShell(app.App): 'action-update': mistralclient.commands.v2.actions.Update, 'action-get-definition': mistralclient.commands.v2.actions.GetDefinition, + 'action-validate': mistralclient.commands.v2.actions.Validate, 'cron-trigger-list': mistralclient.commands.v2.cron_triggers.List, 'cron-trigger-get': mistralclient.commands.v2.cron_triggers.Get, 'cron-trigger-create': diff --git a/mistralclient/tests/unit/v2/test_actions.py b/mistralclient/tests/unit/v2/test_actions.py index 9416fe3c..91d6fdcc 100644 --- a/mistralclient/tests/unit/v2/test_actions.py +++ b/mistralclient/tests/unit/v2/test_actions.py @@ -16,6 +16,7 @@ import pkg_resources as pkg from six.moves.urllib import parse from six.moves.urllib import request +from mistralclient.api import base as api_base from mistralclient.api.v2 import actions from mistralclient.tests.unit.v2 import base @@ -32,6 +33,19 @@ my_action: info: <% $.output %> """ +INVALID_ACTION_DEF = """ +--- +version: 2.0 + +my_action: + base: std.echo + unexpected-property: 'this should fail' + base-input: + output: 'Bye!' + output: + info: <% $.output %> +""" + ACTION = { 'id': '123', 'name': 'my_action', @@ -42,6 +56,7 @@ ACTION = { URL_TEMPLATE = '/actions' URL_TEMPLATE_SCOPE = '/actions?scope=private' URL_TEMPLATE_NAME = '/actions/%s' +URL_TEMPLATE_VALIDATE = '/actions/validate' class TestActionsV2(base.BaseClientV2Test): @@ -169,3 +184,82 @@ class TestActionsV2(base.BaseClientV2Test): self.actions.delete('action') mock.assert_called_once_with(URL_TEMPLATE_NAME % 'action') + + def test_validate(self): + mock = self.mock_http_post( + status_code=200, + content={'valid': True} + ) + + result = self.actions.validate(ACTION_DEF) + + self.assertIsNotNone(result) + self.assertIn('valid', result) + self.assertTrue(result['valid']) + + mock.assert_called_once_with( + URL_TEMPLATE_VALIDATE, + ACTION_DEF, + headers={'content-type': 'text/plain'} + ) + + def test_validate_with_file(self): + mock = self.mock_http_post( + status_code=200, + content={'valid': True} + ) + + # The contents of action_v2.yaml must be identical to ACTION_DEF + path = pkg.resource_filename( + 'mistralclient', + 'tests/unit/resources/action_v2.yaml' + ) + + result = self.actions.validate(path) + + self.assertIsNotNone(result) + self.assertIn('valid', result) + self.assertTrue(result['valid']) + + mock.assert_called_once_with( + URL_TEMPLATE_VALIDATE, + ACTION_DEF, + headers={'content-type': 'text/plain'} + ) + + def test_validate_failed(self): + mock_result = { + "valid": False, + "error": "mocked error message" + } + + mock = self.mock_http_post(status_code=200, content=mock_result) + + result = self.actions.validate(INVALID_ACTION_DEF) + + self.assertIsNotNone(result) + self.assertIn('valid', result) + self.assertFalse(result['valid']) + self.assertIn('error', result) + self.assertIn("mocked error message", result['error']) + + mock.assert_called_once_with( + URL_TEMPLATE_VALIDATE, + INVALID_ACTION_DEF, + headers={'content-type': 'text/plain'} + ) + + def test_validate_api_failed(self): + mock = self.mock_http_post(status_code=500, content={}) + + self.assertRaises( + api_base.APIException, + self.actions.validate, + ACTION_DEF + ) + + mock.assert_called_once_with( + URL_TEMPLATE_VALIDATE, + ACTION_DEF, + headers={'content-type': 'text/plain'} + ) diff --git a/mistralclient/tests/unit/v2/test_cli_actions.py b/mistralclient/tests/unit/v2/test_cli_actions.py index 5466dff6..15f8a7ee 100644 --- a/mistralclient/tests/unit/v2/test_cli_actions.py +++ b/mistralclient/tests/unit/v2/test_cli_actions.py @@ -171,3 +171,22 @@ class TestCLIActionsV2(base.BaseCommandTest): self.call(action_cmd.GetDefinition, app_args=['name']) self.app.stdout.write.assert_called_with(ACTION_DEF) + + @mock.patch('argparse.open', create=True) + def test_validate(self, mock_open): + self.client.actions.validate.return_value = {'valid': True} + + result = self.call(action_cmd.Validate, app_args=['action.yaml']) + + self.assertEqual((True, None), result[1]) + + @mock.patch('argparse.open', create=True) + def test_validate_failed(self, mock_open): + self.client.actions.validate.return_value = { + 'valid': False, + 'error': 'Invalid DSL...' + } + + result = self.call(action_cmd.Validate, app_args=['action.yaml']) + + self.assertEqual((False, 'Invalid DSL...'), result[1]) diff --git a/mistralclient/tests/unit/v2/test_workbooks.py b/mistralclient/tests/unit/v2/test_workbooks.py index 4c8487e9..5427b229 100644 --- a/mistralclient/tests/unit/v2/test_workbooks.py +++ b/mistralclient/tests/unit/v2/test_workbooks.py @@ -177,8 +177,10 @@ class TestWorkbooksV2(base.BaseClientV2Test): mock.assert_called_once_with(URL_TEMPLATE_NAME % 'wb') def test_validate(self): - mock = self.mock_http_post(status_code=200, - content={'valid': True}) + mock = self.mock_http_post( + status_code=200, + content={'valid': True} + ) result = self.workbooks.validate(WB_DEF) @@ -193,8 +195,10 @@ class TestWorkbooksV2(base.BaseClientV2Test): ) def test_validate_with_file(self): - mock = self.mock_http_post(status_code=200, - content={'valid': True}) + mock = self.mock_http_post( + status_code=200, + content={'valid': True} + ) # The contents of wb_v2.yaml must be identical to WB_DEF path = pkg.resource_filename( @@ -229,8 +233,10 @@ class TestWorkbooksV2(base.BaseClientV2Test): self.assertIn('valid', result) self.assertFalse(result['valid']) self.assertIn('error', result) - self.assertIn("Task properties 'action' and 'workflow' " - "can't be specified both", result['error']) + self.assertIn( + "Task properties 'action' and 'workflow' " + "can't be specified both", result['error'] + ) mock.assert_called_once_with( URL_TEMPLATE_VALIDATE, @@ -241,9 +247,11 @@ class TestWorkbooksV2(base.BaseClientV2Test): def test_validate_api_failed(self): mock = self.mock_http_post(status_code=500, content={}) - self.assertRaises(api_base.APIException, - self.workbooks.validate, - WB_DEF) + self.assertRaises( + api_base.APIException, + self.workbooks.validate, + WB_DEF + ) mock.assert_called_once_with( URL_TEMPLATE_VALIDATE,