fpb shows incorrect message if task doesn't have timeout field
* oneOf/anyOf statement in jsonschema provides hard to understand message, in order to solve this problem was created base jsonschema for parameters where timeout is required field * print error messages to stderr instead of stdout Closes-bug: #1396234 Change-Id: I6dc6b8b77cdd4576a239303726ec42f69a1a0e73
This commit is contained in:
parent
a25224557f
commit
f361ae8778
|
@ -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
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -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(
|
||||
|
|
|
@ -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')
|
||||
|
|
|
@ -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):
|
||||
|
|
|
@ -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': [
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in New Issue