From 1451d9c13438357f38ac91a46615dea69c6c15a1 Mon Sep 17 00:00:00 2001 From: Yumeng_Bao Date: Tue, 6 Jun 2017 20:00:03 +0800 Subject: [PATCH] Replace voluptuous with JSONSchema to validate migration action Now in watcher,both JSONSchema and voluptuous are used to validate JSON payloads. We want to remove voluptuous and Use JSONSchema as our only JSON validation tool to keep consistence and also to make it easier to expose the validation schema through our API in future work. In this patch, we replace voluptuous with JSONSchema to validate the migration action in watcher applier. Partially Implements: blueprint jsonschema-validation Change-Id: I02bff5db9bd06567bcc33b61a316c42c805bb20e --- requirements.txt | 1 + watcher/applier/actions/migration.py | 61 +++++++++++-------- .../tests/applier/actions/test_migration.py | 40 ++++-------- 3 files changed, 49 insertions(+), 53 deletions(-) diff --git a/requirements.txt b/requirements.txt index 3e0d42c89..0bad2cca1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,6 +6,7 @@ apscheduler # MIT License enum34;python_version=='2.7' or python_version=='2.6' or python_version=='3.3' # BSD jsonpatch>=1.1 # BSD keystoneauth1>=2.21.0 # Apache-2.0 +jsonschema!=2.5.0,<3.0.0,>=2.0.0 # MIT keystonemiddleware>=4.12.0 # Apache-2.0 lxml!=3.7.0,>=2.3 # BSD oslo.concurrency>=3.8.0 # Apache-2.0 diff --git a/watcher/applier/actions/migration.py b/watcher/applier/actions/migration.py index 94ec0af2b..36d467d22 100644 --- a/watcher/applier/actions/migration.py +++ b/watcher/applier/actions/migration.py @@ -17,15 +17,13 @@ # limitations under the License. # -from oslo_log import log -import six -import voluptuous +import jsonschema +from oslo_log import log from watcher._i18n import _ from watcher.applier.actions import base from watcher.common import exception from watcher.common import nova_helper -from watcher.common import utils LOG = log.getLogger(__name__) @@ -62,28 +60,42 @@ class Migrate(base.BaseAction): DESTINATION_NODE = 'destination_node' SOURCE_NODE = 'source_node' - def check_resource_id(self, value): - if (value is not None and - len(value) > 0 and not - utils.is_uuid_like(value)): - raise voluptuous.Invalid(_("The parameter " - "resource_id is invalid.")) - @property def schema(self): - return voluptuous.Schema({ - voluptuous.Required(self.RESOURCE_ID): self.check_resource_id, - voluptuous.Required( - self.MIGRATION_TYPE, default=self.LIVE_MIGRATION): - voluptuous.Any( - *[self.LIVE_MIGRATION, self.COLD_MIGRATION]), - voluptuous.Required(self.DESTINATION_NODE): - voluptuous.All(voluptuous.Any(*six.string_types), - voluptuous.Length(min=1)), - voluptuous.Required(self.SOURCE_NODE): - voluptuous.All(voluptuous.Any(*six.string_types), - voluptuous.Length(min=1)), - }) + return { + 'type': 'object', + 'properties': { + 'destination_node': { + 'type': 'string', + "minLength": 1 + }, + 'migration_type': { + 'type': 'string', + "enum": ["live", "cold"] + }, + 'resource_id': { + 'type': 'string', + "minlength": 1, + "pattern": ("^([a-fA-F0-9]){8}-([a-fA-F0-9]){4}-" + "([a-fA-F0-9]){4}-([a-fA-F0-9]){4}-" + "([a-fA-F0-9]){12}$") + }, + 'source_node': { + 'type': 'string', + "minLength": 1 + } + }, + 'required': ['destination_node', 'migration_type', + 'resource_id', 'source_node'], + 'additionalProperties': False, + } + + def validate_parameters(self): + try: + jsonschema.validate(self.input_parameters, self.schema) + return True + except jsonschema.ValidationError as e: + raise e @property def instance_uuid(self): @@ -137,7 +149,6 @@ class Migrate(base.BaseAction): LOG.critical("Unexpected error occurred. Migration failed for " "instance %s. Leaving instance on previous " "host.", self.instance_uuid) - return result def migrate(self, destination): diff --git a/watcher/tests/applier/actions/test_migration.py b/watcher/tests/applier/actions/test_migration.py index b91ae8633..8dda5772e 100644 --- a/watcher/tests/applier/actions/test_migration.py +++ b/watcher/tests/applier/actions/test_migration.py @@ -15,8 +15,9 @@ from __future__ import unicode_literals + +import jsonschema import mock -import voluptuous from watcher.applier.actions import base as baction from watcher.applier.actions import migration @@ -93,13 +94,8 @@ class TestMigration(base.TestCase): 'source_node': None, 'destination_node': None} self.action.input_parameters = parameters - exc = self.assertRaises( - voluptuous.MultipleInvalid, self.action.validate_parameters) - self.assertEqual( - sorted([(['migration_type'], voluptuous.ScalarInvalid), - (['source_node'], voluptuous.TypeInvalid), - (['destination_node'], voluptuous.TypeInvalid)]), - sorted([(e.path, type(e)) for e in exc.errors])) + self.assertRaises(jsonschema.ValidationError, + self.action.validate_parameters) def test_parameters_exception_migration_type(self): parameters = {baction.BaseAction.RESOURCE_ID: @@ -108,11 +104,8 @@ class TestMigration(base.TestCase): 'source_node': 'compute-2', 'destination_node': 'compute-3'} self.action.input_parameters = parameters - exc = self.assertRaises( - voluptuous.Invalid, self.action.validate_parameters) - self.assertEqual( - [(['migration_type'], voluptuous.ScalarInvalid)], - [(e.path, type(e)) for e in exc.errors]) + self.assertRaises(jsonschema.ValidationError, + self.action.validate_parameters) def test_parameters_exception_source_node(self): parameters = {baction.BaseAction.RESOURCE_ID: @@ -121,11 +114,8 @@ class TestMigration(base.TestCase): 'source_node': None, 'destination_node': 'compute-3'} self.action.input_parameters = parameters - exc = self.assertRaises( - voluptuous.MultipleInvalid, self.action.validate_parameters) - self.assertEqual( - [(['source_node'], voluptuous.TypeInvalid)], - [(e.path, type(e)) for e in exc.errors]) + self.assertRaises(jsonschema.ValidationError, + self.action.validate_parameters) def test_parameters_exception_destination_node(self): parameters = {baction.BaseAction.RESOURCE_ID: @@ -134,11 +124,8 @@ class TestMigration(base.TestCase): 'source_node': 'compute-1', 'destination_node': None} self.action.input_parameters = parameters - exc = self.assertRaises( - voluptuous.MultipleInvalid, self.action.validate_parameters) - self.assertEqual( - [(['destination_node'], voluptuous.TypeInvalid)], - [(e.path, type(e)) for e in exc.errors]) + self.assertRaises(jsonschema.ValidationError, + self.action.validate_parameters) def test_parameters_exception_resource_id(self): parameters = {baction.BaseAction.RESOURCE_ID: "EFEF", @@ -146,11 +133,8 @@ class TestMigration(base.TestCase): 'source_node': 'compute-2', 'destination_node': 'compute-3'} self.action.input_parameters = parameters - exc = self.assertRaises( - voluptuous.MultipleInvalid, self.action.validate_parameters) - self.assertEqual( - [(['resource_id'], voluptuous.Invalid)], - [(e.path, type(e)) for e in exc.errors]) + self.assertRaises(jsonschema.ValidationError, + self.action.validate_parameters) def test_migration_pre_condition(self): try: