diff --git a/fuel_plugin_builder/CHANGELOG.md b/fuel_plugin_builder/CHANGELOG.md index 3670b36..e1f2c8e 100644 --- a/fuel_plugin_builder/CHANGELOG.md +++ b/fuel_plugin_builder/CHANGELOG.md @@ -2,6 +2,10 @@ ## 1.0.2 (UNRELEASED) +- Show correct message, if 'timeout' field is not specified for task in tasks.yaml + https://bugs.launchpad.net/fuel/+bug/1396234 +- Print error messages to stderr instead of stdout + ## 1.0.1 (2014-11-20) - Show instruction for CentOS if not all requirements are installed diff --git a/fuel_plugin_builder/fuel_plugin_builder/cli.py b/fuel_plugin_builder/fuel_plugin_builder/cli.py index cca4047..2e039c8 100644 --- a/fuel_plugin_builder/fuel_plugin_builder/cli.py +++ b/fuel_plugin_builder/fuel_plugin_builder/cli.py @@ -16,6 +16,7 @@ import argparse import logging +import six import sys from fuel_plugin_builder import actions @@ -28,12 +29,25 @@ from fuel_plugin_builder.logger import configure_logger logger = logging.getLogger(__name__) +def print_err(line): + sys.stderr.write(six.text_type(line)) + sys.stderr.write('\n') + + def handle_exception(exc): logger.exception(exc) if isinstance(exc, errors.FuelCannotFindCommandError): - print(messages.header) - print(messages.install_required_packages) + print_err(messages.header) + print_err(messages.install_required_packages) + + elif isinstance(exc, errors.ValidationError): + print_err('Validation failed') + print_err(exc) + + else: + print_err('Unexpected error') + print_err(exc) sys.exit(-1) diff --git a/fuel_plugin_builder/fuel_plugin_builder/logger.py b/fuel_plugin_builder/fuel_plugin_builder/logger.py index 85a9650..1f588f9 100644 --- a/fuel_plugin_builder/fuel_plugin_builder/logger.py +++ b/fuel_plugin_builder/fuel_plugin_builder/logger.py @@ -20,7 +20,7 @@ import logging def configure_logger(debug=False): logger = logging.getLogger('fuel_plugin_builder') - logger.setLevel(logging.INFO) + logger.setLevel(logging.CRITICAL) if debug: logger.setLevel(logging.DEBUG) diff --git a/fuel_plugin_builder/fuel_plugin_builder/tests/test_base_validator.py b/fuel_plugin_builder/fuel_plugin_builder/tests/test_base_validator.py index b5ab90a..913651f 100644 --- a/fuel_plugin_builder/fuel_plugin_builder/tests/test_base_validator.py +++ b/fuel_plugin_builder/fuel_plugin_builder/tests/test_base_validator.py @@ -14,7 +14,6 @@ # License for the specific language governing permissions and limitations # under the License. -import jsonschema import mock from fuel_plugin_builder import errors @@ -32,7 +31,15 @@ class TestBaseValidator(BaseTestCase): self.plugin_path = '/tmp/plugin_path' self.validator = NewValidator(self.plugin_path) self.data = {'data': 'data1'} - self.schema = {'schema': 'schema1'} + self.schema = self.make_schema(['data'], {'data': {'type': 'string'}}) + + @classmethod + def make_schema(cls, required, properties): + return { + '$schema': 'http://json-schema.org/draft-04/schema#', + 'type': 'object', + 'required': required, + 'properties': properties} @mock.patch('fuel_plugin_builder.validators.base.jsonschema') def test_validate_schema(self, schema_mock): @@ -44,16 +51,38 @@ class TestBaseValidator(BaseTestCase): self.data, self.schema) - @mock.patch('fuel_plugin_builder.validators.base.jsonschema.validate', - side_effect=jsonschema.exceptions.ValidationError('p1', 'p2')) - def test_validate_schema_raises_error(self, validate_mock): + def test_validate_schema_raises_error(self): + schema = self.make_schema(['key'], {'key': {'type': 'string'}}) + data = {} + with self.assertRaisesRegexp( errors.ValidationError, - 'Wrong value format "", for file "file_path", p1'): - self.validator.validate_schema(self.data, self.schema, 'file_path') - validate_mock.assert_called_once_with( - self.data, - self.schema) + "File 'file_path', 'key' is a required property"): + self.validator.validate_schema(data, schema, 'file_path') + + def test_validate_schema_raises_error_path_in_message(self): + schema = self.make_schema( + ['key'], + {'key': {'type': 'array', 'items': {'type': 'string'}}}) + data = {'key': ['str', 'str', 0]} + + expected_error = ("File 'file_path', 0 is not of type " + "'string', value path 'key -> 2'") + with self.assertRaisesRegexp( + errors.ValidationError, + expected_error): + self.validator.validate_schema(data, schema, 'file_path') + + def test_validate_schema_raises_error_custom_value_path(self): + schema = self.make_schema(['key'], {'key': {'type': 'string'}}) + data = {} + + with self.assertRaisesRegexp( + errors.ValidationError, + "File 'file_path', 'key' is a required property, " + "value path '0 -> path2'"): + self.validator.validate_schema( + data, schema, 'file_path', value_path=[0, 'path2']) @mock.patch('fuel_plugin_builder.validators.base.utils') @mock.patch( diff --git a/fuel_plugin_builder/fuel_plugin_builder/tests/test_validator_v1.py b/fuel_plugin_builder/fuel_plugin_builder/tests/test_validator_v1.py index c2473b3..4c49d83 100644 --- a/fuel_plugin_builder/fuel_plugin_builder/tests/test_validator_v1.py +++ b/fuel_plugin_builder/fuel_plugin_builder/tests/test_validator_v1.py @@ -65,9 +65,11 @@ class TestValidatorV1(BaseTestCase): self.assertEqual( [mock.call('param1', v1.PUPPET_PARAMETERS, - self.validator.tasks_path), + self.validator.tasks_path, + value_path=[0, 'parameters']), mock.call('param2', v1.SHELL_PARAMETERS, - self.validator.tasks_path)], + self.validator.tasks_path, + value_path=[1, 'parameters'])], validate_schema_mock.call_args_list) @mock.patch('fuel_plugin_builder.validators.validator_v1.utils') diff --git a/fuel_plugin_builder/fuel_plugin_builder/validators/base.py b/fuel_plugin_builder/fuel_plugin_builder/validators/base.py index c38894c..654d827 100644 --- a/fuel_plugin_builder/fuel_plugin_builder/validators/base.py +++ b/fuel_plugin_builder/fuel_plugin_builder/validators/base.py @@ -32,19 +32,32 @@ class BaseValidator(object): def __init__(self, plugin_path): self.plugin_path = plugin_path - def validate_schema(self, data, schema, path): - logger.debug('Start schema validation for %s file, %s', path, schema) + 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) except jsonschema.exceptions.ValidationError as exc: - value_path = ' -> '.join(map(six.text_type, exc.absolute_path)) raise errors.ValidationError( - 'Wrong value format "{0}", for file "{1}", {2}'.format( - value_path, path, exc.message)) + self._make_error_message(exc, file_path, value_path)) - def validate_file_by_schema(self, schema, path): - data = utils.parse_yaml(path) - self.validate_schema(data, schema, path) + def _make_error_message(self, exc, file_path, value_path): + error_msg = "File '{0}', {1}".format(file_path, exc.message) + + if value_path is None and exc.absolute_path: + value_path = exc.absolute_path + + if value_path: + value_path = ' -> '.join(map(six.text_type, value_path)) + error_msg = '{0}, {1}'.format( + error_msg, "value path '{0}'".format(value_path)) + + return error_msg + + def validate_file_by_schema(self, schema, file_path): + data = utils.parse_yaml(file_path) + self.validate_schema(data, schema, file_path) @abc.abstractmethod def validate(self): diff --git a/fuel_plugin_builder/fuel_plugin_builder/validators/schemas/v1.py b/fuel_plugin_builder/fuel_plugin_builder/validators/schemas/v1.py index c1e9c00..f6b8e87 100644 --- a/fuel_plugin_builder/fuel_plugin_builder/validators/schemas/v1.py +++ b/fuel_plugin_builder/fuel_plugin_builder/validators/schemas/v1.py @@ -69,17 +69,21 @@ SHELL_PARAMETERS = { 'cmd': {'type': 'string'}}} +TASK_BASE_PARAMETERS = { + '$schema': 'http://json-schema.org/draft-04/schema#', + 'type': 'object', + 'required': ['timeout'], + 'properties': { + 'timeout': POSITIVE_INTEGER}} + + TASK_SCHEMA = { '$schema': 'http://json-schema.org/draft-04/schema#', 'type': 'object', 'required': ['parameters', 'type', 'stage', 'role'], 'properties': { 'type': {'enum': ['puppet', 'shell']}, - 'parameters': { - 'type': 'object', - 'oneOf': [ - PUPPET_PARAMETERS, - SHELL_PARAMETERS]}, + 'parameters': TASK_BASE_PARAMETERS, 'stage': {'enum': ['post_deployment', 'pre_deployment']}, 'role': { 'oneOf': [ diff --git a/fuel_plugin_builder/fuel_plugin_builder/validators/validator_v1.py b/fuel_plugin_builder/fuel_plugin_builder/validators/validator_v1.py index b0b44f5..9ce77d3 100644 --- a/fuel_plugin_builder/fuel_plugin_builder/validators/validator_v1.py +++ b/fuel_plugin_builder/fuel_plugin_builder/validators/validator_v1.py @@ -54,13 +54,17 @@ class ValidatorV1(BaseValidator): logger.debug('Start tasks checking "%s"', self.tasks_path) tasks = utils.parse_yaml(self.tasks_path) - for task in tasks: + for idx, task in enumerate(tasks): if task['type'] == 'puppet': schema = v1.PUPPET_PARAMETERS elif task['type'] == 'shell': schema = v1.SHELL_PARAMETERS - self.validate_schema(task['parameters'], schema, self.tasks_path) + self.validate_schema( + task['parameters'], + schema, + self.tasks_path, + value_path=[idx, 'parameters']) def check_releases_paths(self): meta = utils.parse_yaml(self.meta_path)