Merge pull request #1808 from mnowster/129-validate-compose-yml

129 validate compose yml

MANIFEST.in

@@ -4,6 +4,7 @@ include requirements.txt
 include requirements-dev.txt
 include tox.ini
 include *.md
+include compose/config/schema.json
 recursive-include contrib/completion *
 recursive-include tests *
 global-exclude *.pyc

compose/config/config.py

@@ -3,7 +3,6 @@ import os
 import sys
 import yaml
 from collections import namedtuple
-
 import six
 
 from compose.cli.utils import find_candidates_in_parent_dirs
@@ -14,6 +13,7 @@ from .errors import (
     CircularReference,
     ComposeFileNotFound,
 )
+from .validation import validate_against_schema
 
 
 DOCKER_CONFIG_KEYS = [
@@ -65,22 +65,6 @@ ALLOWED_KEYS = DOCKER_CONFIG_KEYS + [
     'name',
 ]
 
-DOCKER_CONFIG_HINTS = {
-    'cpu_share': 'cpu_shares',
-    'add_host': 'extra_hosts',
-    'hosts': 'extra_hosts',
-    'extra_host': 'extra_hosts',
-    'device': 'devices',
-    'link': 'links',
-    'memory_swap': 'memswap_limit',
-    'port': 'ports',
-    'privilege': 'privileged',
-    'priviliged': 'privileged',
-    'privilige': 'privileged',
-    'volume': 'volumes',
-    'workdir': 'working_dir',
-}
-
 
 SUPPORTED_FILENAMES = [
     'docker-compose.yml',
@@ -139,12 +123,18 @@ def get_config_path(base_dir):
 
 
 def load(config_details):
-    dictionary, working_dir, filename = config_details
+    config, working_dir, filename = config_details
-    dictionary = interpolate_environment_variables(dictionary)
+    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."
+        )
+
+    config = interpolate_environment_variables(config)
+    validate_against_schema(config)
 
     service_dicts = []
 
-    for service_name, service_dict in list(dictionary.items()):
+    for service_name, service_dict in list(config.items()):
         loader = ServiceLoader(working_dir=working_dir, filename=filename)
         service_dict = loader.make_service_dict(service_name, service_dict)
         validate_paths(service_dict)
@@ -217,25 +207,11 @@ class ServiceLoader(object):
     def validate_extends_options(self, service_name, extends_options):
         error_prefix = "Invalid 'extends' configuration for %s:" % service_name
 
-        if not isinstance(extends_options, dict):
-            raise ConfigurationError("%s must be a dictionary" % error_prefix)
-
-        if 'service' not in extends_options:
-            raise ConfigurationError(
-                "%s you need to specify a service, e.g. 'service: web'" % error_prefix
-            )
-
         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
             )
 
-        for k, _ in extends_options.items():
-            if k not in ['file', 'service']:
-                raise ConfigurationError(
-                    "%s unsupported configuration option '%s'" % (error_prefix, k)
-                )
-
         return extends_options
 
 
@@ -254,18 +230,8 @@ def validate_extended_service_dict(service_dict, filename, service):
 
 
 def process_container_options(service_dict, working_dir=None):
-    for k in service_dict:
-        if k not in ALLOWED_KEYS:
-            msg = "Unsupported config option for %s service: '%s'" % (service_dict['name'], k)
-            if k in DOCKER_CONFIG_HINTS:
-                msg += " (did you mean '%s'?)" % DOCKER_CONFIG_HINTS[k]
-            raise ConfigurationError(msg)
-
     service_dict = service_dict.copy()
 
-    if 'memswap_limit' in service_dict and 'mem_limit' not in service_dict:
-        raise ConfigurationError("Invalid 'memswap_limit' configuration for %s service: when defining 'memswap_limit' you must set 'mem_limit' as well" % service_dict['name'])
-
     if 'volumes' in service_dict and service_dict.get('volume_driver') is None:
         service_dict['volumes'] = resolve_volume_paths(service_dict, working_dir=working_dir)
 
@@ -335,18 +301,6 @@ def merge_environment(base, override):
     return env
 
 
-def parse_links(links):
-    return dict(parse_link(l) for l in links)
-
-
-def parse_link(link):
-    if ':' in link:
-        source, alias = link.split(':', 1)
-        return (alias, source)
-    else:
-        return (link, link)
-
-
 def get_env_files(options, working_dir=None):
     if 'env_file' not in options:
         return {}
@@ -521,11 +475,6 @@ def parse_labels(labels):
     if isinstance(labels, dict):
         return labels
 
-    raise ConfigurationError(
-        "labels \"%s\" must be a list or mapping" %
-        labels
-    )
-
 
 def split_label(label):
     if '=' in label:

compose/config/schema.json

@@ -0,0 +1,151 @@
+{
+  "$schema": "http://json-schema.org/draft-04/schema#",
+
+  "type": "object",
+
+  "patternProperties": {
+    "^[a-zA-Z0-9._-]+$": {
+      "$ref": "#/definitions/service"
+    }
+  },
+
+  "definitions": {
+    "service": {
+      "type": "object",
+
+      "properties": {
+        "build": {"type": "string"},
+        "cap_add": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "cap_drop": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "command": {"$ref": "#/definitions/string_or_list"},
+        "container_name": {"type": "string"},
+        "cpu_shares": {"type": "string"},
+        "cpuset": {"type": "string"},
+        "detach": {"type": "boolean"},
+        "devices": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "dns": {"$ref": "#/definitions/string_or_list"},
+        "dns_search": {"$ref": "#/definitions/string_or_list"},
+        "dockerfile": {"type": "string"},
+        "domainname": {"type": "string"},
+        "entrypoint": {"type": "string"},
+        "env_file": {"$ref": "#/definitions/string_or_list"},
+
+        "environment": {
+          "oneOf": [
+            {"type": "object"},
+            {"type": "array", "items": {"type": "string"}, "uniqueItems": true}
+          ]
+        },
+
+        "expose": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+
+        "extends": {
+          "type": "object",
+
+          "properties": {
+            "service": {"type": "string"},
+            "file": {"type": "string"}
+          },
+          "required": ["service"],
+          "additionalProperties": false
+        },
+
+        "extra_hosts": {"$ref": "#/definitions/list_or_dict"},
+        "external_links": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "hostname": {"type": "string"},
+        "image": {"type": "string"},
+        "labels": {"$ref": "#/definitions/list_or_dict"},
+        "links": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "log_driver": {"type": "string"},
+
+        "log_opt": {
+          "type": "object",
+
+          "properties": {
+            "address": {"type": "string"}
+          },
+          "required": ["address"]
+        },
+
+        "mac_address": {"type": "string"},
+        "mem_limit": {"type": "number"},
+        "memswap_limit": {"type": "number"},
+        "name": {"type": "string"},
+        "net": {"type": "string"},
+        "pid": {"type": "string"},
+
+        "ports": {
+          "oneOf": [
+            {
+              "type": "array",
+              "items": {"type": "string"},
+              "uniqueItems": true,
+              "format": "ports"
+            },
+            {
+              "type": "string",
+              "format": "ports"
+            },
+            {
+              "type": "number",
+              "format": "ports"
+            }
+          ]
+        },
+
+        "privileged": {"type": "string"},
+        "read_only": {"type": "boolean"},
+        "restart": {"type": "string"},
+        "security_opt": {"type": "string"},
+        "stdin_open": {"type": "string"},
+        "tty": {"type": "string"},
+        "user": {"type": "string"},
+        "volumes": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "volumes_from": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "working_dir": {"type": "string"}
+      },
+
+      "anyOf": [
+        {
+          "required": ["build"],
+          "not": {"required": ["image"]}
+        },
+        {
+          "required": ["image"],
+          "not": {"required": ["build"]}
+        },
+        {
+          "required": ["extends"],
+          "not": {"required": ["build", "image"]}
+        }
+      ],
+
+      "dependencies": {
+        "memswap_limit": ["mem_limit"]
+      },
+      "additionalProperties": false
+    },
+
+    "string_or_list": {
+      "oneOf": [
+        {"type": "string"},
+        {"$ref": "#/definitions/list_of_strings"}
+      ]
+    },
+
+    "list_of_strings": {
+      "type": "array",
+      "items": {"type": "string"},
+      "uniqueItems": true
+    },
+
+    "list_or_dict": {
+      "oneOf": [
+        {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        {"type": "object"}
+      ]
+    }
+
+  },
+  "additionalProperties": false
+}

compose/config/validation.py

@@ -0,0 +1,151 @@
+import os
+
+from docker.utils.ports import split_port
+import json
+from jsonschema import Draft4Validator, FormatChecker, ValidationError
+
+from .errors import ConfigurationError
+
+
+DOCKER_CONFIG_HINTS = {
+    'cpu_share': 'cpu_shares',
+    'add_host': 'extra_hosts',
+    'hosts': 'extra_hosts',
+    'extra_host': 'extra_hosts',
+    'device': 'devices',
+    'link': 'links',
+    'memory_swap': 'memswap_limit',
+    'port': 'ports',
+    'privilege': 'privileged',
+    'priviliged': 'privileged',
+    'privilige': 'privileged',
+    'volume': 'volumes',
+    'workdir': 'working_dir',
+}
+
+
+VALID_NAME_CHARS = '[a-zA-Z0-9\._\-]'
+
+
+@FormatChecker.cls_checks(format="ports", raises=ValidationError("Invalid port formatting, it should be '[[remote_ip:]remote_port:]port[/protocol]'"))
+def format_ports(instance):
+    try:
+        split_port(instance)
+    except ValueError:
+        return False
+    return True
+
+
+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:
+        msg += " (did you mean '{}'?)".format(DOCKER_CONFIG_HINTS[error_key])
+    return msg
+
+
+def process_errors(errors):
+    """
+    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
+    helpful error messages that are relevant.
+    """
+    def _parse_key_from_error_msg(error):
+        return error.message.split("'")[1]
+
+    def _clean_error_message(message):
+        return message.replace("u'", "'")
+
+    def _parse_valid_types_from_schema(schema):
+        """
+        Our defined types using $ref in the schema require some extra parsing
+        retrieve a helpful type for error message display.
+        """
+        if '$ref' in schema:
+            return schema['$ref'].replace("#/definitions/", "").replace("_", " ")
+        else:
+            return str(schema['type'])
+
+    root_msgs = []
+    invalid_keys = []
+    required = []
+    type_errors = []
+    other_errors = []
+
+    for error in errors:
+        # handle root level errors
+        if len(error.path) == 0:
+            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)
+            elif error.validator == 'additionalProperties':
+                invalid_service_name = _parse_key_from_error_msg(error)
+                msg = "Invalid service name '{}' - only {} characters are allowed".format(invalid_service_name, VALID_NAME_CHARS)
+                root_msgs.append(msg)
+            else:
+                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()
+
+            if error.validator == 'additionalProperties':
+                invalid_config_key = _parse_key_from_error_msg(error)
+                invalid_keys.append(get_unsupported_config_msg(service_name, invalid_config_key))
+            elif error.validator == 'anyOf':
+                if 'image' in error.instance and 'build' in error.instance:
+                    required.append("Service '{}' has both an image and build path specified. A service can either be built to image or use an existing image, not both.".format(service_name))
+                elif 'image' not in error.instance and 'build' not in error.instance:
+                    required.append("Service '{}' has neither an image nor a build path specified. Exactly one must be provided.".format(service_name))
+                else:
+                    required.append(_clean_error_message(error.message))
+            elif error.validator == 'oneOf':
+                config_key = error.path[0]
+
+                valid_types = [_parse_valid_types_from_schema(schema) for schema in error.schema['oneOf']]
+                valid_type_msg = " or ".join(valid_types)
+
+                type_errors.append("Service '{}' configuration key '{}' contains an invalid type, valid types are {}".format(
+                    service_name, config_key, valid_type_msg)
+                )
+            elif error.validator == 'type':
+                msg = "a"
+                if error.validator_value == "array":
+                    msg = "an"
+
+                if len(error.path) > 0:
+                    config_key = " ".join(["'%s'" % k for k in error.path])
+                    type_errors.append("Service '{}' configuration key {} contains an invalid type, it should be {} {}".format(service_name, config_key, msg, error.validator_value))
+                else:
+                    root_msgs.append("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':
+                config_key = error.path[0]
+                required.append("Service '{}' option '{}' is invalid, {}".format(service_name, config_key, _clean_error_message(error.message)))
+            elif error.validator == 'dependencies':
+                dependency_key = error.validator_value.keys()[0]
+                required_keys = ",".join(error.validator_value[dependency_key])
+                required.append("Invalid '{}' configuration for '{}' service: when defining '{}' you must set '{}' as well".format(
+                    dependency_key, service_name, dependency_key, required_keys))
+            else:
+                config_key = " ".join(["'%s'" % k for k in error.path])
+                err_msg = "Service '{}' configuration key {} value {}".format(service_name, config_key, error.message)
+                other_errors.append(err_msg)
+
+    return "\n".join(root_msgs + invalid_keys + required + type_errors + other_errors)
+
+
+def validate_against_schema(config):
+    config_source_dir = os.path.dirname(os.path.abspath(__file__))
+    schema_file = os.path.join(config_source_dir, "schema.json")
+
+    with open(schema_file, "r") as schema_fh:
+        schema = json.load(schema_fh)
+
+    validation_output = Draft4Validator(schema, format_checker=FormatChecker(["ports"]))
+
+    errors = [error for error in sorted(validation_output.iter_errors(config), key=str)]
+    if errors:
+        error_msg = process_errors(errors)
+        raise ConfigurationError("Validation failed, reason(s):\n{}".format(error_msg))

compose/service.py

@@ -26,6 +26,7 @@ from .container import Container
 from .legacy import check_for_legacy_containers
 from .progress_stream import stream_output, StreamOutputError
 from .utils import json_hash, parallel_execute
+from .config.validation import VALID_NAME_CHARS
 
 log = logging.getLogger(__name__)
 
@@ -51,8 +52,6 @@ DOCKER_START_KEYS = [
     'security_opt',
 ]
 
-VALID_NAME_CHARS = '[a-zA-Z0-9\._\-]'
-
 
 class BuildError(Exception):
     def __init__(self, service, reason):
@@ -84,14 +83,8 @@ ConvergencePlan = namedtuple('ConvergencePlan', 'action containers')
 
 class Service(object):
     def __init__(self, name, client=None, project='default', links=None, external_links=None, volumes_from=None, net=None, **options):
-        if not re.match('^%s+$' % VALID_NAME_CHARS, name):
-            raise ConfigError('Invalid service name "%s" - only %s are allowed' % (name, VALID_NAME_CHARS))
         if not re.match('^%s+$' % VALID_NAME_CHARS, project):
             raise ConfigError('Invalid project name "%s" - only %s are allowed' % (project, VALID_NAME_CHARS))
-        if 'image' in options and 'build' in options:
-            raise ConfigError('Service %s has both an image and build path specified. A service can either be built to image or use an existing image, not both.' % name)
-        if 'image' not in options and 'build' not in options:
-            raise ConfigError('Service %s has neither an image nor a build path specified. Exactly one must be provided.' % name)
 
         self.name = name
         self.client = client

requirements.txt

@@ -1,4 +1,5 @@
 PyYAML==3.10
+jsonschema==2.5.1
 docker-py==1.3.1
 dockerpty==0.3.4
 docopt==0.6.1

setup.py

@@ -33,6 +33,7 @@ install_requires = [
     'docker-py >= 1.3.1, < 1.4',
     'dockerpty >= 0.3.4, < 0.4',
     'six >= 1.3.0, < 2',
+    'jsonschema >= 2.5.1, < 3',
 ]
 
 

tests/fixtures/extends/specify-file-as-self.yml

@@ -12,5 +12,6 @@ web:
   environment:
     - "BAZ=3"
 otherweb:
+  image: busybox
   environment:
     - "YEP=1"

tests/unit/config_test.py

@@ -5,6 +5,7 @@ import tempfile
 from .. import unittest
 
 from compose.config import config
+from compose.config.errors import ConfigurationError
 
 
 def make_service_dict(name, service_dict, working_dir):
@@ -20,10 +21,10 @@ class ConfigTest(unittest.TestCase):
             config.ConfigDetails(
                 {
                     'foo': {'image': 'busybox'},
-                    'bar': {'environment': ['FOO=1']},
+                    'bar': {'image': 'busybox', 'environment': ['FOO=1']},
                 },
-                'working_dir',
+                'tests/fixtures/extends',
-                'filename.yml'
+                'common.yml'
             )
         )
 
@@ -32,17 +33,18 @@ class ConfigTest(unittest.TestCase):
             sorted([
                 {
                     'name': 'bar',
+                    'image': 'busybox',
                     'environment': {'FOO': '1'},
                 },
                 {
                     'name': 'foo',
                     'image': 'busybox',
                 }
-            ])
+            ], key=lambda d: d['name'])
         )
 
     def test_load_throws_error_when_not_dict(self):
-        with self.assertRaises(config.ConfigurationError):
+        with self.assertRaises(ConfigurationError):
             config.load(
                 config.ConfigDetails(
                     {'web': 'busybox:latest'},
@@ -51,12 +53,125 @@ class ConfigTest(unittest.TestCase):
                 )
             )
 
-    def test_config_validation(self):
+    def test_config_invalid_service_names(self):
-        self.assertRaises(
+        with self.assertRaises(ConfigurationError):
-            config.ConfigurationError,
+            for invalid_name in ['?not?allowed', ' ', '', '!', '/', '\xe2']:
-            lambda: make_service_dict('foo', {'port': ['8000']}, 'tests/')
+                config.load(
-        )
+                    config.ConfigDetails(
-        make_service_dict('foo', {'ports': ['8000']}, 'tests/')
+                        {invalid_name: {'image': 'busybox'}},
+                        'working_dir',
+                        'filename.yml'
+                    )
+                )
+
+    def test_config_valid_service_names(self):
+        for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']:
+            config.load(
+                config.ConfigDetails(
+                    {valid_name: {'image': 'busybox'}},
+                    'tests/fixtures/extends',
+                    'common.yml'
+                )
+            )
+
+    def test_config_invalid_ports_format_validation(self):
+        expected_error_msg = "Service 'web' configuration key 'ports' contains an invalid type"
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            for invalid_ports in [{"1": "8000"}, False, 0]:
+                config.load(
+                    config.ConfigDetails(
+                        {'web': {'image': 'busybox', 'ports': invalid_ports}},
+                        'working_dir',
+                        'filename.yml'
+                    )
+                )
+
+    def test_config_valid_ports_format_validation(self):
+        valid_ports = [["8000", "9000"], ["8000/8050"], ["8000"], "8000", 8000]
+        for ports in valid_ports:
+            config.load(
+                config.ConfigDetails(
+                    {'web': {'image': 'busybox', 'ports': ports}},
+                    'working_dir',
+                    'filename.yml'
+                )
+            )
+
+    def test_config_hint(self):
+        expected_error_msg = "(did you mean 'privileged'?)"
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            config.load(
+                config.ConfigDetails(
+                    {
+                        'foo': {'image': 'busybox', 'privilige': 'something'},
+                    },
+                    'tests/fixtures/extends',
+                    'filename.yml'
+                )
+            )
+
+    def test_invalid_config_build_and_image_specified(self):
+        expected_error_msg = "Service 'foo' has both an image and build path specified."
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            config.load(
+                config.ConfigDetails(
+                    {
+                        'foo': {'image': 'busybox', 'build': '.'},
+                    },
+                    'tests/fixtures/extends',
+                    'filename.yml'
+                )
+            )
+
+    def test_invalid_config_type_should_be_an_array(self):
+        expected_error_msg = "Service 'foo' configuration key 'links' contains an invalid type, it should be an array"
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            config.load(
+                config.ConfigDetails(
+                    {
+                        'foo': {'image': 'busybox', 'links': 'an_link'},
+                    },
+                    'tests/fixtures/extends',
+                    'filename.yml'
+                )
+            )
+
+    def test_invalid_config_not_a_dictionary(self):
+        expected_error_msg = "Top level object needs to be a dictionary."
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            config.load(
+                config.ConfigDetails(
+                    ['foo', 'lol'],
+                    'tests/fixtures/extends',
+                    'filename.yml'
+                )
+            )
+
+    def test_invalid_config_not_unique_items(self):
+        expected_error_msg = "has non-unique elements"
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            config.load(
+                config.ConfigDetails(
+                    {
+                        'web': {'build': '.', 'devices': ['/dev/foo:/dev/foo', '/dev/foo:/dev/foo']}
+                    },
+                    'tests/fixtures/extends',
+                    'filename.yml'
+                )
+            )
+
+    def test_invalid_list_of_strings_format(self):
+        expected_error_msg = "'command' contains an invalid type, valid types are string or list of strings"
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
+            config.load(
+                config.ConfigDetails(
+                    {
+                        'web': {'build': '.', 'command': [1]}
+                    },
+                    'tests/fixtures/extends',
+                    'filename.yml'
+                )
+            )
 
 
 class InterpolationTest(unittest.TestCase):
@@ -104,7 +219,7 @@ class InterpolationTest(unittest.TestCase):
         os.environ['VOLUME_PATH'] = '/host/path'
         d = config.load(
             config.ConfigDetails(
-                config={'foo': {'volumes': ['${VOLUME_PATH}:/container/path']}},
+                config={'foo': {'build': '.', 'volumes': ['${VOLUME_PATH}:/container/path']}},
                 working_dir='.',
                 filename=None,
             )
@@ -372,23 +487,27 @@ class MemoryOptionsTest(unittest.TestCase):
         When you set a 'memswap_limit' it is invalid config unless you also set
         a mem_limit
         """
-        with self.assertRaises(config.ConfigurationError):
+        expected_error_msg = "Invalid 'memswap_limit' configuration for 'foo' service: when defining 'memswap_limit' you must set 'mem_limit' as well"
-            make_service_dict(
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
-                'foo', {
+            config.load(
-                    'memswap_limit': 2000000,
+                config.ConfigDetails(
-                },
+                    {
-                'tests/'
+                        'foo': {'image': 'busybox', 'memswap_limit': 2000000},
+                    },
+                    'tests/fixtures/extends',
+                    'filename.yml'
+                )
             )
 
     def test_validation_with_correct_memswap_values(self):
-        service_dict = make_service_dict(
+        service_dict = config.load(
-            'foo', {
+            config.ConfigDetails(
-                'mem_limit': 1000000,
+                {'foo': {'image': 'busybox', 'mem_limit': 1000000, 'memswap_limit': 2000000}},
-                'memswap_limit': 2000000,
+                'tests/fixtures/extends',
-            },
+                'common.yml'
-            'tests/'
+            )
         )
-        self.assertEqual(service_dict['memswap_limit'], 2000000)
+        self.assertEqual(service_dict[0]['memswap_limit'], 2000000)
 
 
 class EnvTest(unittest.TestCase):
@@ -412,7 +531,7 @@ class EnvTest(unittest.TestCase):
         self.assertEqual(config.parse_environment(environment), environment)
 
     def test_parse_environment_invalid(self):
-        with self.assertRaises(config.ConfigurationError):
+        with self.assertRaises(ConfigurationError):
             config.parse_environment('a=b')
 
     def test_parse_environment_empty(self):
@@ -466,7 +585,7 @@ class EnvTest(unittest.TestCase):
     def test_env_nonexistent_file(self):
         options = {'env_file': 'nonexistent.env'}
         self.assertRaises(
-            config.ConfigurationError,
+            ConfigurationError,
             lambda: make_service_dict('foo', options, 'tests/fixtures/env'),
         )
 
@@ -492,7 +611,7 @@ class EnvTest(unittest.TestCase):
 
         service_dict = config.load(
             config.ConfigDetails(
-                config={'foo': {'volumes': ['$HOSTENV:$CONTAINERENV']}},
+                config={'foo': {'build': '.', 'volumes': ['$HOSTENV:$CONTAINERENV']}},
                 working_dir="tests/fixtures/env",
                 filename=None,
             )
@@ -501,7 +620,7 @@ class EnvTest(unittest.TestCase):
 
         service_dict = config.load(
             config.ConfigDetails(
-                config={'foo': {'volumes': ['/opt${HOSTENV}:/opt${CONTAINERENV}']}},
+                config={'foo': {'build': '.', 'volumes': ['/opt${HOSTENV}:/opt${CONTAINERENV}']}},
                 working_dir="tests/fixtures/env",
                 filename=None,
             )
@@ -573,6 +692,7 @@ class ExtendsTest(unittest.TestCase):
             {
                 'environment':
                 {'YEP': '1'},
+                'image': 'busybox',
                 'name': 'otherweb'
             },
             {
@@ -598,36 +718,67 @@ class ExtendsTest(unittest.TestCase):
             )
 
     def test_extends_validation_empty_dictionary(self):
-        dictionary = {'extends': None}
+        with self.assertRaisesRegexp(ConfigurationError, 'service'):
-
+            config.load(
-        def load_config():
+                config.ConfigDetails(
-            return make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends')
+                    {
-
+                        'web': {'image': 'busybox', 'extends': {}},
-        self.assertRaisesRegexp(config.ConfigurationError, 'dictionary', load_config)
+                    },
-
+                    'tests/fixtures/extends',
-        dictionary['extends'] = {}
+                    'filename.yml'
-        self.assertRaises(config.ConfigurationError, load_config)
+                )
+            )
 
     def test_extends_validation_missing_service_key(self):
-        dictionary = {'extends': {'file': 'common.yml'}}
+        with self.assertRaisesRegexp(ConfigurationError, "'service' is a required property"):
-
+            config.load(
-        def load_config():
+                config.ConfigDetails(
-            return make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends')
+                    {
-
+                        'web': {'image': 'busybox', 'extends': {'file': 'common.yml'}},
-        self.assertRaisesRegexp(config.ConfigurationError, 'service', load_config)
+                    },
+                    'tests/fixtures/extends',
+                    'filename.yml'
+                )
+            )
 
     def test_extends_validation_invalid_key(self):
-        dictionary = {
+        expected_error_msg = "Unsupported config option for 'web' service: 'rogue_key'"
-            'extends':
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
-            {
+            config.load(
-                'service': 'web', 'file': 'common.yml', 'what': 'is this'
+                config.ConfigDetails(
-            }
+                    {
-        }
+                        'web': {
+                            'image': 'busybox',
+                            'extends': {
+                                'file': 'common.yml',
+                                'service': 'web',
+                                'rogue_key': 'is not allowed'
+                            }
+                        },
+                    },
+                    'tests/fixtures/extends',
+                    'filename.yml'
+                )
+            )
 
-        def load_config():
+    def test_extends_validation_sub_property_key(self):
-            return make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends')
+        expected_error_msg = "Service 'web' configuration key 'extends' 'file' contains an invalid type"
-
+        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
-        self.assertRaisesRegexp(config.ConfigurationError, 'what', load_config)
+            config.load(
+                config.ConfigDetails(
+                    {
+                        'web': {
+                            'image': 'busybox',
+                            'extends': {
+                                'file': 1,
+                                'service': 'web',
+                            }
+                        },
+                    },
+                    'tests/fixtures/extends',
+                    'filename.yml'
+                )
+            )
 
     def test_extends_validation_no_file_key_no_filename_set(self):
         dictionary = {'extends': {'service': 'web'}}
@@ -635,15 +786,21 @@ class ExtendsTest(unittest.TestCase):
         def load_config():
             return make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends')
 
-        self.assertRaisesRegexp(config.ConfigurationError, 'file', load_config)
+        self.assertRaisesRegexp(ConfigurationError, 'file', load_config)
 
     def test_extends_validation_valid_config(self):
-        dictionary = {'extends': {'service': 'web', 'file': 'common.yml'}}
+        service = config.load(
+            config.ConfigDetails(
+                {
+                    'web': {'image': 'busybox', 'extends': {'service': 'web', 'file': 'common.yml'}},
+                },
+                'tests/fixtures/extends',
+                'common.yml'
+            )
+        )
 
-        def load_config():
+        self.assertEquals(len(service), 1)
-            return make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends')
+        self.assertIsInstance(service[0], dict)
-
-        self.assertIsInstance(load_config(), dict)
 
     def test_extends_file_defaults_to_self(self):
         """
@@ -678,19 +835,19 @@ class ExtendsTest(unittest.TestCase):
                 }
             }, '.')
 
-        with self.assertRaisesRegexp(config.ConfigurationError, 'links'):
+        with self.assertRaisesRegexp(ConfigurationError, 'links'):
             other_config = {'web': {'links': ['db']}}
 
             with mock.patch.object(config, 'load_yaml', return_value=other_config):
                 print load_config()
 
-        with self.assertRaisesRegexp(config.ConfigurationError, 'volumes_from'):
+        with self.assertRaisesRegexp(ConfigurationError, 'volumes_from'):
             other_config = {'web': {'volumes_from': ['db']}}
 
             with mock.patch.object(config, 'load_yaml', return_value=other_config):
                 print load_config()
 
-        with self.assertRaisesRegexp(config.ConfigurationError, 'net'):
+        with self.assertRaisesRegexp(ConfigurationError, 'net'):
             other_config = {'web': {'net': 'container:db'}}
 
             with mock.patch.object(config, 'load_yaml', return_value=other_config):
@@ -732,7 +889,7 @@ class BuildPathTest(unittest.TestCase):
         self.abs_context_path = os.path.join(os.getcwd(), 'tests/fixtures/build-ctx')
 
     def test_nonexistent_path(self):
-        with self.assertRaises(config.ConfigurationError):
+        with self.assertRaises(ConfigurationError):
             config.load(
                 config.ConfigDetails(
                     {

tests/unit/service_test.py

@@ -29,27 +29,7 @@ class ServiceTest(unittest.TestCase):
     def setUp(self):
         self.mock_client = mock.create_autospec(docker.Client)
 
-    def test_name_validations(self):
-        self.assertRaises(ConfigError, lambda: Service(name='', image='foo'))
-
-        self.assertRaises(ConfigError, lambda: Service(name=' ', image='foo'))
-        self.assertRaises(ConfigError, lambda: Service(name='/', image='foo'))
-        self.assertRaises(ConfigError, lambda: Service(name='!', image='foo'))
-        self.assertRaises(ConfigError, lambda: Service(name='\xe2', image='foo'))
-
-        Service('a', image='foo')
-        Service('foo', image='foo')
-        Service('foo-bar', image='foo')
-        Service('foo.bar', image='foo')
-        Service('foo_bar', image='foo')
-        Service('_', image='foo')
-        Service('___', image='foo')
-        Service('-', image='foo')
-        Service('--', image='foo')
-        Service('.__.', image='foo')
-
     def test_project_validation(self):
-        self.assertRaises(ConfigError, lambda: Service('bar'))
         self.assertRaises(ConfigError, lambda: Service(name='foo', project='>', image='foo'))
 
         Service(name='foo', project='bar.bar__', image='foo')