From ba67b3aff8bdcf1ce951d1f61e69ba19799a09fc Mon Sep 17 00:00:00 2001 From: Dougal Matthews Date: Wed, 30 Nov 2016 08:01:34 +0000 Subject: [PATCH] Allow "version" to be within workflow names in workbooks There were two bugs in the schema validation for Mistral workflow names within Workbooks. The goal in this part of the validation is to detect the version property and assign it the enum type and then assin all other single word properties to use the workflow/action schemas. 1. Detecting version properties. The code before this patch has the following line. "version": {"enum": ["2.0", 2.0]}, This looked safe, but "version" is actually a regular expression. Meaning that it will match any string containging "version". Adding ^ at the start and $ at the end means that it matches a string from the start to the end and is exactly "version". 2. Detecting workflows/actions. The current regular expression was "^(?!version)\w+$": _action_schema This worked in most cases. It basically means "Any word that doesn't start with version". So this worked, but was incorrect for workflows called "versionworkflow" - it didn't match this when it should have. Using ?: to create a non-capturing portion of the regular expression solved this issue. Any workflows that have a name that doesn't match this regular expression. Such as those with a space (like "my worflow") were silently ignored before this patch. The addition of additionalProperties: False in the schema shows users an error if they use an invalid workflow name. Closes-Bug: #1645354 Change-Id: Ib2b406a05cf15b41be075f886de77509a9da8535 --- .../workbook/v2/workbook_schema_test.yaml | 43 ++++++++++++++++ .../tests/unit/workbook/v2/test_workbook.py | 51 +++++++++++++++++++ mistral/workbook/v2/workbook.py | 17 ++++--- 3 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 mistral/tests/resources/workbook/v2/workbook_schema_test.yaml diff --git a/mistral/tests/resources/workbook/v2/workbook_schema_test.yaml b/mistral/tests/resources/workbook/v2/workbook_schema_test.yaml new file mode 100644 index 000000000..1e3973247 --- /dev/null +++ b/mistral/tests/resources/workbook/v2/workbook_schema_test.yaml @@ -0,0 +1,43 @@ +version: '2.0' + +name: workbook_schema_test +description: > + This is a test workbook to verify workbook the schema validation. + Specifically we want to test the validation of workflow names. + See bug #1645354 for more details. + +actions: + actionversion: + base: std.noop + + versionaction: + base: std.noop + + actionversionaction: + base: std.noop + +workflows: + + workflowversion: + description: Workflow name ending with version + tasks: + task1: + action: actionversion + + versionworkflow: + description: Workflow name starting with version + tasks: + task1: + action: versionaction + + workflowversionworkflow: + description: Workflow name with version in the middle + tasks: + task1: + action: actionversionaction + + version_workflow: + description: Workflow name starting with version and an underscore + tasks: + task1: + workflow: workflowversion diff --git a/mistral/tests/unit/workbook/v2/test_workbook.py b/mistral/tests/unit/workbook/v2/test_workbook.py index b550869d1..a53eea74b 100644 --- a/mistral/tests/unit/workbook/v2/test_workbook.py +++ b/mistral/tests/unit/workbook/v2/test_workbook.py @@ -14,11 +14,13 @@ # limitations under the License. import copy +import re import yaml from mistral import exceptions as exc from mistral.tests.unit.workbook.v2 import base +from mistral.workbook.v2 import workbook class WorkbookSpecValidation(base.WorkbookSpecValidationTestCase): @@ -379,3 +381,52 @@ class WorkbookSpecValidation(base.WorkbookSpecValidationTestCase): for workflows, expect_error in tests: self._parse_dsl_spec(changes=workflows, expect_error=expect_error) + + def test_workflow_name_validation(self): + wb_spec = self._parse_dsl_spec(dsl_file='workbook_schema_test.yaml') + + d = wb_spec.to_dict() + + self.assertEqual('2.0', d['version']) + self.assertEqual('2.0', d['workflows']['version']) + + workflow_names = ['workflowversion', 'versionworkflow', + 'workflowversionworkflow', 'version_workflow'] + + action_names = ['actionversion', 'versionaction', + 'actionversionaction'] + + for name in workflow_names: + self.assertEqual('2.0', d['workflows'][name]['version']) + self.assertEqual(name, d['workflows'][name]['name']) + + for name in action_names: + self.assertEqual('2.0', d['actions'][name]['version']) + self.assertEqual(name, d['actions'][name]['name']) + + def test_name_regex(self): + + # We want to match a string containing version at any point. + valid_names = ( + "workflowversion", + "versionworkflow", + "workflowversionworkflow", + "version_workflow", + ) + + for valid in valid_names: + result = re.match(workbook.NON_VERSION_WORD_REGEX, valid) + self.assertNotEqual(None, result, + "Expected match for: {}".format(valid)) + + # ... except, we don't want to match a string that isn't just one word + # or is exactly "version" + invalid_names = ( + "version", + "my workflow", + ) + + for invalid in invalid_names: + result = re.match(workbook.NON_VERSION_WORD_REGEX, invalid) + self.assertEqual(None, result, + "Didn't expected match for: {}".format(invalid)) diff --git a/mistral/workbook/v2/workbook.py b/mistral/workbook/v2/workbook.py index eecc814a6..84c5f639d 100644 --- a/mistral/workbook/v2/workbook.py +++ b/mistral/workbook/v2/workbook.py @@ -17,6 +17,9 @@ from mistral.workbook.v2 import actions as act from mistral.workbook.v2 import base from mistral.workbook.v2 import workflows as wf +# We want to match any single word that isn't exactly "version" +NON_VERSION_WORD_REGEX = "^(?!version$)\w+$" + class WorkbookSpec(base.BaseSpec): # See http://json-schema.org @@ -33,17 +36,19 @@ class WorkbookSpec(base.BaseSpec): "type": "object", "minProperties": 1, "patternProperties": { - "version": {"enum": ["2.0", 2.0]}, - "^(?!version)\w+$": _action_schema - } + "^version$": {"enum": ["2.0", 2.0]}, + NON_VERSION_WORD_REGEX: _action_schema + }, + "additionalProperties": False }, "workflows": { "type": "object", "minProperties": 1, "patternProperties": { - "version": {"enum": ["2.0", 2.0]}, - "^(?!version)\w+$": _workflow_schema - } + "^version$": {"enum": ["2.0", 2.0]}, + NON_VERSION_WORD_REGEX: _workflow_schema + }, + "additionalProperties": False } }, "additionalProperties": False