diff --git a/contrib/devstack/lib/mistral b/contrib/devstack/lib/mistral index 37888fb21..4f19e22d4 100644 --- a/contrib/devstack/lib/mistral +++ b/contrib/devstack/lib/mistral @@ -101,6 +101,7 @@ function configure_mistral { iniset $MISTRAL_CONF_FILE keystone_authtoken admin_tenant_name $SERVICE_TENANT_NAME iniset $MISTRAL_CONF_FILE keystone_authtoken admin_user $MISTRAL_ADMIN_USER iniset $MISTRAL_CONF_FILE keystone_authtoken admin_password $SERVICE_PASSWORD + iniset $MISTRAL_CONF_FILE keystone_authtoken auth_uri "http://${KEYSTONE_AUTH_HOST}:5000/v3" # Setup RabbitMQ credentials iniset $MISTRAL_CONF_FILE DEFAULT rabbit_userid $RABBIT_USERID @@ -111,8 +112,8 @@ function configure_mistral { iniset $MISTRAL_CONF_FILE database max_overflow -1 iniset $MISTRAL_CONF_FILE database max_pool_size 1000 - # Configure keystone auth url - iniset $MISTRAL_CONF_FILE keystone_authtoken auth_uri "http://${KEYSTONE_AUTH_HOST}:5000/v3" + # Configure action execution deletion policy + iniset $MISTRAL_CONF_FILE api allow_action_execution_deletion True } diff --git a/mistral/api/controllers/v2/action_execution.py b/mistral/api/controllers/v2/action_execution.py index e97d9fee6..fc49f11c9 100644 --- a/mistral/api/controllers/v2/action_execution.py +++ b/mistral/api/controllers/v2/action_execution.py @@ -14,6 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from oslo_config import cfg from oslo_log import log as logging from pecan import rest from wsme import types as wtypes @@ -191,6 +192,29 @@ class ActionExecutionsController(rest.RestController): return _get_action_executions() + @rest_utils.wrap_wsme_controller_exception + @wsme_pecan.wsexpose(None, wtypes.text, status_code=204) + def delete(self, id): + """Delete the specified action_execution.""" + + LOG.info("Delete action_execution [id=%s]" % id) + + if not cfg.CONF.api.allow_action_execution_deletion: + raise exc.NotAllowedException("Action execution deletion is not " + "allowed.") + + action_ex = db_api.get_action_execution(id) + + if action_ex.task_execution_id: + raise exc.NotAllowedException("Only ad-hoc action execution can " + "be deleted.") + + if not states.is_completed(action_ex.state): + raise exc.NotAllowedException("Only completed action execution " + "can be deleted.") + + return db_api.delete_action_execution(id) + class TasksActionExecutionController(rest.RestController): @wsme_pecan.wsexpose(ActionExecutions, wtypes.text) diff --git a/mistral/config.py b/mistral/config.py index 91b837919..298f9797d 100644 --- a/mistral/config.py +++ b/mistral/config.py @@ -36,7 +36,10 @@ launch_opt = cfg.ListOpt( api_opts = [ cfg.StrOpt('host', default='0.0.0.0', help='Mistral API server host'), - cfg.IntOpt('port', default=8989, help='Mistral API server port') + cfg.IntOpt('port', default=8989, help='Mistral API server port'), + cfg.BoolOpt('allow_action_execution_deletion', default=False, + help='Enables the ability to delete action_execution which ' + 'has no relationship with workflows.'), ] pecan_opts = [ diff --git a/mistral/exceptions.py b/mistral/exceptions.py index 4879875f1..f5d2710c7 100644 --- a/mistral/exceptions.py +++ b/mistral/exceptions.py @@ -127,3 +127,8 @@ class SizeLimitExceededException(MistralException): class CoordinationException(MistralException): http_code = 500 + + +class NotAllowedException(MistralException): + http_code = 403 + message = "Operation not allowed" diff --git a/mistral/tests/functional/api/v2/test_mistral_basic_v2.py b/mistral/tests/functional/api/v2/test_mistral_basic_v2.py index a7659cf3c..efcc79425 100644 --- a/mistral/tests/functional/api/v2/test_mistral_basic_v2.py +++ b/mistral/tests/functional/api/v2/test_mistral_basic_v2.py @@ -14,6 +14,8 @@ import json +from oslo_log import log as logging +import six from tempest import test from tempest_lib import decorators from tempest_lib import exceptions @@ -22,6 +24,9 @@ from mistral.tests.functional import base from mistral import utils +LOG = logging.getLogger(__name__) + + class WorkbookTestsV2(base.TestCase): _service = 'workflowv2' @@ -976,15 +981,26 @@ class TasksTestsV2(base.TestCase): ) -# TODO(namkhotkin) Need more tests on action executions. class ActionExecutionTestsV2(base.TestCase): - _service = 'workflowv2' + @classmethod + def resource_cleanup(cls): + for action_ex in cls.client.action_executions: + try: + cls.client.delete_obj('action_executions', action_ex) + except Exception as e: + LOG.exception('Exception raised when deleting ' + 'action_executions %s, error message: %s.' + % (action_ex, six.text_type(e))) + + cls.client.action_executions = [] + + super(ActionExecutionTestsV2, cls).resource_cleanup() + @test.attr(type='sanity') def test_run_action_execution(self): - resp, body = self.client.post_json( - 'action_executions', + resp, body = self.client.create_action_execution( { 'name': 'std.echo', 'input': '{"output": "Hello, Mistral!"}' @@ -992,7 +1008,6 @@ class ActionExecutionTestsV2(base.TestCase): ) self.assertEqual(201, resp.status) - body = json.loads(body) output = json.loads(body['output']) self.assertDictEqual( {'result': 'Hello, Mistral!'}, @@ -1001,8 +1016,7 @@ class ActionExecutionTestsV2(base.TestCase): @test.attr(type='sanity') def test_run_action_std_http(self): - resp, body = self.client.post_json( - 'action_executions', + resp, body = self.client.create_action_execution( { 'name': 'std.http', 'input': '{"url": "http://wiki.openstack.org"}' @@ -1010,14 +1024,12 @@ class ActionExecutionTestsV2(base.TestCase): ) self.assertEqual(201, resp.status) - body = json.loads(body) output = json.loads(body['output']) self.assertTrue(output['result']['status'] in range(200, 307)) @test.attr(type='sanity') def test_run_action_std_http_error(self): - resp, body = self.client.post_json( - 'action_executions', + resp, body = self.client.create_action_execution( { 'name': 'std.http', 'input': '{"url": "http://www.google.ru/not-found-test"}' @@ -1025,14 +1037,12 @@ class ActionExecutionTestsV2(base.TestCase): ) self.assertEqual(201, resp.status) - body = json.loads(body) output = json.loads(body['output']) self.assertEqual(404, output['result']['status']) @test.attr(type='sanity') def test_create_action_execution(self): - resp, body = self.client.post_json( - 'action_executions', + resp, body = self.client.create_action_execution( { 'name': 'std.echo', 'input': '{"output": "Hello, Mistral!"}', @@ -1041,9 +1051,6 @@ class ActionExecutionTestsV2(base.TestCase): ) self.assertEqual(201, resp.status) - - body = json.loads(body) - self.assertEqual('RUNNING', body['state']) # We must reread action execution in order to get actual @@ -1059,3 +1066,12 @@ class ActionExecutionTestsV2(base.TestCase): {'result': 'Hello, Mistral!'}, output ) + + @test.attr(type='negative') + def test_delete_nonexistent_action_execution(self): + self.assertRaises( + exceptions.NotFound, + self.client.delete_obj, + 'action_executions', + 'nonexist' + ) diff --git a/mistral/tests/functional/base.py b/mistral/tests/functional/base.py index cb0eb0632..08fbd823e 100644 --- a/mistral/tests/functional/base.py +++ b/mistral/tests/functional/base.py @@ -70,6 +70,7 @@ class MistralClientBase(rest_client.RestClient): self.workflows = [] self.triggers = [] self.actions = [] + self.action_executions = [] def get_list_obj(self, name): resp, body = self.get(name) @@ -204,6 +205,15 @@ class MistralClientV2(MistralClientBase): return [t for t in all_tasks if t['workflow_name'] == wf_name] + def create_action_execution(self, request_body): + resp, body = self.post_json('action_executions', request_body) + + params = json.loads(request_body.get('params', '{}')) + if params.get('save_result', False): + self.action_executions.append(json.loads(body)['id']) + + return resp, json.loads(body) + class AuthProv(auth.KeystoneV2AuthProvider): def __init__(self): diff --git a/mistral/tests/unit/api/v2/test_action_executions.py b/mistral/tests/unit/api/v2/test_action_executions.py index 76dd34981..70e7e1392 100644 --- a/mistral/tests/unit/api/v2/test_action_executions.py +++ b/mistral/tests/unit/api/v2/test_action_executions.py @@ -17,7 +17,9 @@ import copy import datetime import json + import mock +from oslo_config import cfg from mistral.db.v2 import api as db_api from mistral.db.v2.sqlalchemy import models @@ -45,6 +47,34 @@ ACTION_EX_DB = models.ActionExecution( updated_at=datetime.datetime(1970, 1, 1) ) +AD_HOC_ACTION_EX_DB = models.ActionExecution( + id='123', + state=states.SUCCESS, + state_info=states.SUCCESS, + tags=['foo', 'fee'], + name='std.echo', + description='something', + accepted=True, + input={}, + output={}, + created_at=datetime.datetime(1970, 1, 1), + updated_at=datetime.datetime(1970, 1, 1) +) + +ACTION_EX_DB_NOT_COMPLETE = models.ActionExecution( + id='123', + state=states.RUNNING, + state_info=states.RUNNING, + tags=['foo', 'fee'], + name='std.echo', + description='something', + accepted=False, + input={}, + output={}, + created_at=datetime.datetime(1970, 1, 1), + updated_at=datetime.datetime(1970, 1, 1) +) + ACTION_EX = { 'id': '123', 'workflow_name': 'flow', @@ -80,24 +110,39 @@ BROKEN_ACTION = copy.copy(ACTION_EX) BROKEN_ACTION['output'] = 'string not escaped' MOCK_ACTION = mock.MagicMock(return_value=ACTION_EX_DB) +MOCK_ACTION_NOT_COMPLETE = mock.MagicMock( + return_value=ACTION_EX_DB_NOT_COMPLETE +) +MOCK_AD_HOC_ACTION = mock.MagicMock(return_value=AD_HOC_ACTION_EX_DB) MOCK_ACTIONS = mock.MagicMock(return_value=[ACTION_EX_DB]) MOCK_EMPTY = mock.MagicMock(return_value=[]) MOCK_NOT_FOUND = mock.MagicMock(side_effect=exc.NotFoundException()) +MOCK_DELETE = mock.MagicMock(return_value=None) class TestActionExecutionsController(base.FunctionalTest): + def setUp(self): + super(TestActionExecutionsController, self).setUp() + + self.addCleanup( + cfg.CONF.set_default, + 'allow_action_execution_deletion', + False, + group='api' + ) + @mock.patch.object(db_api, 'get_action_execution', MOCK_ACTION) def test_get(self): resp = self.app.get('/v2/action_executions/123') - self.assertEqual(resp.status_int, 200) + self.assertEqual(200, resp.status_int) self.assertDictEqual(ACTION_EX, resp.json) @mock.patch.object(db_api, 'get_action_execution', MOCK_NOT_FOUND) def test_get_not_found(self): resp = self.app.get('/v2/action_executions/123', expect_errors=True) - self.assertEqual(resp.status_int, 404) + self.assertEqual(404, resp.status_int) @mock.patch.object(rpc.EngineClient, 'start_action') def test_post(self, f): @@ -112,7 +157,7 @@ class TestActionExecutionsController(base.FunctionalTest): } ) - self.assertEqual(resp.status_int, 201) + self.assertEqual(201, resp.status_int) action_exec = ACTION_EX del action_exec['task_name'] @@ -136,7 +181,7 @@ class TestActionExecutionsController(base.FunctionalTest): {'name': 'nova.servers_list'} ) - self.assertEqual(resp.status_int, 201) + self.assertEqual(201, resp.status_int) self.assertEqual('{"result": "123"}', resp.json['output']) f.assert_called_once_with('nova.servers_list', {}, description=None) @@ -148,7 +193,7 @@ class TestActionExecutionsController(base.FunctionalTest): expect_errors=True ) - self.assertEqual(resp.status_int, 400) + self.assertEqual(400, resp.status_int) def test_post_bad_input(self): resp = self.app.post_json( @@ -157,7 +202,7 @@ class TestActionExecutionsController(base.FunctionalTest): expect_errors=True ) - self.assertEqual(resp.status_int, 400) + self.assertEqual(400, resp.status_int) @mock.patch.object(rpc.EngineClient, 'on_action_complete') def test_put(self, f): @@ -165,7 +210,7 @@ class TestActionExecutionsController(base.FunctionalTest): resp = self.app.put_json('/v2/action_executions/123', UPDATED_ACTION) - self.assertEqual(resp.status_int, 200) + self.assertEqual(200, resp.status_int) self.assertDictEqual(UPDATED_ACTION, resp.json) f.assert_called_once_with( @@ -179,7 +224,7 @@ class TestActionExecutionsController(base.FunctionalTest): resp = self.app.put_json('/v2/action_executions/123', ERROR_ACTION) - self.assertEqual(resp.status_int, 200) + self.assertEqual(200, resp.status_int) self.assertDictEqual(ERROR_ACTION, resp.json) f.assert_called_once_with( @@ -199,7 +244,7 @@ class TestActionExecutionsController(base.FunctionalTest): expect_errors=True ) - self.assertEqual(resp.status_int, 404) + self.assertEqual(404, resp.status_int) def test_put_bad_result(self): resp = self.app.put_json( @@ -208,7 +253,7 @@ class TestActionExecutionsController(base.FunctionalTest): expect_errors=True ) - self.assertEqual(resp.status_int, 400) + self.assertEqual(400, resp.status_int) @mock.patch.object(rpc.EngineClient, 'on_action_complete') def test_put_without_result(self, f): @@ -219,21 +264,69 @@ class TestActionExecutionsController(base.FunctionalTest): resp = self.app.put_json('/v2/action_executions/123', action_ex) - self.assertEqual(resp.status_int, 200) + self.assertEqual(200, resp.status_int) @mock.patch.object(db_api, 'get_action_executions', MOCK_ACTIONS) def test_get_all(self): resp = self.app.get('/v2/action_executions') - self.assertEqual(resp.status_int, 200) + self.assertEqual(200, resp.status_int) - self.assertEqual(len(resp.json['action_executions']), 1) + self.assertEqual(1, len(resp.json['action_executions'])) self.assertDictEqual(ACTION_EX, resp.json['action_executions'][0]) @mock.patch.object(db_api, 'get_action_executions', MOCK_EMPTY) def test_get_all_empty(self): resp = self.app.get('/v2/action_executions') - self.assertEqual(resp.status_int, 200) + self.assertEqual(200, resp.status_int) - self.assertEqual(len(resp.json['action_executions']), 0) + self.assertEqual(0, len(resp.json['action_executions'])) + + @mock.patch.object(db_api, 'get_action_execution', MOCK_AD_HOC_ACTION) + @mock.patch.object(db_api, 'delete_action_execution', MOCK_DELETE) + def test_delete(self): + cfg.CONF.set_default('allow_action_execution_deletion', True, 'api') + + resp = self.app.delete('/v2/action_executions/123') + + self.assertEqual(204, resp.status_int) + + @mock.patch.object(db_api, 'get_action_execution', MOCK_NOT_FOUND) + def test_delete_not_found(self): + cfg.CONF.set_default('allow_action_execution_deletion', True, 'api') + + resp = self.app.delete('/v2/action_executions/123', expect_errors=True) + + self.assertEqual(404, resp.status_int) + + def test_delete_not_allowed(self): + resp = self.app.delete('/v2/action_executions/123', expect_errors=True) + + self.assertEqual(403, resp.status_int) + self.assertIn("Action execution deletion is not allowed", resp.body) + + @mock.patch.object(db_api, 'get_action_execution', MOCK_ACTION) + def test_delete_action_exeuction_with_task(self): + cfg.CONF.set_default('allow_action_execution_deletion', True, 'api') + + resp = self.app.delete('/v2/action_executions/123', expect_errors=True) + + self.assertEqual(403, resp.status_int) + self.assertIn("Only ad-hoc action execution can be deleted", resp.body) + + @mock.patch.object( + db_api, + 'get_action_execution', + MOCK_ACTION_NOT_COMPLETE + ) + def test_delete_action_exeuction_not_complete(self): + cfg.CONF.set_default('allow_action_execution_deletion', True, 'api') + + resp = self.app.delete('/v2/action_executions/123', expect_errors=True) + + self.assertEqual(403, resp.status_int) + self.assertIn( + "Only completed action execution can be deleted", + resp.body + ) diff --git a/mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py b/mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py index 410c5a418..f37c7dc0e 100644 --- a/mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py +++ b/mistral/tests/unit/db/v2/test_sqlalchemy_db_api.py @@ -633,6 +633,18 @@ class ActionExecutionTest(SQLAlchemyTest): created.id ) + def test_delete_other_tenant_action_execution(self): + created = db_api.create_action_execution(ACTION_EXECS[0]) + + # Create a new user. + auth_context.set_ctx(test_base.get_context(default=False)) + + self.assertRaises( + exc.NotFoundException, + db_api.delete_action_execution, + created.id + ) + def test_trim_status_info(self): created = db_api.create_action_execution(ACTION_EXECS[0])