Perform schema validation

Define a schema that we can pass to jsonschema to validate against the
config a user has supplied. This will help catch a wide variety of common
errors that occur.

If the config does not pass schema validation then it raises an exception
and prints out human readable reasons.

Signed-off-by: Mazz Mosley <mazz@houseofmnowster.com>

compose/config/config.py

@@ -3,6 +3,8 @@ import os
 import sys
 import yaml
 from collections import namedtuple
+import json
+import jsonschema
 
 import six
 
@@ -131,13 +133,31 @@ def get_config_path(base_dir):
     return os.path.join(path, winner)
 
 
+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 = jsonschema.Draft4Validator(schema)
+
+    errors = [error.message for error in sorted(validation_output.iter_errors(config), key=str)]
+    if errors:
+        raise ConfigurationError("Validation failed, reason(s): {}".format("\n".join(errors)))
+
+
 def load(config_details):
-    dictionary, working_dir, filename = config_details
+    config, working_dir, filename = config_details
-    dictionary = interpolate_environment_variables(dictionary)
+    config = interpolate_environment_variables(config)
 
     service_dicts = []
 
-    for service_name, service_dict in list(dictionary.items()):
+    validate_against_schema(config)
+
+    for service_name, service_dict in list(config.items()):
+        if not isinstance(service_dict, dict):
+            raise ConfigurationError('Service "%s" doesn\'t have any configuration options. All top level keys in your docker-compose.yml must map to a dictionary of configuration options.' % service_name)
         loader = ServiceLoader(working_dir=working_dir, filename=filename)
         service_dict = loader.make_service_dict(service_name, service_dict)
         validate_paths(service_dict)
@@ -210,25 +230,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
 
 
@@ -256,9 +262,6 @@ def process_container_options(service_dict, working_dir=None):
 
     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['volumes'], working_dir=working_dir)
 

compose/schema.json

@@ -0,0 +1,79 @@
+{
+  "$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"},
+        "env_file": {"$ref": "#/definitions/string_or_list"},
+        "environment": {
+          "oneOf": [
+            {"type": "object"},
+            {"type": "array", "items": {"type": "string"}, "uniqueItems": true}
+          ]
+        },
+        "image": {"type": "string"},
+        "mem_limit": {"type": "number"},
+        "memswap_limit": {"type": "number"},
+
+        "extends": {
+          "type": "object",
+
+          "properties": {
+            "service": {"type": "string"},
+            "file": {"type": "string"}
+          },
+          "required": ["service"],
+          "additionalProperties": false
+        }
+
+      },
+
+      "anyOf": [
+        {
+          "required": ["build"],
+          "not": {"required": ["image"]}
+        },
+        {
+          "required": ["image"],
+          "not": {"required": ["build"]}
+        },
+        {
+          "required": ["extends"],
+          "not": {"required": ["build", "image"]}
+        }
+      ],
+
+      "dependencies": {
+        "memswap_limit": ["mem_limit"]
+      }
+
+    },
+
+    "string_or_list": {
+      "oneOf": [
+        {"type": "string"},
+        {"$ref": "#/definitions/list_of_strings"}
+      ]
+    },
+
+    "list_of_strings": {
+      "type": "array",
+      "items": {"type": "string"},
+      "uniqueItems": true
+    }
+
+  },
+
+  "additionalProperties": false
+}

compose/service.py

@@ -82,14 +82,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

@@ -20,10 +20,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,13 +32,14 @@ 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):
@@ -327,23 +328,26 @@ 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):
+        with self.assertRaisesRegexp(config.ConfigurationError, "u'mem_limit' is a dependency of u'memswap_limit'"):
-            make_service_dict(
+            config.load(
-                'foo', {
+                config.ConfigDetails(
-                    'memswap_limit': 2000000,
+                    {
-                },
+                        'foo': {'image': 'busybox', 'memswap_limit': 2000000},
-                'tests/'
+                    },
+                    '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):
@@ -528,6 +532,7 @@ class ExtendsTest(unittest.TestCase):
             {
                 'environment':
                 {'YEP': '1'},
+                'image': 'busybox',
                 'name': 'otherweb'
             },
             {
@@ -553,36 +558,47 @@ class ExtendsTest(unittest.TestCase):
             )
 
     def test_extends_validation_empty_dictionary(self):
-        dictionary = {'extends': None}
+        with self.assertRaisesRegexp(config.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(config.ConfigurationError, "u'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 = {
+        with self.assertRaisesRegexp(config.ConfigurationError, "'rogue_key' was unexpected"):
-            'extends':
+            config.load(
-            {
+                config.ConfigDetails(
-                'service': 'web', 'file': 'common.yml', 'what': 'is this'
+                    {
-            }
+                        'web': {
-        }
+                            'image': 'busybox',
-
+                            'extends': {
-        def load_config():
+                                'file': 'common.yml',
-            return make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends')
+                                'service': 'web',
-
+                                'rogue_key': 'is not allowed'
-        self.assertRaisesRegexp(config.ConfigurationError, 'what', load_config)
+                            }
+                        },
+                    },
+                    'tests/fixtures/extends',
+                    'filename.yml'
+                )
+            )
 
     def test_extends_validation_no_file_key_no_filename_set(self):
         dictionary = {'extends': {'service': 'web'}}
@@ -593,12 +609,18 @@ class ExtendsTest(unittest.TestCase):
         self.assertRaisesRegexp(config.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):
         """

tests/unit/service_test.py

@@ -49,7 +49,6 @@ class ServiceTest(unittest.TestCase):
         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')