From 96e9b470597594beaade8e1851cbb2b3f5c3b37c Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 9 Nov 2015 20:01:20 -0500 Subject: [PATCH] Inclide the filename in validation errors. Signed-off-by: Daniel Nephin --- compose/config/config.py | 4 +- compose/config/interpolation.py | 7 --- compose/config/validation.py | 60 ++++++++++++------ tests/unit/config/config_test.py | 104 +++++++++++++++---------------- 4 files changed, 95 insertions(+), 80 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 21788551d..2c1fdeb9c 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -229,9 +229,9 @@ def load(config_details): def process_config_file(config_file, service_name=None): - validate_top_level_object(config_file.config) + validate_top_level_object(config_file) processed_config = interpolate_environment_variables(config_file.config) - validate_against_fields_schema(processed_config) + validate_against_fields_schema(processed_config, config_file.filename) if service_name and service_name not in processed_config: raise ConfigurationError( diff --git a/compose/config/interpolation.py b/compose/config/interpolation.py index f8e1da610..ba7e35c1e 100644 --- a/compose/config/interpolation.py +++ b/compose/config/interpolation.py @@ -18,13 +18,6 @@ def interpolate_environment_variables(config): def process_service(service_name, service_dict, mapping): - if not isinstance(service_dict, dict): - raise ConfigurationError( - 'Service "%s" doesn\'t have any configuration options. ' - 'All top level keys in your docker-compose.yml must map ' - 'to a dictionary of configuration options.' % service_name - ) - return dict( (key, interpolate_value(service_name, key, val, mapping)) for (key, val) in service_dict.items() diff --git a/compose/config/validation.py b/compose/config/validation.py index 2928238c3..38866b0f4 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -66,21 +66,38 @@ def format_boolean_in_environment(instance): return True -def validate_service_names(config): - for service_name in config.keys(): +def validate_top_level_service_objects(config_file): + """Perform some high level validation of the service name and value. + + This validation must happen before interpolation, which must happen + before the rest of validation, which is why it's separate from the + rest of the service validation. + """ + for service_name, service_dict in config_file.config.items(): if not isinstance(service_name, six.string_types): raise ConfigurationError( - "Service name: {} needs to be a string, eg '{}'".format( + "In file '{}' service name: {} needs to be a string, eg '{}'".format( + config_file.filename, service_name, service_name)) + if not isinstance(service_dict, dict): + raise ConfigurationError( + "In file '{}' service '{}' doesn\'t have any configuration options. " + "All top level keys in your docker-compose.yml must map " + "to a dictionary of configuration options.".format( + config_file.filename, + service_name)) -def validate_top_level_object(config): - if not isinstance(config, dict): + +def validate_top_level_object(config_file): + if not isinstance(config_file.config, dict): raise ConfigurationError( - "Top level object needs to be a dictionary. Check your .yml file " - "that you have defined a service at the top level.") - validate_service_names(config) + "Top level object in '{}' needs to be an object not '{}'. Check " + "that you have defined a service at the top level.".format( + config_file.filename, + type(config_file.config))) + validate_top_level_service_objects(config_file) def validate_extends_file_path(service_name, extends_options, filename): @@ -252,26 +269,28 @@ def process_errors(errors, service_name=None): return '\n'.join(format_error_message(error, service_name) for error in errors) -def validate_against_fields_schema(config): - return _validate_against_schema( +def validate_against_fields_schema(config, filename): + _validate_against_schema( config, "fields_schema.json", - ["ports", "environment"]) + format_checker=["ports", "environment"], + filename=filename) def validate_against_service_schema(config, service_name): - return _validate_against_schema( + _validate_against_schema( config, "service_schema.json", - ["ports"], - service_name) + format_checker=["ports"], + service_name=service_name) def _validate_against_schema( config, schema_filename, format_checker=(), - service_name=None): + service_name=None, + filename=None): config_source_dir = os.path.dirname(os.path.abspath(__file__)) if sys.platform == "win32": @@ -293,6 +312,11 @@ def _validate_against_schema( format_checker=FormatChecker(format_checker)) errors = [error for error in sorted(validation_output.iter_errors(config), key=str)] - if errors: - error_msg = process_errors(errors, service_name) - raise ConfigurationError("Validation failed, reason(s):\n{}".format(error_msg)) + if not errors: + return + + error_msg = process_errors(errors, service_name) + file_msg = " in file '{}'".format(filename) if filename else '' + raise ConfigurationError("Validation failed{}, reason(s):\n{}".format( + file_msg, + error_msg)) diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index ada5e9cae..3038af80d 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -94,6 +94,7 @@ class ConfigTest(unittest.TestCase): config.load(config_details) error_msg = "Unsupported config option for 'web' service: 'name'" assert error_msg in exc.exconly() + assert "Validation failed in file 'filename.yml'" in exc.exconly() def test_load_invalid_service_definition(self): config_details = build_config_details( @@ -102,11 +103,12 @@ class ConfigTest(unittest.TestCase): 'filename.yml') with pytest.raises(ConfigurationError) as exc: config.load(config_details) - error_msg = "Service \"web\" doesn\'t have any configuration options" + error_msg = "service 'web' doesn't have any configuration options" assert error_msg in exc.exconly() def test_config_integer_service_name_raise_validation_error(self): - expected_error_msg = "Service name: 1 needs to be a string, eg '1'" + expected_error_msg = ("In file 'filename.yml' service name: 1 needs to " + "be a string, eg '1'") with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): config.load( build_config_details( @@ -156,25 +158,26 @@ class ConfigTest(unittest.TestCase): def test_load_with_multiple_files_and_empty_override(self): base_file = config.ConfigFile( - 'base.yaml', + 'base.yml', {'web': {'image': 'example/web'}}) - override_file = config.ConfigFile('override.yaml', None) + override_file = config.ConfigFile('override.yml', None) details = config.ConfigDetails('.', [base_file, override_file]) with pytest.raises(ConfigurationError) as exc: config.load(details) - assert 'Top level object needs to be a dictionary' in exc.exconly() + error_msg = "Top level object in 'override.yml' needs to be an object" + assert error_msg in exc.exconly() def test_load_with_multiple_files_and_empty_base(self): - base_file = config.ConfigFile('base.yaml', None) + base_file = config.ConfigFile('base.yml', None) override_file = config.ConfigFile( - 'override.yaml', + 'override.yml', {'web': {'image': 'example/web'}}) details = config.ConfigDetails('.', [base_file, override_file]) with pytest.raises(ConfigurationError) as exc: config.load(details) - assert 'Top level object needs to be a dictionary' in exc.exconly() + assert "Top level object in 'base.yml' needs to be an object" in exc.exconly() def test_load_with_multiple_files_and_extends_in_override_file(self): base_file = config.ConfigFile( @@ -225,17 +228,17 @@ class ConfigTest(unittest.TestCase): with pytest.raises(ConfigurationError) as exc: config.load(details) - assert 'Service "bogus" doesn\'t have any configuration' in exc.exconly() + assert "service 'bogus' doesn't have any configuration" in exc.exconly() + assert "In file 'override.yaml'" in exc.exconly() def test_config_valid_service_names(self): for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']: - config.load( + services = config.load( build_config_details( {valid_name: {'image': 'busybox'}}, 'tests/fixtures/extends', - 'common.yml' - ) - ) + 'common.yml')) + assert services[0]['name'] == valid_name def test_config_invalid_ports_format_validation(self): expected_error_msg = "Service 'web' configuration key 'ports' contains an invalid type" @@ -300,7 +303,8 @@ class ConfigTest(unittest.TestCase): ) def test_invalid_config_not_a_dictionary(self): - expected_error_msg = "Top level object needs to be a dictionary." + expected_error_msg = ("Top level object in 'filename.yml' needs to be " + "an object.") with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): config.load( build_config_details( @@ -382,12 +386,13 @@ class ConfigTest(unittest.TestCase): ) def test_config_ulimits_invalid_keys_validation_error(self): - expected_error_msg = "Service 'web' configuration key 'ulimits' contains unsupported option: 'not_soft_or_hard'" + expected = ("Service 'web' configuration key 'ulimits' 'nofile' contains " + "unsupported option: 'not_soft_or_hard'") - with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): - config.load( - build_config_details( - {'web': { + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details( + { + 'web': { 'image': 'busybox', 'ulimits': { 'nofile': { @@ -396,50 +401,43 @@ class ConfigTest(unittest.TestCase): "hard": 20000, } } - }}, - 'working_dir', - 'filename.yml' - ) - ) + } + }, + 'working_dir', + 'filename.yml')) + assert expected in exc.exconly() def test_config_ulimits_required_keys_validation_error(self): - expected_error_msg = "Service 'web' configuration key 'ulimits' u?'hard' is a required property" - with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): - config.load( - build_config_details( - {'web': { + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details( + { + 'web': { 'image': 'busybox', - 'ulimits': { - 'nofile': { - "soft": 10000, - } - } - }}, - 'working_dir', - 'filename.yml' - ) - ) + 'ulimits': {'nofile': {"soft": 10000}} + } + }, + 'working_dir', + 'filename.yml')) + assert "Service 'web' configuration key 'ulimits' 'nofile'" in exc.exconly() + assert "'hard' is a required property" in exc.exconly() def test_config_ulimits_soft_greater_than_hard_error(self): - expected_error_msg = "cannot contain a 'soft' value higher than 'hard' value" + expected = "cannot contain a 'soft' value higher than 'hard' value" - with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): - config.load( - build_config_details( - {'web': { + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details( + { + 'web': { 'image': 'busybox', 'ulimits': { - 'nofile': { - "soft": 10000, - "hard": 1000 - } + 'nofile': {"soft": 10000, "hard": 1000} } - }}, - 'working_dir', - 'filename.yml' - ) - ) + } + }, + 'working_dir', + 'filename.yml')) + assert expected in exc.exconly() def test_valid_config_which_allows_two_type_definitions(self): expose_values = [["8000"], [8000]]