From c907f35e741ff2f40c775c07d311f53a0d4c2373 Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Thu, 20 Aug 2015 15:05:33 +0100
Subject: [PATCH 01/16] Raise if working_dir is None

Check for this in the init so we can remove the duplication of
raising in further functions.

A ServiceLoader isn't valid without one.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/config.py | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/compose/config/config.py b/compose/config/config.py
index cfa8086f0..e08b503f3 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -152,7 +152,11 @@ def load(config_details):
 
 class ServiceLoader(object):
     def __init__(self, working_dir, filename=None, already_seen=None):
+        if working_dir is None:
+            raise Exception("No working_dir passed to ServiceLoader()")
+
         self.working_dir = os.path.abspath(working_dir)
+
         if filename:
             self.filename = os.path.abspath(filename)
         else:
@@ -176,9 +180,6 @@ class ServiceLoader(object):
 
         extends_options = self.validate_extends_options(service_dict['name'], service_dict['extends'])
 
-        if self.working_dir is None:
-            raise Exception("No working_dir passed to ServiceLoader()")
-
         if 'file' in extends_options:
             extends_from_filename = extends_options['file']
             other_config_path = expand_path(self.working_dir, extends_from_filename)
@@ -320,9 +321,6 @@ def get_env_files(options, working_dir=None):
     if 'env_file' not in options:
         return {}
 
-    if working_dir is None:
-        raise Exception("No working_dir passed to get_env_files()")
-
     env_files = options.get('env_file', [])
     if not isinstance(env_files, list):
         env_files = [env_files]

From 1344533b240ddc344029536df8361125617e1a3d Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Fri, 21 Aug 2015 17:04:10 +0100
Subject: [PATCH 02/16] filename is not optional

While it can be set to ultimately a value of None, when a
config file is read in from stdin, it is not optional.

We kinda make use of it's ability to be set to None in our
tests but functionally and design wise, it is required.

If filename is not set, extends does not work.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/config.py       | 2 +-
 tests/integration/testcases.py | 2 +-
 tests/unit/config_test.py      | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/compose/config/config.py b/compose/config/config.py
index e08b503f3..b7697f002 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -151,7 +151,7 @@ def load(config_details):
 
 
 class ServiceLoader(object):
-    def __init__(self, working_dir, filename=None, already_seen=None):
+    def __init__(self, working_dir, filename, already_seen=None):
         if working_dir is None:
             raise Exception("No working_dir passed to ServiceLoader()")
 
diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py
index 08ef9f272..d9d666d27 100644
--- a/tests/integration/testcases.py
+++ b/tests/integration/testcases.py
@@ -31,7 +31,7 @@ class DockerClientTestCase(unittest.TestCase):
         if 'command' not in kwargs:
             kwargs['command'] = ["top"]
 
-        options = ServiceLoader(working_dir='.').make_service_dict(name, kwargs)
+        options = ServiceLoader(working_dir='.', filename=None).make_service_dict(name, kwargs)
 
         labels = options.setdefault('labels', {})
         labels['com.docker.compose.test-name'] = self.id()
diff --git a/tests/unit/config_test.py b/tests/unit/config_test.py
index e488ceb52..aa10982b4 100644
--- a/tests/unit/config_test.py
+++ b/tests/unit/config_test.py
@@ -11,11 +11,11 @@ from compose.config import config
 from compose.config.errors import ConfigurationError
 
 
-def make_service_dict(name, service_dict, working_dir):
+def make_service_dict(name, service_dict, working_dir, filename=None):
     """
     Test helper function to construct a ServiceLoader
     """
-    return config.ServiceLoader(working_dir=working_dir).make_service_dict(name, service_dict)
+    return config.ServiceLoader(working_dir=working_dir, filename=filename).make_service_dict(name, service_dict)
 
 
 def service_sort(services):

From 8a6061bfb9a3e4a98c11ad385eee45710af81e3f Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Fri, 21 Aug 2015 17:07:37 +0100
Subject: [PATCH 03/16] __init__ takes service name and dict

Moving service name and dict out of the function make_service_dict
and into __init__. We always call make_service_dict with those so
let's put them in the initialiser. Slightly cleaner design intent.

The whole purpose of the ServiceLoader is to take a
service name&service dictionary then validate, process and return
service dictionaries ready to be created.

This is also another step towards cleaning the code up so we can
interpolate and validate an extended dictionary.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/config.py       | 42 +++++++++++++++++++---------------
 tests/integration/testcases.py |  2 +-
 tests/unit/config_test.py      |  6 ++++-
 3 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/compose/config/config.py b/compose/config/config.py
index b7697f002..9d90bd614 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -142,8 +142,12 @@ def load(config_details):
     service_dicts = []
 
     for service_name, service_dict in list(processed_config.items()):
-        loader = ServiceLoader(working_dir=working_dir, filename=filename)
-        service_dict = loader.make_service_dict(service_name, service_dict)
+        loader = ServiceLoader(
+            working_dir=working_dir,
+            filename=filename,
+            service_name=service_name,
+            service_dict=service_dict)
+        service_dict = loader.make_service_dict()
         validate_paths(service_dict)
         service_dicts.append(service_dict)
 
@@ -151,7 +155,7 @@ def load(config_details):
 
 
 class ServiceLoader(object):
-    def __init__(self, working_dir, filename, already_seen=None):
+    def __init__(self, working_dir, filename, service_name, service_dict, already_seen=None):
         if working_dir is None:
             raise Exception("No working_dir passed to ServiceLoader()")
 
@@ -162,17 +166,19 @@ class ServiceLoader(object):
         else:
             self.filename = filename
         self.already_seen = already_seen or []
+        self.service_dict = service_dict.copy()
+        self.service_dict['name'] = service_name
 
     def detect_cycle(self, name):
         if self.signature(name) in self.already_seen:
             raise CircularReference(self.already_seen + [self.signature(name)])
 
-    def make_service_dict(self, name, service_dict):
-        service_dict = service_dict.copy()
-        service_dict['name'] = name
-        service_dict = resolve_environment(service_dict, working_dir=self.working_dir)
-        service_dict = self.resolve_extends(service_dict)
-        return process_container_options(service_dict, working_dir=self.working_dir)
+    def make_service_dict(self):
+        # service_dict = service_dict.copy()
+        # service_dict['name'] = name
+        self.service_dict = resolve_environment(self.service_dict, working_dir=self.working_dir)
+        self.service_dict = self.resolve_extends(self.service_dict)
+        return process_container_options(self.service_dict, working_dir=self.working_dir)
 
     def resolve_extends(self, service_dict):
         if 'extends' not in service_dict:
@@ -188,11 +194,6 @@ class ServiceLoader(object):
 
         other_working_dir = os.path.dirname(other_config_path)
         other_already_seen = self.already_seen + [self.signature(service_dict['name'])]
-        other_loader = ServiceLoader(
-            working_dir=other_working_dir,
-            filename=other_config_path,
-            already_seen=other_already_seen,
-        )
 
         base_service = extends_options['service']
         other_config = load_yaml(other_config_path)
@@ -204,11 +205,16 @@ class ServiceLoader(object):
             raise ConfigurationError(msg)
 
         other_service_dict = other_config[base_service]
-        other_loader.detect_cycle(extends_options['service'])
-        other_service_dict = other_loader.make_service_dict(
-            service_dict['name'],
-            other_service_dict,
+        other_loader = ServiceLoader(
+            working_dir=other_working_dir,
+            filename=other_config_path,
+            service_name=service_dict['name'],
+            service_dict=other_service_dict,
+            already_seen=other_already_seen,
         )
+
+        other_loader.detect_cycle(extends_options['service'])
+        other_service_dict = other_loader.make_service_dict()
         validate_extended_service_dict(
             other_service_dict,
             filename=other_config_path,
diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py
index d9d666d27..58240d5ea 100644
--- a/tests/integration/testcases.py
+++ b/tests/integration/testcases.py
@@ -31,7 +31,7 @@ class DockerClientTestCase(unittest.TestCase):
         if 'command' not in kwargs:
             kwargs['command'] = ["top"]
 
-        options = ServiceLoader(working_dir='.', filename=None).make_service_dict(name, kwargs)
+        options = ServiceLoader(working_dir='.', filename=None, service_name=name, service_dict=kwargs).make_service_dict()
 
         labels = options.setdefault('labels', {})
         labels['com.docker.compose.test-name'] = self.id()
diff --git a/tests/unit/config_test.py b/tests/unit/config_test.py
index aa10982b4..f3a4bd306 100644
--- a/tests/unit/config_test.py
+++ b/tests/unit/config_test.py
@@ -15,7 +15,11 @@ def make_service_dict(name, service_dict, working_dir, filename=None):
     """
     Test helper function to construct a ServiceLoader
     """
-    return config.ServiceLoader(working_dir=working_dir, filename=filename).make_service_dict(name, service_dict)
+    return config.ServiceLoader(
+        working_dir=working_dir,
+        filename=filename,
+        service_name=name,
+        service_dict=service_dict).make_service_dict()
 
 
 def service_sort(services):

From 02c52ae673a66c7a8f6455611d8561d8f6954383 Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Mon, 24 Aug 2015 12:23:45 +0100
Subject: [PATCH 04/16] Move resolve_environment within __init__

resolve_environment is specific to ServiceLoader, the function
does not need to be on the global scope, it is a part of the
ServiceLoader object. The environment needs to be resolved
before we can make any service dicts, it belongs in the
constructor.

This is cleaning up the design a little and being clearer
about intent and scope of functions.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/config.py | 45 ++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/compose/config/config.py b/compose/config/config.py
index 9d90bd614..ff9b35933 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -169,17 +169,36 @@ class ServiceLoader(object):
         self.service_dict = service_dict.copy()
         self.service_dict['name'] = service_name
 
+        self.resolve_environment()
+
     def detect_cycle(self, name):
         if self.signature(name) in self.already_seen:
             raise CircularReference(self.already_seen + [self.signature(name)])
 
     def make_service_dict(self):
-        # service_dict = service_dict.copy()
-        # service_dict['name'] = name
-        self.service_dict = resolve_environment(self.service_dict, working_dir=self.working_dir)
         self.service_dict = self.resolve_extends(self.service_dict)
         return process_container_options(self.service_dict, working_dir=self.working_dir)
 
+    def resolve_environment(self):
+        """
+        Unpack any environment variables from an env_file, if set.
+        Interpolate environment values if set.
+        """
+        if 'environment' not in self.service_dict and 'env_file' not in self.service_dict:
+            return
+
+        env = {}
+
+        if 'env_file' in self.service_dict:
+            for f in get_env_files(self.service_dict, working_dir=self.working_dir):
+                env.update(env_vars_from_file(f))
+            del self.service_dict['env_file']
+
+        env.update(parse_environment(self.service_dict.get('environment')))
+        env = dict(resolve_env_var(k, v) for k, v in six.iteritems(env))
+
+        self.service_dict['environment'] = env
+
     def resolve_extends(self, service_dict):
         if 'extends' not in service_dict:
             return service_dict
@@ -334,26 +353,6 @@ def get_env_files(options, working_dir=None):
     return [expand_path(working_dir, path) for path in env_files]
 
 
-def resolve_environment(service_dict, working_dir=None):
-    service_dict = service_dict.copy()
-
-    if 'environment' not in service_dict and 'env_file' not in service_dict:
-        return service_dict
-
-    env = {}
-
-    if 'env_file' in service_dict:
-        for f in get_env_files(service_dict, working_dir=working_dir):
-            env.update(env_vars_from_file(f))
-        del service_dict['env_file']
-
-    env.update(parse_environment(service_dict.get('environment')))
-    env = dict(resolve_env_var(k, v) for k, v in six.iteritems(env))
-
-    service_dict['environment'] = env
-    return service_dict
-
-
 def parse_environment(environment):
     if not environment:
         return {}

From 538a501eece5f645285f8235cf21507127750300 Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Mon, 24 Aug 2015 15:58:17 +0100
Subject: [PATCH 05/16] Refactor validating extends file path

Separating out the steps we need to resolve extends, so that it
will be clear to insert pre-processing of interpolation and
validation.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/config.py     | 36 ++++++++++++++++++------------------
 compose/config/validation.py | 13 +++++++++++++
 2 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/compose/config/config.py b/compose/config/config.py
index ff9b35933..51bd93846 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -11,6 +11,7 @@ from .errors import ComposeFileNotFound
 from .errors import ConfigurationError
 from .interpolation import interpolate_environment_variables
 from .validation import validate_against_schema
+from .validation import validate_extends_file_path
 from .validation import validate_service_names
 from .validation import validate_top_level_object
 from compose.cli.utils import find_candidates_in_parent_dirs
@@ -171,12 +172,20 @@ class ServiceLoader(object):
 
         self.resolve_environment()
 
+        if 'extends' in self.service_dict:
+            validate_extends_file_path(
+                service_name,
+                self.service_dict['extends'],
+                self.filename
+            )
+
+
     def detect_cycle(self, name):
         if self.signature(name) in self.already_seen:
             raise CircularReference(self.already_seen + [self.signature(name)])
 
     def make_service_dict(self):
-        self.service_dict = self.resolve_extends(self.service_dict)
+        self.service_dict = self.resolve_extends()
         return process_container_options(self.service_dict, working_dir=self.working_dir)
 
     def resolve_environment(self):
@@ -199,11 +208,12 @@ class ServiceLoader(object):
 
         self.service_dict['environment'] = env
 
-    def resolve_extends(self, service_dict):
-        if 'extends' not in service_dict:
-            return service_dict
+    def resolve_extends(self):
+        if 'extends' not in self.service_dict:
+            return self.service_dict
 
-        extends_options = self.validate_extends_options(service_dict['name'], service_dict['extends'])
+        extends_options = self.service_dict['extends']
+        service_name = self.service_dict['name']
 
         if 'file' in extends_options:
             extends_from_filename = extends_options['file']
@@ -212,7 +222,7 @@ class ServiceLoader(object):
             other_config_path = self.filename
 
         other_working_dir = os.path.dirname(other_config_path)
-        other_already_seen = self.already_seen + [self.signature(service_dict['name'])]
+        other_already_seen = self.already_seen + [self.signature(service_name)]
 
         base_service = extends_options['service']
         other_config = load_yaml(other_config_path)
@@ -227,7 +237,7 @@ class ServiceLoader(object):
         other_loader = ServiceLoader(
             working_dir=other_working_dir,
             filename=other_config_path,
-            service_name=service_dict['name'],
+            service_name=service_name,
             service_dict=other_service_dict,
             already_seen=other_already_seen,
         )
@@ -240,21 +250,11 @@ class ServiceLoader(object):
             service=extends_options['service'],
         )
 
-        return merge_service_dicts(other_service_dict, service_dict)
+        return merge_service_dicts(other_service_dict, self.service_dict)
 
     def signature(self, name):
         return (self.filename, name)
 
-    def validate_extends_options(self, service_name, extends_options):
-        error_prefix = "Invalid 'extends' configuration for %s:" % service_name
-
-        if 'file' not in extends_options and self.filename is None:
-            raise ConfigurationError(
-                "%s you need to specify a 'file', e.g. 'file: something.yml'" % error_prefix
-            )
-
-        return extends_options
-
 
 def validate_extended_service_dict(service_dict, filename, service):
     error_prefix = "Cannot extend service '%s' in %s:" % (service, filename)
diff --git a/compose/config/validation.py b/compose/config/validation.py
index d83504274..1ae8981ca 100644
--- a/compose/config/validation.py
+++ b/compose/config/validation.py
@@ -66,6 +66,19 @@ def validate_top_level_object(func):
     return func_wrapper
 
 
+def validate_extends_file_path(service_name, extends_options, filename):
+    """
+    The service to be extended must either be defined in the config key 'file',
+    or within 'filename'.
+    """
+    error_prefix = "Invalid 'extends' configuration for %s:" % service_name
+
+    if 'file' not in extends_options and filename is None:
+        raise ConfigurationError(
+            "%s you need to specify a 'file', e.g. 'file: something.yml'" % error_prefix
+        )
+
+
 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:

From 37bf8235b71a45b4b303b937129e07997784c61b Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Mon, 24 Aug 2015 16:00:47 +0100
Subject: [PATCH 06/16] Get extended config path

Refactored out into it's own function.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/config.py | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/compose/config/config.py b/compose/config/config.py
index 51bd93846..0f3099dc0 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -179,6 +179,9 @@ class ServiceLoader(object):
                 self.filename
             )
 
+            self.extended_config_path = self.get_extended_config_path(
+                self.service_dict['extends']
+            )
 
     def detect_cycle(self, name):
         if self.signature(name) in self.already_seen:
@@ -215,11 +218,7 @@ class ServiceLoader(object):
         extends_options = self.service_dict['extends']
         service_name = self.service_dict['name']
 
-        if 'file' in extends_options:
-            extends_from_filename = extends_options['file']
-            other_config_path = expand_path(self.working_dir, extends_from_filename)
-        else:
-            other_config_path = self.filename
+        other_config_path = self.get_extended_config_path(extends_options)
 
         other_working_dir = os.path.dirname(other_config_path)
         other_already_seen = self.already_seen + [self.signature(service_name)]
@@ -252,6 +251,18 @@ class ServiceLoader(object):
 
         return merge_service_dicts(other_service_dict, self.service_dict)
 
+    def get_extended_config_path(self, extends_options):
+        """
+        Service we are extending either has a value for 'file' set, which we
+        need to obtain a full path too or we are extending from a service
+        defined in our own file.
+        """
+        if 'file' in extends_options:
+            extends_from_filename = extends_options['file']
+            return expand_path(self.working_dir, extends_from_filename)
+
+        return self.filename
+
     def signature(self, name):
         return (self.filename, name)
 

From 36757cde1cbe38d9673f00af0f515038b8280cfe Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Tue, 25 Aug 2015 17:54:06 +0100
Subject: [PATCH 07/16] Validate extended service against our schema

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/config.py                      |  7 +--
 tests/fixtures/extends/invalid-links.yml      |  9 ++++
 tests/fixtures/extends/invalid-net.yml        |  8 ++++
 tests/fixtures/extends/invalid-volumes.yml    |  9 ++++
 .../extends/service-with-invalid-schema.yml   |  5 +++
 tests/unit/config_test.py                     | 45 ++++++++-----------
 6 files changed, 53 insertions(+), 30 deletions(-)
 create mode 100644 tests/fixtures/extends/invalid-links.yml
 create mode 100644 tests/fixtures/extends/invalid-net.yml
 create mode 100644 tests/fixtures/extends/invalid-volumes.yml
 create mode 100644 tests/fixtures/extends/service-with-invalid-schema.yml

diff --git a/compose/config/config.py b/compose/config/config.py
index 0f3099dc0..65a5b5472 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -182,6 +182,8 @@ class ServiceLoader(object):
             self.extended_config_path = self.get_extended_config_path(
                 self.service_dict['extends']
             )
+            extended_config = load_yaml(self.extended_config_path)
+            validate_against_schema(extended_config)
 
     def detect_cycle(self, name):
         if self.signature(name) in self.already_seen:
@@ -217,10 +219,9 @@ class ServiceLoader(object):
 
         extends_options = self.service_dict['extends']
         service_name = self.service_dict['name']
+        other_config_path = self.extended_config_path
 
-        other_config_path = self.get_extended_config_path(extends_options)
-
-        other_working_dir = os.path.dirname(other_config_path)
+        other_working_dir = os.path.dirname(self.extended_config_path)
         other_already_seen = self.already_seen + [self.signature(service_name)]
 
         base_service = extends_options['service']
diff --git a/tests/fixtures/extends/invalid-links.yml b/tests/fixtures/extends/invalid-links.yml
new file mode 100644
index 000000000..edfeb8b23
--- /dev/null
+++ b/tests/fixtures/extends/invalid-links.yml
@@ -0,0 +1,9 @@
+myweb:
+  build: '.'
+  extends:
+    service: web
+  command: top
+web:
+  build: '.'
+  links:
+    - "mydb:db"
diff --git a/tests/fixtures/extends/invalid-net.yml b/tests/fixtures/extends/invalid-net.yml
new file mode 100644
index 000000000..fbcd020bc
--- /dev/null
+++ b/tests/fixtures/extends/invalid-net.yml
@@ -0,0 +1,8 @@
+myweb:
+  build: '.'
+  extends:
+    service: web
+  command: top
+web:
+  build: '.'
+  net: "container:db"
diff --git a/tests/fixtures/extends/invalid-volumes.yml b/tests/fixtures/extends/invalid-volumes.yml
new file mode 100644
index 000000000..3db0118e0
--- /dev/null
+++ b/tests/fixtures/extends/invalid-volumes.yml
@@ -0,0 +1,9 @@
+myweb:
+  build: '.'
+  extends:
+    service: web
+  command: top
+web:
+  build: '.'
+  volumes_from:
+    - "db"
diff --git a/tests/fixtures/extends/service-with-invalid-schema.yml b/tests/fixtures/extends/service-with-invalid-schema.yml
new file mode 100644
index 000000000..90dc76a0e
--- /dev/null
+++ b/tests/fixtures/extends/service-with-invalid-schema.yml
@@ -0,0 +1,5 @@
+myweb:
+  extends:
+    service: web
+web:
+  command: top
diff --git a/tests/unit/config_test.py b/tests/unit/config_test.py
index f3a4bd306..98ae51385 100644
--- a/tests/unit/config_test.py
+++ b/tests/unit/config_test.py
@@ -867,6 +867,12 @@ class ExtendsTest(unittest.TestCase):
         self.assertEquals(len(service), 1)
         self.assertIsInstance(service[0], dict)
 
+    def test_extended_service_with_invalid_config(self):
+        expected_error_msg = "Service 'myweb' has neither an image nor a build path specified"
+
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            load_from_filename('tests/fixtures/extends/service-with-invalid-schema.yml')
+
     def test_extends_file_defaults_to_self(self):
         """
         Test not specifying a file in our extends options that the
@@ -891,37 +897,22 @@ class ExtendsTest(unittest.TestCase):
             }
         ]))
 
-    def test_blacklisted_options(self):
-        def load_config():
-            return make_service_dict('myweb', {
-                'extends': {
-                    'file': 'whatever',
-                    'service': 'web',
-                }
-            }, '.')
+    def test_invalid_links_in_extended_service(self):
+        expected_error_msg = "services with 'links' cannot be extended"
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            load_from_filename('tests/fixtures/extends/invalid-links.yml')
 
-        with self.assertRaisesRegexp(ConfigurationError, 'links'):
-            other_config = {'web': {'links': ['db']}}
+    def test_invalid_volumes_from_in_extended_service(self):
+        expected_error_msg = "services with 'volumes_from' cannot be extended"
 
-            with mock.patch.object(config, 'load_yaml', return_value=other_config):
-                print(load_config())
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            load_from_filename('tests/fixtures/extends/invalid-volumes.yml')
 
-        with self.assertRaisesRegexp(ConfigurationError, 'volumes_from'):
-            other_config = {'web': {'volumes_from': ['db']}}
+    def test_invalid_net_in_extended_service(self):
+        expected_error_msg = "services with 'net: container' cannot be extended"
 
-            with mock.patch.object(config, 'load_yaml', return_value=other_config):
-                print(load_config())
-
-        with self.assertRaisesRegexp(ConfigurationError, 'net'):
-            other_config = {'web': {'net': 'container:db'}}
-
-            with mock.patch.object(config, 'load_yaml', return_value=other_config):
-                print(load_config())
-
-        other_config = {'web': {'net': 'host'}}
-
-        with mock.patch.object(config, 'load_yaml', return_value=other_config):
-            print(load_config())
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            load_from_filename('tests/fixtures/extends/invalid-net.yml')
 
     def test_volume_path(self):
         dicts = load_from_filename('tests/fixtures/volume-path/docker-compose.yml')

From 4a8b2947caae7151db39a425e71f0f66dfd060ea Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Wed, 26 Aug 2015 13:23:29 +0100
Subject: [PATCH 08/16] Interpolate extended config

This refactoring is now really coming together. Construction is
happening in the __init__, which is a constructor and helps
clean up the design and clarity of intent of the code. We can now
see (nearly) everything that is being constructed when a ServiceLoader
is created. It needs all of these data constructs to perform the
domain logic and actions. Which are now clearer to see and moving
more towards the principle of functions doing (mostly)one thing and
function names being more descriptive.

resolve_extends is now concerned with the resolving of extends, rather
than the construction, validation, pre processing and *then* resolving
of extends.

Happy days :)

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/config.py                      | 44 ++++++++++---------
 compose/config/validation.py                  |  8 ++++
 .../extends/valid-interpolation-2.yml         |  3 ++
 .../fixtures/extends/valid-interpolation.yml  |  5 +++
 tests/unit/config_test.py                     | 11 +++++
 5 files changed, 50 insertions(+), 21 deletions(-)
 create mode 100644 tests/fixtures/extends/valid-interpolation-2.yml
 create mode 100644 tests/fixtures/extends/valid-interpolation.yml

diff --git a/compose/config/config.py b/compose/config/config.py
index 65a5b5472..e3ba2aeb8 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -11,6 +11,7 @@ from .errors import ComposeFileNotFound
 from .errors import ConfigurationError
 from .interpolation import interpolate_environment_variables
 from .validation import validate_against_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
@@ -178,12 +179,25 @@ class ServiceLoader(object):
                 self.service_dict['extends'],
                 self.filename
             )
-
             self.extended_config_path = self.get_extended_config_path(
                 self.service_dict['extends']
             )
-            extended_config = load_yaml(self.extended_config_path)
-            validate_against_schema(extended_config)
+            self.extended_service_name = self.service_dict['extends']['service']
+
+            full_extended_config = pre_process_config(
+                load_yaml(self.extended_config_path)
+            )
+
+            validate_extended_service_exists(
+                self.extended_service_name,
+                full_extended_config,
+                self.extended_config_path
+            )
+            validate_against_schema(full_extended_config)
+
+            self.extended_config = full_extended_config[self.extended_service_name]
+        else:
+            self.extended_config = None
 
     def detect_cycle(self, name):
         if self.signature(name) in self.already_seen:
@@ -214,40 +228,28 @@ class ServiceLoader(object):
         self.service_dict['environment'] = env
 
     def resolve_extends(self):
-        if 'extends' not in self.service_dict:
+        if self.extended_config is None:
             return self.service_dict
 
-        extends_options = self.service_dict['extends']
         service_name = self.service_dict['name']
-        other_config_path = self.extended_config_path
 
         other_working_dir = os.path.dirname(self.extended_config_path)
         other_already_seen = self.already_seen + [self.signature(service_name)]
 
-        base_service = extends_options['service']
-        other_config = load_yaml(other_config_path)
-
-        if base_service not in other_config:
-            msg = (
-                "Cannot extend service '%s' in %s: Service not found"
-            ) % (base_service, other_config_path)
-            raise ConfigurationError(msg)
-
-        other_service_dict = other_config[base_service]
         other_loader = ServiceLoader(
             working_dir=other_working_dir,
-            filename=other_config_path,
+            filename=self.extended_config_path,
             service_name=service_name,
-            service_dict=other_service_dict,
+            service_dict=self.extended_config,
             already_seen=other_already_seen,
         )
 
-        other_loader.detect_cycle(extends_options['service'])
+        other_loader.detect_cycle(self.extended_service_name)
         other_service_dict = other_loader.make_service_dict()
         validate_extended_service_dict(
             other_service_dict,
-            filename=other_config_path,
-            service=extends_options['service'],
+            filename=self.extended_config_path,
+            service=self.extended_service_name,
         )
 
         return merge_service_dicts(other_service_dict, self.service_dict)
diff --git a/compose/config/validation.py b/compose/config/validation.py
index 1ae8981ca..304e7e760 100644
--- a/compose/config/validation.py
+++ b/compose/config/validation.py
@@ -79,6 +79,14 @@ 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:
diff --git a/tests/fixtures/extends/valid-interpolation-2.yml b/tests/fixtures/extends/valid-interpolation-2.yml
new file mode 100644
index 000000000..cb7bd93fc
--- /dev/null
+++ b/tests/fixtures/extends/valid-interpolation-2.yml
@@ -0,0 +1,3 @@
+web:
+  build: '.'
+  hostname: "host-${HOSTNAME_VALUE}"
diff --git a/tests/fixtures/extends/valid-interpolation.yml b/tests/fixtures/extends/valid-interpolation.yml
new file mode 100644
index 000000000..68e8740fb
--- /dev/null
+++ b/tests/fixtures/extends/valid-interpolation.yml
@@ -0,0 +1,5 @@
+myweb:
+  extends:
+    service: web
+    file: valid-interpolation-2.yml
+  command: top
diff --git a/tests/unit/config_test.py b/tests/unit/config_test.py
index 98ae51385..7624bbdfd 100644
--- a/tests/unit/config_test.py
+++ b/tests/unit/config_test.py
@@ -914,6 +914,17 @@ class ExtendsTest(unittest.TestCase):
         with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
             load_from_filename('tests/fixtures/extends/invalid-net.yml')
 
+    @mock.patch.dict(os.environ)
+    def test_valid_interpolation_in_extended_service(self):
+        os.environ.update(
+            HOSTNAME_VALUE="penguin",
+        )
+        expected_interpolated_value = "host-penguin"
+
+        service_dicts = load_from_filename('tests/fixtures/extends/valid-interpolation.yml')
+        for service in service_dicts:
+            self.assertTrue(service['hostname'], expected_interpolated_value)
+
     def test_volume_path(self):
         dicts = load_from_filename('tests/fixtures/volume-path/docker-compose.yml')
 

From 950577d60f7d9ff76c1087f0f93de97303975d71 Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Fri, 28 Aug 2015 18:49:17 +0100
Subject: [PATCH 09/16] Split validation into fields and service

We want to give feedback to the user as soon as possible about the
validity of the config supplied for the services.

When extending a service, we can validate that the fields are
correct against our schema but we must wait until the *end* of
the extends cycle once all of the extended dicts have been merged
into the service dict, to perform the final validation check on the
config to ensure it is a complete valid service.

Doing this before that had happened resulted in false reports of
invalid config, as common config when split out, by itself, is not
a valid service but *is* valid config to be included.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/config.py                      | 11 ++++--
 .../{schema.json => fields_schema.json}       | 18 ---------
 compose/config/service_schema.json            | 39 +++++++++++++++++++
 compose/config/validation.py                  | 18 +++++++--
 .../extends/service-with-invalid-schema.yml   |  3 +-
 .../service-with-valid-composite-extends.yml  |  5 +++
 .../fixtures/extends/valid-common-config.yml  |  6 +++
 tests/fixtures/extends/valid-common.yml       |  3 ++
 .../extends/valid-composite-extends.yml       |  2 +
 tests/unit/config_test.py                     |  9 +++++
 10 files changed, 88 insertions(+), 26 deletions(-)
 rename compose/config/{schema.json => fields_schema.json} (90%)
 create mode 100644 compose/config/service_schema.json
 create mode 100644 tests/fixtures/extends/service-with-valid-composite-extends.yml
 create mode 100644 tests/fixtures/extends/valid-common-config.yml
 create mode 100644 tests/fixtures/extends/valid-common.yml
 create mode 100644 tests/fixtures/extends/valid-composite-extends.yml

diff --git a/compose/config/config.py b/compose/config/config.py
index e3ba2aeb8..736f5aeb3 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -10,7 +10,8 @@ from .errors import CircularReference
 from .errors import ComposeFileNotFound
 from .errors import ConfigurationError
 from .interpolation import interpolate_environment_variables
-from .validation import validate_against_schema
+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
@@ -139,7 +140,7 @@ def load(config_details):
     config, working_dir, filename = config_details
 
     processed_config = pre_process_config(config)
-    validate_against_schema(processed_config)
+    validate_against_fields_schema(processed_config)
 
     service_dicts = []
 
@@ -193,7 +194,7 @@ class ServiceLoader(object):
                 full_extended_config,
                 self.extended_config_path
             )
-            validate_against_schema(full_extended_config)
+            validate_against_fields_schema(full_extended_config)
 
             self.extended_config = full_extended_config[self.extended_service_name]
         else:
@@ -205,6 +206,10 @@ class ServiceLoader(object):
 
     def make_service_dict(self):
         self.service_dict = self.resolve_extends()
+
+        if not self.already_seen:
+            validate_against_service_schema(self.service_dict)
+
         return process_container_options(self.service_dict, working_dir=self.working_dir)
 
     def resolve_environment(self):
diff --git a/compose/config/schema.json b/compose/config/fields_schema.json
similarity index 90%
rename from compose/config/schema.json
rename to compose/config/fields_schema.json
index 94fe4fc52..92305c575 100644
--- a/compose/config/schema.json
+++ b/compose/config/fields_schema.json
@@ -106,24 +106,6 @@
         "working_dir": {"type": "string"}
       },
 
-      "anyOf": [
-        {
-          "required": ["build"],
-          "not": {"required": ["image"]}
-        },
-        {
-          "required": ["image"],
-          "not": {"anyOf": [
-            {"required": ["build"]},
-            {"required": ["dockerfile"]}
-          ]}
-        },
-        {
-          "required": ["extends"],
-          "not": {"required": ["build", "image"]}
-        }
-      ],
-
       "dependencies": {
         "memswap_limit": ["mem_limit"]
       },
diff --git a/compose/config/service_schema.json b/compose/config/service_schema.json
new file mode 100644
index 000000000..5cb5d6d07
--- /dev/null
+++ b/compose/config/service_schema.json
@@ -0,0 +1,39 @@
+{
+  "$schema": "http://json-schema.org/draft-04/schema#",
+
+  "type": "object",
+
+  "properties": {
+      "name": {"type": "string"}
+  },
+
+  "required": ["name"],
+
+  "allOf": [
+    {"$ref": "fields_schema.json#/definitions/service"},
+    {"$ref": "#/definitions/service_constraints"}
+  ],
+
+  "definitions": {
+    "service_constraints": {
+      "anyOf": [
+        {
+          "required": ["build"],
+          "not": {"required": ["image"]}
+        },
+        {
+          "required": ["image"],
+          "not": {"anyOf": [
+            {"required": ["build"]},
+            {"required": ["dockerfile"]}
+          ]}
+        },
+        {
+          "required": ["extends"],
+          "not": {"required": ["build", "image"]}
+        }
+      ]
+    }
+  }
+
+}
diff --git a/compose/config/validation.py b/compose/config/validation.py
index 304e7e760..3ae5485a7 100644
--- a/compose/config/validation.py
+++ b/compose/config/validation.py
@@ -5,6 +5,7 @@ from functools import wraps
 from docker.utils.ports import split_port
 from jsonschema import Draft4Validator
 from jsonschema import FormatChecker
+from jsonschema import RefResolver
 from jsonschema import ValidationError
 
 from .errors import ConfigurationError
@@ -210,14 +211,25 @@ def process_errors(errors):
     return "\n".join(root_msgs + invalid_keys + required + type_errors + other_errors)
 
 
-def validate_against_schema(config):
+def validate_against_fields_schema(config):
+    schema_filename = "fields_schema.json"
+    return _validate_against_schema(config, schema_filename)
+
+
+def validate_against_service_schema(config):
+    schema_filename = "service_schema.json"
+    return _validate_against_schema(config, schema_filename)
+
+
+def _validate_against_schema(config, schema_filename):
     config_source_dir = os.path.dirname(os.path.abspath(__file__))
-    schema_file = os.path.join(config_source_dir, "schema.json")
+    schema_file = os.path.join(config_source_dir, schema_filename)
 
     with open(schema_file, "r") as schema_fh:
         schema = json.load(schema_fh)
 
-    validation_output = Draft4Validator(schema, format_checker=FormatChecker(["ports"]))
+    resolver = RefResolver('file://' + config_source_dir + '/', schema)
+    validation_output = Draft4Validator(schema, resolver=resolver, format_checker=FormatChecker(["ports"]))
 
     errors = [error for error in sorted(validation_output.iter_errors(config), key=str)]
     if errors:
diff --git a/tests/fixtures/extends/service-with-invalid-schema.yml b/tests/fixtures/extends/service-with-invalid-schema.yml
index 90dc76a0e..00c36647e 100644
--- a/tests/fixtures/extends/service-with-invalid-schema.yml
+++ b/tests/fixtures/extends/service-with-invalid-schema.yml
@@ -1,5 +1,4 @@
 myweb:
   extends:
+    file: valid-composite-extends.yml
     service: web
-web:
-  command: top
diff --git a/tests/fixtures/extends/service-with-valid-composite-extends.yml b/tests/fixtures/extends/service-with-valid-composite-extends.yml
new file mode 100644
index 000000000..6c419ed07
--- /dev/null
+++ b/tests/fixtures/extends/service-with-valid-composite-extends.yml
@@ -0,0 +1,5 @@
+myweb:
+  build: '.'
+  extends:
+    file: 'valid-composite-extends.yml'
+    service: web
diff --git a/tests/fixtures/extends/valid-common-config.yml b/tests/fixtures/extends/valid-common-config.yml
new file mode 100644
index 000000000..d8f13e7a8
--- /dev/null
+++ b/tests/fixtures/extends/valid-common-config.yml
@@ -0,0 +1,6 @@
+myweb:
+  build: '.'
+  extends:
+    file: valid-common.yml
+    service: common-config
+  command: top
diff --git a/tests/fixtures/extends/valid-common.yml b/tests/fixtures/extends/valid-common.yml
new file mode 100644
index 000000000..07ad68e3e
--- /dev/null
+++ b/tests/fixtures/extends/valid-common.yml
@@ -0,0 +1,3 @@
+common-config:
+  environment:
+    - FOO=1
diff --git a/tests/fixtures/extends/valid-composite-extends.yml b/tests/fixtures/extends/valid-composite-extends.yml
new file mode 100644
index 000000000..8816c3f3b
--- /dev/null
+++ b/tests/fixtures/extends/valid-composite-extends.yml
@@ -0,0 +1,2 @@
+web:
+  command: top
diff --git a/tests/unit/config_test.py b/tests/unit/config_test.py
index 7624bbdfd..8f4251cfa 100644
--- a/tests/unit/config_test.py
+++ b/tests/unit/config_test.py
@@ -866,6 +866,7 @@ class ExtendsTest(unittest.TestCase):
 
         self.assertEquals(len(service), 1)
         self.assertIsInstance(service[0], dict)
+        self.assertEquals(service[0]['command'], "/bin/true")
 
     def test_extended_service_with_invalid_config(self):
         expected_error_msg = "Service 'myweb' has neither an image nor a build path specified"
@@ -873,6 +874,10 @@ class ExtendsTest(unittest.TestCase):
         with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
             load_from_filename('tests/fixtures/extends/service-with-invalid-schema.yml')
 
+    def test_extended_service_with_valid_config(self):
+        service = load_from_filename('tests/fixtures/extends/service-with-valid-composite-extends.yml')
+        self.assertEquals(service[0]['command'], "top")
+
     def test_extends_file_defaults_to_self(self):
         """
         Test not specifying a file in our extends options that the
@@ -955,6 +960,10 @@ class ExtendsTest(unittest.TestCase):
         with self.assertRaisesRegexp(ConfigurationError, err_msg):
             load_from_filename('tests/fixtures/extends/nonexistent-service.yml')
 
+    def test_partial_service_config_in_extends_is_still_valid(self):
+        dicts = load_from_filename('tests/fixtures/extends/valid-common-config.yml')
+        self.assertEqual(dicts[0]['environment'], {'FOO': '1'})
+
 
 class BuildPathTest(unittest.TestCase):
     def setUp(self):

From 9fa6e42f5562be98a4541941f40327f248179b43 Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Mon, 31 Aug 2015 17:52:00 +0100
Subject: [PATCH 10/16] process_errors handle both schemas

Now the schema has been split into two, we need to modify the
process_errors function to accomodate.

Previously if an error.path was empty then it meant they were root
errors. Now that service_schema checks after the service has been
resolved, our service name is a key within the dictionary and
so our root error logic check is no longer true.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/validation.py | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/compose/config/validation.py b/compose/config/validation.py
index 3ae5485a7..59fb13948 100644
--- a/compose/config/validation.py
+++ b/compose/config/validation.py
@@ -125,7 +125,7 @@ def process_errors(errors):
 
     for error in errors:
         # handle root level errors
-        if len(error.path) == 0:
+        if len(error.path) == 0 and not error.instance.get('name'):
             if error.validator == 'type':
                 msg = "Top level object needs to be a dictionary. Check your .yml file that you have defined a service at the top level."
                 root_msgs.append(msg)
@@ -137,11 +137,13 @@ def process_errors(errors):
                 root_msgs.append(_clean_error_message(error.message))
 
         else:
-            # handle service level errors
-            service_name = error.path[0]
-
-            # pop the service name off our path
-            error.path.popleft()
+            try:
+                # field_schema errors will have service name on the path
+                service_name = error.path[0]
+                error.path.popleft()
+            except IndexError:
+                # service_schema errors will have the name in the instance
+                service_name = error.instance.get('name')
 
             if error.validator == 'additionalProperties':
                 invalid_config_key = _parse_key_from_error_msg(error)

From 4b487e3957bf6aad71358fb4fdc2c7bf952b1927 Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Tue, 1 Sep 2015 16:14:02 +0100
Subject: [PATCH 11/16] Refactor extends back out of __init__

If make_service_dict is our factory function then we'll give it the
responsibility of validation/construction and resolving.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/config.py | 67 +++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 36 deletions(-)

diff --git a/compose/config/config.py b/compose/config/config.py
index 736f5aeb3..70eac267b 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -170,42 +170,18 @@ class ServiceLoader(object):
             self.filename = filename
         self.already_seen = already_seen or []
         self.service_dict = service_dict.copy()
+        self.service_name = service_name
         self.service_dict['name'] = service_name
 
-        self.resolve_environment()
-
-        if 'extends' in self.service_dict:
-            validate_extends_file_path(
-                service_name,
-                self.service_dict['extends'],
-                self.filename
-            )
-            self.extended_config_path = self.get_extended_config_path(
-                self.service_dict['extends']
-            )
-            self.extended_service_name = self.service_dict['extends']['service']
-
-            full_extended_config = pre_process_config(
-                load_yaml(self.extended_config_path)
-            )
-
-            validate_extended_service_exists(
-                self.extended_service_name,
-                full_extended_config,
-                self.extended_config_path
-            )
-            validate_against_fields_schema(full_extended_config)
-
-            self.extended_config = full_extended_config[self.extended_service_name]
-        else:
-            self.extended_config = None
-
     def detect_cycle(self, name):
         if self.signature(name) in self.already_seen:
             raise CircularReference(self.already_seen + [self.signature(name)])
 
     def make_service_dict(self):
-        self.service_dict = self.resolve_extends()
+        self.resolve_environment()
+        if 'extends' in self.service_dict:
+            self.validate_and_construct_extends()
+            self.service_dict = self.resolve_extends()
 
         if not self.already_seen:
             validate_against_service_schema(self.service_dict)
@@ -232,19 +208,38 @@ class ServiceLoader(object):
 
         self.service_dict['environment'] = env
 
+    def validate_and_construct_extends(self):
+        validate_extends_file_path(
+            self.service_name,
+            self.service_dict['extends'],
+            self.filename
+        )
+        self.extended_config_path = self.get_extended_config_path(
+            self.service_dict['extends']
+        )
+        self.extended_service_name = self.service_dict['extends']['service']
+
+        full_extended_config = pre_process_config(
+            load_yaml(self.extended_config_path)
+        )
+
+        validate_extended_service_exists(
+            self.extended_service_name,
+            full_extended_config,
+            self.extended_config_path
+        )
+        validate_against_fields_schema(full_extended_config)
+
+        self.extended_config = full_extended_config[self.extended_service_name]
+
     def resolve_extends(self):
-        if self.extended_config is None:
-            return self.service_dict
-
-        service_name = self.service_dict['name']
-
         other_working_dir = os.path.dirname(self.extended_config_path)
-        other_already_seen = self.already_seen + [self.signature(service_name)]
+        other_already_seen = self.already_seen + [self.signature(self.service_name)]
 
         other_loader = ServiceLoader(
             working_dir=other_working_dir,
             filename=self.extended_config_path,
-            service_name=service_name,
+            service_name=self.service_name,
             service_dict=self.extended_config,
             already_seen=other_already_seen,
         )

From 9979880c9fc5371f2e0a26fa4d43bfdad156263f Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Tue, 1 Sep 2015 17:00:52 +0100
Subject: [PATCH 12/16] Add in volume_driver

I'd missed out this field by accident previously.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/fields_schema.json | 1 +
 1 file changed, 1 insertion(+)

diff --git a/compose/config/fields_schema.json b/compose/config/fields_schema.json
index 92305c575..299f6de4f 100644
--- a/compose/config/fields_schema.json
+++ b/compose/config/fields_schema.json
@@ -102,6 +102,7 @@
         "tty": {"type": "string"},
         "user": {"type": "string"},
         "volumes": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "volume_driver": {"type": "string"},
         "volumes_from": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
         "working_dir": {"type": "string"}
       },

From f51a5431ec0c3318af5c39599805f20cd135d5f9 Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Wed, 2 Sep 2015 14:58:45 +0100
Subject: [PATCH 13/16] Correct some schema field definitions

Now validation is split in two, the integration tests helped
highlight some places where the schema definition was incorrect.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/fields_schema.json | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/compose/config/fields_schema.json b/compose/config/fields_schema.json
index 299f6de4f..2a122b7a3 100644
--- a/compose/config/fields_schema.json
+++ b/compose/config/fields_schema.json
@@ -19,7 +19,12 @@
         "cap_drop": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
         "command": {"$ref": "#/definitions/string_or_list"},
         "container_name": {"type": "string"},
-        "cpu_shares": {"type": "string"},
+        "cpu_shares": {
+          "oneOf": [
+            {"type": "number"},
+            {"type": "string"}
+          ]
+        },
         "cpuset": {"type": "string"},
         "detach": {"type": "boolean"},
         "devices": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
@@ -27,7 +32,7 @@
         "dns_search": {"$ref": "#/definitions/string_or_list"},
         "dockerfile": {"type": "string"},
         "domainname": {"type": "string"},
-        "entrypoint": {"type": "string"},
+        "entrypoint": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
         "env_file": {"$ref": "#/definitions/string_or_list"},
 
         "environment": {
@@ -75,7 +80,7 @@
         },
         "name": {"type": "string"},
         "net": {"type": "string"},
-        "pid": {"type": "string"},
+        "pid": {"type": ["string", "null"]},
 
         "ports": {
           "type": "array",
@@ -94,10 +99,10 @@
           "uniqueItems": true
         },
 
-        "privileged": {"type": "string"},
+        "privileged": {"type": "boolean"},
         "read_only": {"type": "boolean"},
         "restart": {"type": "string"},
-        "security_opt": {"type": "string"},
+        "security_opt": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
         "stdin_open": {"type": "string"},
         "tty": {"type": "string"},
         "user": {"type": "string"},

From 9b8e404d138a1999594231b12ba29c935b93eb69 Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Wed, 2 Sep 2015 15:00:28 +0100
Subject: [PATCH 14/16] Pass service_name to process_errors

Previously on Buffy... The process_errors was parsing a load of
ValidationErrors that we get back from jsonschema which included
assumptions about the state of the instance we're validating.

Now it's split in two and we're doing field separate to service,
those assumptions don't hold and we can't logically retrieve the
service_name from the error parsing when we're doing service schema
validation, have to explicitly pass this in.

process_errors is high on my list for some future re-factoring to help
make it a bit clearer, smaller state of doing things.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 compose/config/config.py     |  2 +-
 compose/config/validation.py | 19 ++++++++++---------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/compose/config/config.py b/compose/config/config.py
index 70eac267b..8df45b8a9 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -184,7 +184,7 @@ class ServiceLoader(object):
             self.service_dict = self.resolve_extends()
 
         if not self.already_seen:
-            validate_against_service_schema(self.service_dict)
+            validate_against_service_schema(self.service_dict, self.service_name)
 
         return process_container_options(self.service_dict, working_dir=self.working_dir)
 
diff --git a/compose/config/validation.py b/compose/config/validation.py
index 59fb13948..632bdf03b 100644
--- a/compose/config/validation.py
+++ b/compose/config/validation.py
@@ -95,7 +95,7 @@ def get_unsupported_config_msg(service_name, error_key):
     return msg
 
 
-def process_errors(errors):
+def process_errors(errors, service_name=None):
     """
     jsonschema gives us an error tree full of information to explain what has
     gone wrong. Process each error and pull out relevant information and re-write
@@ -137,13 +137,14 @@ def process_errors(errors):
                 root_msgs.append(_clean_error_message(error.message))
 
         else:
-            try:
+            if not service_name:
                 # field_schema errors will have service name on the path
                 service_name = error.path[0]
                 error.path.popleft()
-            except IndexError:
-                # service_schema errors will have the name in the instance
-                service_name = error.instance.get('name')
+            else:
+                # service_schema errors have the service name passed in, as that
+                # is not available on error.path or necessarily error.instance
+                service_name = service_name
 
             if error.validator == 'additionalProperties':
                 invalid_config_key = _parse_key_from_error_msg(error)
@@ -218,12 +219,12 @@ def validate_against_fields_schema(config):
     return _validate_against_schema(config, schema_filename)
 
 
-def validate_against_service_schema(config):
+def validate_against_service_schema(config, service_name):
     schema_filename = "service_schema.json"
-    return _validate_against_schema(config, schema_filename)
+    return _validate_against_schema(config, schema_filename, service_name)
 
 
-def _validate_against_schema(config, schema_filename):
+def _validate_against_schema(config, schema_filename, service_name=None):
     config_source_dir = os.path.dirname(os.path.abspath(__file__))
     schema_file = os.path.join(config_source_dir, schema_filename)
 
@@ -235,5 +236,5 @@ def _validate_against_schema(config, schema_filename):
 
     errors = [error for error in sorted(validation_output.iter_errors(config), key=str)]
     if errors:
-        error_msg = process_errors(errors)
+        error_msg = process_errors(errors, service_name)
         raise ConfigurationError("Validation failed, reason(s):\n{}".format(error_msg))

From d31d24d19fc7faa54bd793e812ca3a4447afaa27 Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Wed, 2 Sep 2015 15:03:29 +0100
Subject: [PATCH 15/16] Work around some coupling of links, net & volume_from

This is minimal disruptive change I could make to ensure
the service integration tests worked, now we have some validation
happening.

There is some coupling/entanglement/assumption going on here.

Project when creating a service, using it's class method from_dicts
performs some transformations on links, net & volume_from, which
get passed on to Service when creating. Service itself, then performs
some transformation on those values. This worked fine in the tests before
because those options were merely passed on via make_service_dict.

This is no longer true with our validation in place. You can't pass to
ServiceLoader [(obj, 'string')] for links, the validation expects it to be
a list of strings. Which it would be when passed into Project.from_dicts
method.

I think the tests need some re-factoring but for now, manually deleting
keys out of the kwargs and then putting them back in for Service Creation
allows the tests to continue.

I am not super happy about this approach. Hopefully we can come back and
improve it.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 tests/integration/testcases.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py
index 58240d5ea..4557c07b6 100644
--- a/tests/integration/testcases.py
+++ b/tests/integration/testcases.py
@@ -31,11 +31,29 @@ class DockerClientTestCase(unittest.TestCase):
         if 'command' not in kwargs:
             kwargs['command'] = ["top"]
 
+        links = kwargs.get('links', None)
+        volumes_from = kwargs.get('volumes_from', None)
+        net = kwargs.get('net', None)
+
+        workaround_options = ['links', 'volumes_from', 'net']
+        for key in workaround_options:
+            try:
+                del kwargs[key]
+            except KeyError:
+                pass
+
         options = ServiceLoader(working_dir='.', filename=None, service_name=name, service_dict=kwargs).make_service_dict()
 
         labels = options.setdefault('labels', {})
         labels['com.docker.compose.test-name'] = self.id()
 
+        if links:
+            options['links'] = links
+        if volumes_from:
+            options['volumes_from'] = volumes_from
+        if net:
+            options['net'] = net
+
         return Service(
             project='composetest',
             client=self.client,

From 6a399a5b2f1ed0e014fcb21bae80cae3b725e506 Mon Sep 17 00:00:00 2001
From: Mazz Mosley <mazz@houseofmnowster.com>
Date: Wed, 2 Sep 2015 15:41:25 +0100
Subject: [PATCH 16/16] Update tests to be compatible with validation

Some were missing build '.' from their dicts, others were the
incorrect type and one I've moved from integration to unit.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>
---
 tests/integration/service_test.py | 12 +-----
 tests/unit/config_test.py         | 67 ++++++++++++++++++++++++-------
 2 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py
index 0cf8cdb0e..177471ffa 100644
--- a/tests/integration/service_test.py
+++ b/tests/integration/service_test.py
@@ -165,16 +165,6 @@ class ServiceTest(DockerClientTestCase):
         service.start_container(container)
         self.assertEqual(set(container.get('HostConfig.ExtraHosts')), set(extra_hosts))
 
-    def test_create_container_with_extra_hosts_string(self):
-        extra_hosts = 'somehost:162.242.195.82'
-        service = self.create_service('db', extra_hosts=extra_hosts)
-        self.assertRaises(ConfigError, lambda: service.create_container())
-
-    def test_create_container_with_extra_hosts_list_of_dicts(self):
-        extra_hosts = [{'somehost': '162.242.195.82'}, {'otherhost': '50.31.209.229'}]
-        service = self.create_service('db', extra_hosts=extra_hosts)
-        self.assertRaises(ConfigError, lambda: service.create_container())
-
     def test_create_container_with_extra_hosts_dicts(self):
         extra_hosts = {'somehost': '162.242.195.82', 'otherhost': '50.31.209.229'}
         extra_hosts_list = ['somehost:162.242.195.82', 'otherhost:50.31.209.229']
@@ -515,7 +505,7 @@ class ServiceTest(DockerClientTestCase):
         self.assertEqual(container['HostConfig']['Privileged'], True)
 
     def test_expose_does_not_publish_ports(self):
-        service = self.create_service('web', expose=[8000])
+        service = self.create_service('web', expose=["8000"])
         container = create_and_start_container(service).inspect()
         self.assertEqual(container['NetworkSettings']['Ports'], {'8000/tcp': None})
 
diff --git a/tests/unit/config_test.py b/tests/unit/config_test.py
index 8f4251cfa..21f1261ec 100644
--- a/tests/unit/config_test.py
+++ b/tests/unit/config_test.py
@@ -206,6 +206,39 @@ class ConfigTest(unittest.TestCase):
                 )
             )
 
+    def test_config_extra_hosts_string_raises_validation_error(self):
+        expected_error_msg = "Service 'web' configuration key 'extra_hosts' contains an invalid type"
+
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            config.load(
+                config.ConfigDetails(
+                    {'web': {
+                        'image': 'busybox',
+                        'extra_hosts': 'somehost:162.242.195.82'
+                    }},
+                    'working_dir',
+                    'filename.yml'
+                )
+            )
+
+    def test_config_extra_hosts_list_of_dicts_validation_error(self):
+        expected_error_msg = "Service 'web' configuration key 'extra_hosts' contains an invalid type"
+
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            config.load(
+                config.ConfigDetails(
+                    {'web': {
+                        'image': 'busybox',
+                        'extra_hosts': [
+                            {'somehost': '162.242.195.82'},
+                            {'otherhost': '50.31.209.229'}
+                        ]
+                    }},
+                    'working_dir',
+                    'filename.yml'
+                )
+            )
+
 
 class InterpolationTest(unittest.TestCase):
     @mock.patch.dict(os.environ)
@@ -240,7 +273,7 @@ class InterpolationTest(unittest.TestCase):
                 'web': {
                     'image': '${FOO}',
                     'command': '${BAR}',
-                    'entrypoint': '${BAR}',
+                    'container_name': '${BAR}',
                 },
             },
             working_dir='.',
@@ -286,12 +319,13 @@ class InterpolationTest(unittest.TestCase):
     @mock.patch.dict(os.environ)
     def test_volume_binding_with_home(self):
         os.environ['HOME'] = '/home/user'
-        d = make_service_dict('foo', {'volumes': ['~:/container/path']}, working_dir='.')
+        d = make_service_dict('foo', {'build': '.', 'volumes': ['~:/container/path']}, working_dir='.')
         self.assertEqual(d['volumes'], ['/home/user:/container/path'])
 
     @mock.patch.dict(os.environ)
     def test_volume_binding_with_local_dir_name_raises_warning(self):
         def make_dict(**config):
+            config['build'] = '.'
             make_service_dict('foo', config, working_dir='.')
 
         with mock.patch('compose.config.config.log.warn') as warn:
@@ -336,6 +370,7 @@ class InterpolationTest(unittest.TestCase):
 
     def test_named_volume_with_driver_does_not_expand(self):
         d = make_service_dict('foo', {
+            'build': '.',
             'volumes': ['namedvolume:/data'],
             'volume_driver': 'foodriver',
         }, working_dir='.')
@@ -345,6 +380,7 @@ class InterpolationTest(unittest.TestCase):
     def test_home_directory_with_driver_does_not_expand(self):
         os.environ['NAME'] = 'surprise!'
         d = make_service_dict('foo', {
+            'build': '.',
             'volumes': ['~:/data'],
             'volume_driver': 'foodriver',
         }, working_dir='.')
@@ -504,36 +540,36 @@ class MergeLabelsTest(unittest.TestCase):
 
     def test_no_override(self):
         service_dict = config.merge_service_dicts(
-            make_service_dict('foo', {'labels': ['foo=1', 'bar']}, 'tests/'),
-            make_service_dict('foo', {}, 'tests/'),
+            make_service_dict('foo', {'build': '.', 'labels': ['foo=1', 'bar']}, 'tests/'),
+            make_service_dict('foo', {'build': '.'}, 'tests/'),
         )
         self.assertEqual(service_dict['labels'], {'foo': '1', 'bar': ''})
 
     def test_no_base(self):
         service_dict = config.merge_service_dicts(
-            make_service_dict('foo', {}, 'tests/'),
-            make_service_dict('foo', {'labels': ['foo=2']}, 'tests/'),
+            make_service_dict('foo', {'build': '.'}, 'tests/'),
+            make_service_dict('foo', {'build': '.', 'labels': ['foo=2']}, 'tests/'),
         )
         self.assertEqual(service_dict['labels'], {'foo': '2'})
 
     def test_override_explicit_value(self):
         service_dict = config.merge_service_dicts(
-            make_service_dict('foo', {'labels': ['foo=1', 'bar']}, 'tests/'),
-            make_service_dict('foo', {'labels': ['foo=2']}, 'tests/'),
+            make_service_dict('foo', {'build': '.', 'labels': ['foo=1', 'bar']}, 'tests/'),
+            make_service_dict('foo', {'build': '.', 'labels': ['foo=2']}, 'tests/'),
         )
         self.assertEqual(service_dict['labels'], {'foo': '2', 'bar': ''})
 
     def test_add_explicit_value(self):
         service_dict = config.merge_service_dicts(
-            make_service_dict('foo', {'labels': ['foo=1', 'bar']}, 'tests/'),
-            make_service_dict('foo', {'labels': ['bar=2']}, 'tests/'),
+            make_service_dict('foo', {'build': '.', 'labels': ['foo=1', 'bar']}, 'tests/'),
+            make_service_dict('foo', {'build': '.', 'labels': ['bar=2']}, 'tests/'),
         )
         self.assertEqual(service_dict['labels'], {'foo': '1', 'bar': '2'})
 
     def test_remove_explicit_value(self):
         service_dict = config.merge_service_dicts(
-            make_service_dict('foo', {'labels': ['foo=1', 'bar=2']}, 'tests/'),
-            make_service_dict('foo', {'labels': ['bar']}, 'tests/'),
+            make_service_dict('foo', {'build': '.', 'labels': ['foo=1', 'bar=2']}, 'tests/'),
+            make_service_dict('foo', {'build': '.', 'labels': ['bar']}, 'tests/'),
         )
         self.assertEqual(service_dict['labels'], {'foo': '1', 'bar': ''})
 
@@ -615,6 +651,7 @@ class EnvTest(unittest.TestCase):
 
         service_dict = make_service_dict(
             'foo', {
+                'build': '.',
                 'environment': {
                     'FILE_DEF': 'F1',
                     'FILE_DEF_EMPTY': '',
@@ -633,7 +670,7 @@ class EnvTest(unittest.TestCase):
     def test_env_from_file(self):
         service_dict = make_service_dict(
             'foo',
-            {'env_file': 'one.env'},
+            {'build': '.', 'env_file': 'one.env'},
             'tests/fixtures/env',
         )
         self.assertEqual(
@@ -644,7 +681,7 @@ class EnvTest(unittest.TestCase):
     def test_env_from_multiple_files(self):
         service_dict = make_service_dict(
             'foo',
-            {'env_file': ['one.env', 'two.env']},
+            {'build': '.', 'env_file': ['one.env', 'two.env']},
             'tests/fixtures/env',
         )
         self.assertEqual(
@@ -666,7 +703,7 @@ class EnvTest(unittest.TestCase):
         os.environ['ENV_DEF'] = 'E3'
         service_dict = make_service_dict(
             'foo',
-            {'env_file': 'resolve.env'},
+            {'build': '.', 'env_file': 'resolve.env'},
             'tests/fixtures/env',
         )
         self.assertEqual(