From 5f7b49d1cb0d3d7372d2cc6aa3119fed164930a9 Mon Sep 17 00:00:00 2001 From: Simon Pasquier Date: Wed, 20 Apr 2016 15:07:29 +0000 Subject: [PATCH] Revert "Lift too strict restrictions on cross-deps role name" This reverts commit 4a6e3f93debd7ae2304a53f173f3bc6195169028. Change-Id: Ib3fc11af6ffbc780ab8e7170f7f5cff676f45cf8 --- fuel_plugin_builder/errors.py | 4 - .../tests/test_base_validator.py | 5 +- .../tests/test_validator_v4.py | 87 ++----------------- fuel_plugin_builder/validators/base.py | 10 +-- .../validators/formatchecker.py | 53 ----------- fuel_plugin_builder/validators/schemas/v4.py | 46 +++------- .../validators/validator_v4.py | 5 +- 7 files changed, 27 insertions(+), 183 deletions(-) delete mode 100644 fuel_plugin_builder/validators/formatchecker.py diff --git a/fuel_plugin_builder/errors.py b/fuel_plugin_builder/errors.py index 12c0df9..fe21caf 100644 --- a/fuel_plugin_builder/errors.py +++ b/fuel_plugin_builder/errors.py @@ -33,10 +33,6 @@ class ValidationError(FuelPluginException): pass -class TaskFieldError(ValidationError): - pass - - class FileIsEmpty(ValidationError): def __init__(self, file_path): super(FileIsEmpty, self).__init__( diff --git a/fuel_plugin_builder/tests/test_base_validator.py b/fuel_plugin_builder/tests/test_base_validator.py index 42fe604..f9d9953 100644 --- a/fuel_plugin_builder/tests/test_base_validator.py +++ b/fuel_plugin_builder/tests/test_base_validator.py @@ -13,7 +13,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -import jsonschema + import mock from fuel_plugin_builder import errors @@ -37,7 +37,6 @@ class TestBaseValidator(BaseTestCase): self.validator = NewValidator(self.plugin_path) self.data = {'data': 'data1'} self.schema = self.make_schema(['data'], {'data': {'type': 'string'}}) - self.format_checker = jsonschema.FormatChecker @classmethod def make_schema(cls, required, properties): @@ -55,7 +54,7 @@ class TestBaseValidator(BaseTestCase): 'file_path') schema_mock.validate.assert_called_once_with( self.data, - self.schema, format_checker=self.format_checker) + self.schema) def test_validate_schema_raises_error(self): schema = self.make_schema(['key'], {'key': {'type': 'string'}}) diff --git a/fuel_plugin_builder/tests/test_validator_v4.py b/fuel_plugin_builder/tests/test_validator_v4.py index 79f4850..9fb1b21 100644 --- a/fuel_plugin_builder/tests/test_validator_v4.py +++ b/fuel_plugin_builder/tests/test_validator_v4.py @@ -290,7 +290,7 @@ class TestValidatorV4(TestValidatorV3): mock_data = [{ 'id': 'plugin_task', 'type': 'puppet', - 'groups': ['controller'], + 'group': ['controller'], 'reexecute_on': ['bla']}] err_msg = "File '/tmp/plugin_path/deployment_tasks.yaml', " \ "'bla' is not one of" @@ -333,8 +333,7 @@ class TestValidatorV4(TestValidatorV3): ] err_msg = "File '/tmp/plugin_path/deployment_tasks.yaml', " \ - "'(role|groups|roles)' is " \ - "a required property, value path '0'" + "'role' is a required property, value path '0'" for task in deployment_tasks_data: self.check_raised_exception( utils_mock, [task], @@ -343,89 +342,21 @@ class TestValidatorV4(TestValidatorV3): # This is the section of tests inherited from the v3 validator # where decorators is re-defined for module v4 + @mock.patch('fuel_plugin_builder.validators.validator_v4.utils') + def test_check_deployment_task_role(self, utils_mock, *args): + super(TestValidatorV4, self).test_check_deployment_task_role( + utils_mock) + @mock.patch('fuel_plugin_builder.validators.validator_v4.utils') @mock.patch('fuel_plugin_builder.validators.base.utils.exists') def test_check_tasks_no_file(self, exists_mock, utils_mock, *args): super(TestValidatorV4, self).test_check_deployment_task_role( exists_mock, utils_mock) - @mock.patch('fuel_plugin_builder.validators.validator_v4.utils') - def test_check_deployment_task_role(self, utils_mock, *args): - utils_mock.parse_yaml.return_value = [ - {'id': 'plugin_name', 'type': 'group', 'groups': ['a', 'b']}, - {'id': 'plugin_name', 'type': 'group', 'groups': '*'}, - {'id': 'plugin_name', 'type': 'puppet', 'role': ['a', 'b']}, - {'id': 'plugin_name', 'type': 'puppet', 'role': '*'}, - {'id': 'plugin_name', 'type': 'shell', 'roles': ['a', 'b']}, - {'id': 'plugin_name', 'type': 'shell', 'roles': '*'}, - {'id': 'plugin_name', 'type': 'skipped', 'role': '/test/'}, - {'id': 'plugin_name', 'type': 'stage'}, - {'id': 'plugin_name', 'type': 'reboot', 'groups': 'contrail'}, - { - 'id': 'plugin_name', - 'type': 'copy_files', - 'role': '*', - 'parameters': { - 'files': [ - {'src': 'some_source', 'dst': 'some_destination'}]} - }, - { - 'id': 'plugin_name', - 'type': 'sync', - 'role': 'plugin_name', - 'parameters': { - 'src': 'some_source', 'dst': 'some_destination'} - }, - { - 'id': 'plugin_name', - 'type': 'upload_file', - 'role': '/^.*plugin\w+name$/', - 'parameters': { - 'path': 'some_path', 'data': 'some_data'} - }, - ] - - self.validator.check_deployment_tasks() - @mock.patch('fuel_plugin_builder.validators.validator_v4.utils') def test_check_deployment_task_role_failed(self, utils_mock, *args): - mock_data = [{ - 'id': 'plugin_name', - 'type': 'group', - 'role': ['plugin_n@me']}] - err_msg = "field should" - self.check_raised_exception( - utils_mock, mock_data, - err_msg, self.validator.check_deployment_tasks) - - @mock.patch('fuel_plugin_builder.validators.validator_v4.utils') - def test_check_deployment_task_required_missing(self, utils_mock, *args): - mock_data = [{ - 'id': 'plugin_name', - 'type': 'puppet'}] - err_msg = "is a required property" - self.check_raised_exception( - utils_mock, mock_data, - err_msg, self.validator.check_deployment_tasks) - - @mock.patch('fuel_plugin_builder.validators.validator_v4.utils') - def test_check_deployment_task_required_roles_missing_is_ok( - self, utils_mock, *args): - utils_mock.parse_yaml.return_value = [{ - 'id': 'plugin_name', - 'type': 'stage'}] - self.validator.check_deployment_tasks() - - @mock.patch('fuel_plugin_builder.validators.validator_v4.utils') - def test_check_deployment_task_role_regexp_failed(self, utils_mock, *args): - mock_data = [{ - 'id': 'plugin_name', - 'type': 'group', - 'role': '/[0-9]++/'}] - err_msg = "field should.*multiple repeat" - self.check_raised_exception( - utils_mock, mock_data, - err_msg, self.validator.check_deployment_tasks) + super(TestValidatorV4, self).test_check_deployment_task_role_failed( + utils_mock) @mock.patch('fuel_plugin_builder.validators.validator_v4.utils') def test_check_group_type_deployment_task_does_not_contain_manifests( diff --git a/fuel_plugin_builder/validators/base.py b/fuel_plugin_builder/validators/base.py index 078c6b2..f5e45af 100644 --- a/fuel_plugin_builder/validators/base.py +++ b/fuel_plugin_builder/validators/base.py @@ -35,16 +35,15 @@ class BaseValidator(object): def basic_version(self): pass - def __init__(self, plugin_path, format_checker=jsonschema.FormatChecker): + def __init__(self, plugin_path): self.plugin_path = plugin_path - self.format_checker = format_checker def validate_schema(self, data, schema, file_path, value_path=None): logger.debug( 'Start schema validation for %s file, %s', file_path, schema) + try: - jsonschema.validate(data, schema, - format_checker=self.format_checker) + jsonschema.validate(data, schema) except jsonschema.exceptions.ValidationError as exc: raise errors.ValidationError( self._make_error_message(exc, file_path, value_path)) @@ -105,7 +104,6 @@ class BaseValidator(object): @abc.abstractmethod def validate(self): """Performs validation - """ def check_schemas(self): @@ -171,11 +169,9 @@ class BaseValidator(object): def check_compatibility(self): """Json schema doesn't have any conditions, so we have - to make sure here, that this validation schema can be used for described fuel releases """ - meta = utils.parse_yaml(self.meta_path) for fuel_release in meta['fuel_version']: if StrictVersion(fuel_release) < StrictVersion(self.basic_version): diff --git a/fuel_plugin_builder/validators/formatchecker.py b/fuel_plugin_builder/validators/formatchecker.py deleted file mode 100644 index 0be98f5..0000000 --- a/fuel_plugin_builder/validators/formatchecker.py +++ /dev/null @@ -1,53 +0,0 @@ -# -*- coding: utf-8 -*- - -# Copyright 2016 Mirantis, Inc. -# -# 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. - -import re -from sre_constants import error as sre_error - -import jsonschema -import six - -from fuel_plugin_builder import errors - - -class FormatChecker(jsonschema.FormatChecker): - - def __init__(self, role_patterns=(), *args, **kwargs): - super(FormatChecker, self).__init__() - - @self.checks('fuel_task_role_format') - def _validate_role(instance): - sre_msg = None - if isinstance(instance, six.string_types): - if instance.startswith('/') and instance.endswith('/'): - try: - if re.compile(instance[1:-1]): - return True - except sre_error as e: - sre_msg = str(e) - else: - for role_pattern in role_patterns: - if re.match(role_pattern, instance): - return True - err_msg = "Role field should be either a valid " \ - "regexp enclosed by " \ - "slashes or a string of '{0}' or an " \ - "array of those. " \ - "Got '{1}' instead.".format(", ".join(role_patterns), - instance) - if sre_msg: - err_msg += "SRE error: \"{0}\"".format(sre_msg) - raise errors.TaskFieldError(err_msg) diff --git a/fuel_plugin_builder/validators/schemas/v4.py b/fuel_plugin_builder/validators/schemas/v4.py index 6f046df..6f206e2 100644 --- a/fuel_plugin_builder/validators/schemas/v4.py +++ b/fuel_plugin_builder/validators/schemas/v4.py @@ -27,23 +27,16 @@ COMPATIBLE_COMPONENT_NAME_PATTERN = \ '^({0}):([0-9a-z_-]+:)*([0-9a-z_-]+|(\*)?)$'.format(COMPONENTS_TYPES_STR) -TASK_NAME_PATTERN = TASK_ROLE_PATTERN = '^[0-9a-zA-Z_-]+$|^\*$' +TASK_NAME_PATTERN = TASK_ROLE_PATTERN = '^[0-9a-zA-Z_-]+$' NETWORK_ROLE_PATTERN = '^[0-9a-z_-]+$' FILE_PERMISSIONS_PATTERN = '^[0-7]{4}$' TASK_VERSION_PATTERN = '^\d+.\d+.\d+$' STAGE_PATTERN = '^(post_deployment|pre_deployment)' \ '(/[-+]?([0-9]*\.[0-9]+|[0-9]+))?$' -DEFAULT_TASK_ROLE_GROUP_ASSIGNMENT = ('roles', 'groups', 'role') -TASK_OBLIGATORY_FIELDS = ['id', 'type'] - class SchemaV4(SchemaV3): - def __init__(self): - super(SchemaV4, self).__init__() - self.role_pattern = TASK_ROLE_PATTERN - @property def _task_relation(self): return { @@ -65,13 +58,13 @@ class SchemaV4(SchemaV3): 'oneOf': [ { 'type': 'string', - 'format': 'fuel_task_role_format' + 'enum': ['*', 'master', 'self'] }, { 'type': 'array', 'items': { 'type': 'string', - 'format': 'fuel_task_role_format' + 'pattern': TASK_ROLE_PATTERN } } ] @@ -102,9 +95,7 @@ class SchemaV4(SchemaV3): } } - def _gen_task_schema(self, task_types, required=None, - parameters=None, - required_any=DEFAULT_TASK_ROLE_GROUP_ASSIGNMENT): + def _gen_task_schema(self, task_types, required=None, parameters=None): """Generate deployment task schema using prototype. :param task_types: task types @@ -128,21 +119,11 @@ class SchemaV4(SchemaV3): } parameters.setdefault("properties", {}) parameters["properties"].setdefault("strategy", self._task_strategy) - task_specific_req_fields = list(set(TASK_OBLIGATORY_FIELDS + - (required or []))) - required_fields = [] - # Some tasks are ephemeral, so we can leave it as is without target - # groups|role|roles, others must have at least one such field - if required_any: - for group_name_alias in DEFAULT_TASK_ROLE_GROUP_ASSIGNMENT: - required_fields.append({"required": task_specific_req_fields - + [group_name_alias]}) - else: - required_fields.append({"required": task_specific_req_fields}) return { '$schema': 'http://json-schema.org/draft-04/schema#', 'type': 'object', + 'required': list(set(['id', 'type'] + (required or []))), 'properties': { 'type': {'enum': task_types}, 'id': { @@ -151,8 +132,6 @@ class SchemaV4(SchemaV3): 'version': { 'type': 'string', "pattern": TASK_VERSION_PATTERN}, 'role': self._task_role, - 'groups': self._task_role, - 'roles': self._task_role, 'required_for': self.task_group, 'requires': self.task_group, 'cross-depends': { @@ -169,8 +148,7 @@ class SchemaV4(SchemaV3): 'pattern': TASK_ROLE_PATTERN}}, 'reexecute_on': self._task_reexecute, 'parameters': parameters or {}, - }, - 'anyOf': required_fields + } } @property @@ -202,7 +180,7 @@ class SchemaV4(SchemaV3): def copy_files_task(self): return self._gen_task_schema( "copy_files", - ['parameters'], + ['role', 'parameters'], { 'type': 'object', 'required': ['files'], @@ -225,7 +203,7 @@ class SchemaV4(SchemaV3): @property def group_task(self): - return self._gen_task_schema("group", []) + return self._gen_task_schema("group", ['role']) @property def puppet_task(self): @@ -264,7 +242,7 @@ class SchemaV4(SchemaV3): def shell_task(self): return self._gen_task_schema( "shell", - [], + ['role'], { 'type': 'object', 'required': ['cmd'], @@ -287,13 +265,13 @@ class SchemaV4(SchemaV3): @property def stage_task(self): - return self._gen_task_schema("stage", required_any=()) + return self._gen_task_schema("stage") @property def sync_task(self): return self._gen_task_schema( "sync", - ['parameters'], + ['role', 'parameters'], { 'type': 'object', 'required': ['src', 'dst'], @@ -309,7 +287,7 @@ class SchemaV4(SchemaV3): def upload_file_task(self): return self._gen_task_schema( "upload_file", - ['parameters'], + ['role', 'parameters'], { 'type': 'object', 'required': ['path', 'data'], diff --git a/fuel_plugin_builder/validators/validator_v4.py b/fuel_plugin_builder/validators/validator_v4.py index 66ac17d..bb1ebb9 100644 --- a/fuel_plugin_builder/validators/validator_v4.py +++ b/fuel_plugin_builder/validators/validator_v4.py @@ -19,11 +19,9 @@ from os.path import join as join_path from fuel_plugin_builder import errors from fuel_plugin_builder import utils -from fuel_plugin_builder.validators.formatchecker import FormatChecker from fuel_plugin_builder.validators.schemas import SchemaV4 from fuel_plugin_builder.validators import ValidatorV3 - logger = logging.getLogger(__name__) @@ -32,8 +30,7 @@ class ValidatorV4(ValidatorV3): schema = SchemaV4() def __init__(self, *args, **kwargs): - super(ValidatorV4, self).__init__(format_checker=FormatChecker( - role_patterns=[self.schema.role_pattern]), *args, **kwargs) + super(ValidatorV4, self).__init__(*args, **kwargs) self.components_path = join_path(self.plugin_path, 'components.yaml') @property