From 1c702e2555c487db25a6a26c34707368071e14ba Mon Sep 17 00:00:00 2001 From: Nikolay Mahotkin Date: Tue, 15 Sep 2015 14:25:58 +0300 Subject: [PATCH] Adding 'is_system' to definition model * Fixed work of mistral-db-manage populate * Fixed modifying of standard workflows Closes-Bug: #1478448 Closes-Bug: #1396121 Change-Id: Ia2825408ee43a6dfc80e33fa9614e11f3ff6d97b --- mistral/api/controllers/v2/workflow.py | 10 ++++- ...007_move_system_flag_to_base_definition.py | 40 +++++++++++++++++++ mistral/db/v2/sqlalchemy/models.py | 2 +- mistral/services/workflows.py | 31 ++++++++++---- mistral/tests/unit/api/v2/test_workflows.py | 26 ++++++++++++ 5 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 mistral/db/sqlalchemy/migration/alembic_migrations/versions/007_move_system_flag_to_base_definition.py diff --git a/mistral/api/controllers/v2/workflow.py b/mistral/api/controllers/v2/workflow.py index 25565c08..9c90428c 100644 --- a/mistral/api/controllers/v2/workflow.py +++ b/mistral/api/controllers/v2/workflow.py @@ -26,6 +26,7 @@ from mistral.api.controllers.v2 import types from mistral.api.controllers.v2 import validation from mistral.api.hooks import content_type as ct_hook from mistral.db.v2 import api as db_api +from mistral import exceptions as exc from mistral.services import workflows from mistral.utils import rest_utils from mistral.workbook import parser as spec_parser @@ -172,7 +173,14 @@ class WorkflowsController(rest.RestController, hooks.HookController): """Delete the named workflow.""" LOG.info("Delete workflow [name=%s]" % name) - db_api.delete_workflow_definition(name) + with db_api.transaction(): + wf_db = db_api.get_workflow_definition(name) + + if wf_db.is_system: + msg = "Attempt to delete a system workflow: %s" % name + raise exc.DataAccessException(msg) + + db_api.delete_workflow_definition(name) @rest_utils.wrap_pecan_controller_exception @wsme_pecan.wsexpose(Workflows, types.uuid, int, types.uniquelist, diff --git a/mistral/db/sqlalchemy/migration/alembic_migrations/versions/007_move_system_flag_to_base_definition.py b/mistral/db/sqlalchemy/migration/alembic_migrations/versions/007_move_system_flag_to_base_definition.py new file mode 100644 index 00000000..dcfc975b --- /dev/null +++ b/mistral/db/sqlalchemy/migration/alembic_migrations/versions/007_move_system_flag_to_base_definition.py @@ -0,0 +1,40 @@ +# Copyright 2015 OpenStack Foundation. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Move system flag to base definition + +Revision ID: 007 +Revises: 006 +Create Date: 2015-09-15 11:24:43.081824 + +""" + +# revision identifiers, used by Alembic. +revision = '007' +down_revision = '006' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column( + 'workbooks_v2', + sa.Column('is_system', sa.Boolean(), nullable=True) + ) + op.add_column( + 'workflow_definitions_v2', + sa.Column('is_system', sa.Boolean(), nullable=True) + ) diff --git a/mistral/db/v2/sqlalchemy/models.py b/mistral/db/v2/sqlalchemy/models.py index 6830e575..5ec07aa9 100644 --- a/mistral/db/v2/sqlalchemy/models.py +++ b/mistral/db/v2/sqlalchemy/models.py @@ -43,6 +43,7 @@ class Definition(mb.MistralSecureModelBase): definition = sa.Column(sa.Text(), nullable=True) spec = sa.Column(st.JsonDictType()) tags = sa.Column(st.JsonListType()) + is_system = sa.Column(sa.Boolean()) # There's no WorkbookExecution so we safely omit "Definition" in the name. @@ -82,7 +83,6 @@ class ActionDefinition(Definition): # Service properties. action_class = sa.Column(sa.String(200)) attributes = sa.Column(st.JsonDictType()) - is_system = sa.Column(sa.Boolean()) # Execution objects. diff --git a/mistral/services/workflows.py b/mistral/services/workflows.py index 0ff5daae..f421cf39 100644 --- a/mistral/services/workflows.py +++ b/mistral/services/workflows.py @@ -13,6 +13,7 @@ # limitations under the License. from mistral.db.v2 import api as db_api +from mistral import exceptions as exc from mistral import utils from mistral.workbook import parser as spec_parser @@ -25,21 +26,29 @@ def register_standard_workflows(): for wf_path in workflow_paths: workflow_definition = open(wf_path).read() - create_workflows(workflow_definition, scope='public') + + create_workflows(workflow_definition, scope='public', is_system=True) + + +def _clear_system_workflow_db(): + db_api.delete_workflow_definitions(is_system=True) def sync_db(): + _clear_system_workflow_db() register_standard_workflows() -def create_workflows(definition, scope='private'): +def create_workflows(definition, scope='private', is_system=False): wf_list_spec = spec_parser.get_workflow_list_spec_from_yaml(definition) db_wfs = [] with db_api.transaction(): for wf_spec in wf_list_spec.get_workflows(): - db_wfs.append(_create_workflow(wf_spec, definition, scope)) + db_wfs.append( + _create_workflow(wf_spec, definition, scope, is_system) + ) return db_wfs @@ -60,25 +69,33 @@ def update_workflows(definition, scope='private'): return db_wfs -def _get_workflow_values(wf_spec, definition, scope): +def _get_workflow_values(wf_spec, definition, scope, is_system=False): values = { 'name': wf_spec.get_name(), 'tags': wf_spec.get_tags(), 'definition': definition, 'spec': wf_spec.to_dict(), - 'scope': scope + 'scope': scope, + 'is_system': is_system } return values -def _create_workflow(wf_spec, definition, scope): +def _create_workflow(wf_spec, definition, scope, is_system): return db_api.create_workflow_definition( - _get_workflow_values(wf_spec, definition, scope) + _get_workflow_values(wf_spec, definition, scope, is_system) ) def _update_workflow(wf_spec, definition, scope): + workflow = db_api.load_workflow_definition(wf_spec.get_name()) + + if workflow and workflow.is_system: + raise exc.InvalidActionException( + "Attempt to modify a system workflow: %s" % + workflow.name + ) values = _get_workflow_values(wf_spec, definition, scope) return db_api.update_workflow_definition(values['name'], values) diff --git a/mistral/tests/unit/api/v2/test_workflows.py b/mistral/tests/unit/api/v2/test_workflows.py index 66fdd7fe..44e88d7e 100644 --- a/mistral/tests/unit/api/v2/test_workflows.py +++ b/mistral/tests/unit/api/v2/test_workflows.py @@ -46,6 +46,9 @@ WF_DB = models.WorkflowDefinition( spec={'input': ['param1']} ) +WF_DB_SYSTEM = WF_DB.get_clone() +WF_DB_SYSTEM.is_system = True + WF = { 'id': '123e4567-e89b-12d3-a456-426655440000', 'name': 'flow', @@ -137,6 +140,7 @@ flow: """ MOCK_WF = mock.MagicMock(return_value=WF_DB) +MOCK_WF_SYSTEM = mock.MagicMock(return_value=WF_DB_SYSTEM) MOCK_WF_WITH_INPUT = mock.MagicMock(return_value=WF_DB_WITH_INPUT) MOCK_WFS = mock.MagicMock(return_value=[WF_DB]) MOCK_UPDATED_WF = mock.MagicMock(return_value=UPDATED_WF_DB) @@ -184,6 +188,20 @@ class TestWorkflowsController(base.FunctionalTest): self.assertEqual(resp.status_int, 200) self.assertDictEqual({'workflows': [UPDATED_WF]}, resp.json) + @mock.patch.object( + db_api, "load_workflow_definition", MOCK_WF_SYSTEM + ) + def test_put_system(self): + resp = self.app.put( + '/v2/workflows', + UPDATED_WF_DEFINITION, + headers={'Content-Type': 'text/plain'}, + expect_errors=True + ) + + self.assertEqual(resp.status_int, 400) + self.assertIn("Attempt to modify a system workflow", resp.text) + @mock.patch.object( db_api, "update_workflow_definition", MOCK_WF_WITH_INPUT ) @@ -266,11 +284,19 @@ class TestWorkflowsController(base.FunctionalTest): self.assertIn("Invalid DSL", resp.body) @mock.patch.object(db_api, "delete_workflow_definition", MOCK_DELETE) + @mock.patch.object(db_api, "get_workflow_definition", MOCK_WF) def test_delete(self): resp = self.app.delete('/v2/workflows/123') self.assertEqual(resp.status_int, 204) + @mock.patch.object(db_api, "get_workflow_definition", MOCK_WF_SYSTEM) + def test_delete_system(self): + resp = self.app.delete('/v2/workflows/123', expect_errors=True) + + self.assertEqual(resp.status_int, 400) + self.assertIn("Attempt to delete a system workflow", resp.text) + @mock.patch.object(db_api, "delete_workflow_definition", MOCK_NOT_FOUND) def test_delete_not_found(self): resp = self.app.delete('/v2/workflows/123', expect_errors=True)