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

@@ -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,

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)
+    )

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.

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)

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)