Inclide the filename in validation errors.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

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(

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()

compose/config/validation.py

@@ -66,21 +66,38 @@ def format_boolean_in_environment(instance):
     return True
 
 
-def validate_service_names(config):
+def validate_top_level_service_objects(config_file):
-    for service_name in config.keys():
+    """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 "
+            "Top level object in '{}' needs to be an object not '{}'. Check "
-            "that you have defined a service at the top level.")
+            "that you have defined a service at the top level.".format(
-    validate_service_names(config)
+                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):
+def validate_against_fields_schema(config, filename):
-    return _validate_against_schema(
+    _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"],
+        format_checker=["ports"],
-        service_name)
+        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:
+    if not errors:
-        error_msg = process_errors(errors, service_name)
+        return
-        raise ConfigurationError("Validation failed, reason(s):\n{}".format(error_msg))
+
+    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))

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):
+        with pytest.raises(ConfigurationError) as exc:
-            config.load(
+            config.load(build_config_details(
-                build_config_details(
+                {
-                    {'web': {
+                    '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):
+        with pytest.raises(ConfigurationError) as exc:
-            config.load(
+            config.load(build_config_details(
-                build_config_details(
+                {
-                    {'web': {
+                    'web': {
                         'image': 'busybox',
-                        'ulimits': {
+                        'ulimits': {'nofile': {"soft": 10000}}
-                            'nofile': {
+                    }
-                                "soft": 10000,
+                },
-                            }
+                'working_dir',
-                        }
+                'filename.yml'))
-                    }},
+        assert "Service 'web' configuration key 'ulimits' 'nofile'" in exc.exconly()
-                    'working_dir',
+        assert "'hard' is a required property" in exc.exconly()
-                    'filename.yml'
-                )
-            )
 
     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):
+        with pytest.raises(ConfigurationError) as exc:
-            config.load(
+            config.load(build_config_details(
-                build_config_details(
+                {
-                    {'web': {
+                    'web': {
                         'image': 'busybox',
                         'ulimits': {
-                            'nofile': {
+                            'nofile': {"soft": 10000, "hard": 1000}
-                                "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]]