Validate additional files before merging them.

Consolidates all the top level config handling into `process_config_file` which
is now used for both files and merge sources.

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

compose/cli/main.py

@@ -13,12 +13,12 @@ from requests.exceptions import ReadTimeout
 
 from .. import __version__
 from .. import legacy
+from ..config import ConfigurationError
 from ..config import parse_environment
 from ..const import DEFAULT_TIMEOUT
 from ..const import HTTP_TIMEOUT
 from ..const import IS_WINDOWS_PLATFORM
 from ..progress_stream import StreamOutputError
-from ..project import ConfigurationError
 from ..project import NoSuchService
 from ..service import BuildError
 from ..service import ConvergenceStrategy

compose/config/__init__.py

@@ -1,5 +1,4 @@
 # flake8: noqa
-from .config import ConfigDetails
 from .config import ConfigurationError
 from .config import DOCKER_CONFIG_KEYS
 from .config import find

compose/config/config.py

@@ -13,7 +13,6 @@ from .errors import ConfigurationError
 from .interpolation import interpolate_environment_variables
 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_top_level_object
 
@@ -99,6 +98,10 @@ class ConfigFile(namedtuple('_ConfigFile', 'filename config')):
     :type  config: :class:`dict`
     """
 
+    @classmethod
+    def from_filename(cls, filename):
+        return cls(filename, load_yaml(filename))
+
 
 def find(base_dir, filenames):
     if filenames == ['-']:
@@ -114,7 +117,7 @@ def find(base_dir, filenames):
     log.debug("Using configuration files: {}".format(",".join(filenames)))
     return ConfigDetails(
         os.path.dirname(filenames[0]),
-        [ConfigFile(f, load_yaml(f)) for f in filenames])
+        [ConfigFile.from_filename(f) for f in filenames])
 
 
 def get_default_config_files(base_dir):
@@ -183,12 +186,10 @@ def load(config_details):
         validate_paths(service_dict)
         return service_dict
 
-    def load_file(filename, config):
+    def build_services(filename, config):
-        processed_config = interpolate_environment_variables(config)
-        validate_against_fields_schema(processed_config)
         return [
             build_service(filename, name, service_config)
-            for name, service_config in processed_config.items()
+            for name, service_config in config.items()
         ]
 
     def merge_services(base, override):
@@ -200,16 +201,27 @@ def load(config_details):
             for name in all_service_names
         }
 
-    config_file = config_details.config_files[0]
+    config_file = process_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)
+        next_file = process_config_file(next_file)
 
-        config_file = ConfigFile(
+        config = merge_services(config_file.config, next_file.config)
-            config_file.filename,
+        config_file = config_file._replace(config=config)
-            merge_services(config_file.config, next_file.config))
 
-    return load_file(config_file.filename, config_file.config)
+    return build_services(config_file.filename, config_file.config)
+
+
+def process_config_file(config_file, service_name=None):
+    validate_top_level_object(config_file.config)
+    processed_config = interpolate_environment_variables(config_file.config)
+    validate_against_fields_schema(processed_config)
+
+    if service_name and service_name not in processed_config:
+        raise ConfigurationError(
+            "Cannot extend service '{}' in {}: Service not found".format(
+                service_name, config_file.filename))
+
+    return config_file._replace(config=processed_config)
 
 
 class ServiceLoader(object):
@@ -259,22 +271,13 @@ class ServiceLoader(object):
         if not isinstance(extends, dict):
             extends = {'service': extends}
 
-        validate_extends_file_path(self.service_name, extends, self.filename)
         config_path = self.get_extended_config_path(extends)
         service_name = extends['service']
 
-        config = load_yaml(config_path)
+        extended_file = process_config_file(
-        validate_top_level_object(config)
+            ConfigFile.from_filename(config_path),
-        full_extended_config = interpolate_environment_variables(config)
+            service_name=service_name)
-
+        service_config = extended_file.config[service_name]
-        validate_extended_service_exists(
-            service_name,
-            full_extended_config,
-            config_path
-        )
-        validate_against_fields_schema(full_extended_config)
-
-        service_config = full_extended_config[service_name]
         return config_path, service_config, service_name
 
     def resolve_extends(self, extended_config_path, service_config, service_name):
@@ -304,6 +307,7 @@ class ServiceLoader(object):
         need to obtain a full path too or we are extending from a service
         defined in our own file.
         """
+        validate_extends_file_path(self.service_name, extends_options, self.filename)
         if 'file' in extends_options:
             return expand_path(self.working_dir, extends_options['file'])
         return self.filename

compose/config/validation.py

@@ -96,14 +96,6 @@ def validate_extends_file_path(service_name, extends_options, filename):
         )
 
 
-def validate_extended_service_exists(extended_service_name, full_extended_config, extended_config_path):
-    if extended_service_name not in full_extended_config:
-        msg = (
-            "Cannot extend service '%s' in %s: Service not found"
-        ) % (extended_service_name, extended_config_path)
-        raise ConfigurationError(msg)
-
-
 def get_unsupported_config_msg(service_name, error_key):
     msg = "Unsupported config option for '{}' service: '{}'".format(service_name, error_key)
     if error_key in DOCKER_CONFIG_HINTS:
@@ -264,7 +256,7 @@ def process_errors(errors, service_name=None):
                             msg))
                 else:
                     root_msgs.append(
-                        "Service '{}' doesn\'t have any configuration options. "
+                        "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(service_name))
             elif error.validator == 'required':

tests/unit/config/config_test.py

@@ -195,6 +195,19 @@ class ConfigTest(unittest.TestCase):
         ]
         self.assertEqual(service_sort(service_dicts), service_sort(expected))
 
+    def test_load_with_multiple_files_and_invalid_override(self):
+        base_file = config.ConfigFile(
+            'base.yaml',
+            {'web': {'image': 'example/web'}})
+        override_file = config.ConfigFile(
+            'override.yaml',
+            {'bogus': 'thing'})
+        details = config.ConfigDetails('.', [base_file, override_file])
+
+        with pytest.raises(ConfigurationError) as exc:
+            config.load(details)
+        assert 'Service "bogus" doesn\'t have any configuration' in exc.exconly()
+
     def test_config_valid_service_names(self):
         for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']:
             config.load(