Fixes #2203 - properly validate files when multiple files are used.

Remove the single-use decorators so the functionality can be used directly as a function.

Signed-off-by: Daniel Nephin <[email protected]>

compose/config/config.py

@@ -14,7 +14,6 @@ from .validation import validate_against_fields_schema
 from .validation import validate_against_service_schema
 from .validation import validate_extended_service_exists
 from .validation import validate_extends_file_path
-from .validation import validate_service_names
 from .validation import validate_top_level_object
 
 
@@ -165,16 +164,6 @@ def find_candidates_in_parent_dirs(filenames, path):
     return (candidates, path)
 
 
-@validate_top_level_object
-@validate_service_names
-def pre_process_config(config):
-    """
-    Pre validation checks and processing of the config file to interpolate env
-    vars returning a config dict ready to be tested against the schema.
-    """
-    return interpolate_environment_variables(config)
-
-
 def load(config_details):
     """Load the configuration from a working directory and a list of
     configuration files.  Files are loaded in order, and merged on top
@@ -194,7 +183,7 @@ def load(config_details):
         return service_dict
 
     def load_file(filename, config):
-        processed_config = pre_process_config(config)
+        processed_config = interpolate_environment_variables(config)
         validate_against_fields_schema(processed_config)
         return [
             build_service(filename, name, service_config)
@@ -209,7 +198,10 @@ def load(config_details):
         }
 
     config_file = config_details.config_files[0]
+    validate_top_level_object(config_file.config)
     for next_file in config_details.config_files[1:]:
+        validate_top_level_object(next_file.config)
+
         config_file = ConfigFile(
             config_file.filename,
             merge_services(config_file.config, next_file.config))
@@ -283,9 +275,9 @@ class ServiceLoader(object):
         )
         self.extended_service_name = extends['service']
 
-        full_extended_config = pre_process_config(
-            load_yaml(self.extended_config_path)
-        )
+        config = load_yaml(self.extended_config_path)
+        validate_top_level_object(config)
+        full_extended_config = interpolate_environment_variables(config)
 
         validate_extended_service_exists(
             self.extended_service_name,

compose/config/validation.py

@@ -2,7 +2,6 @@ import json
 import logging
 import os
 import sys
-from functools import wraps
 
 import six
 from docker.utils.ports import split_port
@@ -65,27 +64,21 @@ def format_boolean_in_environment(instance):
     return True
 
 
-def validate_service_names(func):
-    @wraps(func)
-    def func_wrapper(config):
-        for service_name in config.keys():
-            if type(service_name) is int:
-                raise ConfigurationError(
-                    "Service name: {} needs to be a string, eg '{}'".format(service_name, service_name)
-                )
-        return func(config)
-    return func_wrapper
+def validate_service_names(config):
+    for service_name in config.keys():
+        if not isinstance(service_name, six.string_types):
+            raise ConfigurationError(
+                "Service name: {} needs to be a string, eg '{}'".format(
+                    service_name,
+                    service_name))
 
 
-def validate_top_level_object(func):
-    @wraps(func)
-    def func_wrapper(config):
-        if not isinstance(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."
-            )
-        return func(config)
-    return func_wrapper
+def validate_top_level_object(config):
+    if not isinstance(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)
 
 
 def validate_extends_file_path(service_name, extends_options, filename):

tests/unit/config/config_test.py

@@ -134,6 +134,28 @@ class ConfigTest(unittest.TestCase):
         ]
         self.assertEqual(service_sort(service_dicts), service_sort(expected))
 
+    def test_load_with_multiple_files_and_empty_override(self):
+        base_file = config.ConfigFile(
+            'base.yaml',
+            {'web': {'image': 'example/web'}})
+        override_file = config.ConfigFile('override.yaml', 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()
+
+    def test_load_with_multiple_files_and_empty_base(self):
+        base_file = config.ConfigFile('base.yaml', None)
+        override_file = config.ConfigFile(
+            'override.yaml',
+            {'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()
+
     def test_config_valid_service_names(self):
         for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']:
             config.load(