Merge pull request #79 from orchardup/strict-config

Throw an error if you specify an unrecognised option in `fig.yml`

fig/cli/command.py

@@ -7,8 +7,10 @@ import logging
 import os
 import re
 import yaml
+import six
 
 from ..project import Project
+from ..service import ConfigError
 from .docopt_command import DocoptCommand
 from .formatter import Formatter
 from .utils import cached_property, docker_url, call_silently, is_mac, is_ubuntu
@@ -69,7 +71,10 @@ If it's at a non-standard location, specify the URL with the DOCKER_HOST environ
 
             exit(1)
 
+        try:
             return Project.from_config(self.project_name, config, self.client)
+        except ConfigError as e:
+            raise UserError(six.text_type(e))
 
     @cached_property
     def project_name(self):

fig/service.py

@@ -10,6 +10,14 @@ from .container import Container
 log = logging.getLogger(__name__)
 
 
+DOCKER_CONFIG_KEYS = ['image', 'command', 'hostname', 'user', 'detach', 'stdin_open', 'tty', 'mem_limit', 'ports', 'environment', 'dns', 'volumes', 'volumes_from', 'entrypoint']
+DOCKER_CONFIG_HINTS = {
+    'link': 'links',
+    'port': 'ports',
+    'volume': 'volumes',
+}
+
+
 class BuildError(Exception):
     pass
 
@@ -18,14 +26,27 @@ class CannotBeScaledError(Exception):
     pass
 
 
+class ConfigError(ValueError):
+    pass
+
+
 class Service(object):
     def __init__(self, name, client=None, project='default', links=[], **options):
         if not re.match('^[a-zA-Z0-9]+$', name):
-            raise ValueError('Invalid name: %s' % name)
+            raise ConfigError('Invalid name: %s' % name)
         if not re.match('^[a-zA-Z0-9]+$', project):
-            raise ValueError('Invalid project: %s' % project)
+            raise ConfigError('Invalid project: %s' % project)
         if 'image' in options and 'build' in options:
-            raise ValueError('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)
+            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)
+
+        supported_options = DOCKER_CONFIG_KEYS + ['build']
+
+        for k in options:
+            if k not in supported_options:
+                msg = "Unsupported config option for %s service: '%s'" % (name, k)
+                if k in DOCKER_CONFIG_HINTS:
+                    msg += " (did you mean '%s'?)" % DOCKER_CONFIG_HINTS[k]
+                raise ConfigError(msg)
 
         self.name = name
         self.client = client
@@ -218,8 +239,7 @@ class Service(object):
         return links
 
     def _get_container_options(self, override_options, one_off=False):
-        keys = ['image', 'command', 'hostname', 'user', 'detach', 'stdin_open', 'tty', 'mem_limit', 'ports', 'environment', 'dns', 'volumes', 'volumes_from', 'entrypoint']
+        container_options = dict((k, self.options[k]) for k in DOCKER_CONFIG_KEYS if k in self.options)
-        container_options = dict((k, self.options[k]) for k in keys if k in self.options)
         container_options.update(override_options)
 
         container_options['name'] = self.next_container_name(one_off)

tests/service_test.py

@@ -1,30 +1,34 @@
 from __future__ import unicode_literals
 from __future__ import absolute_import
 from fig import Service
-from fig.service import CannotBeScaledError
+from fig.service import CannotBeScaledError, ConfigError
 from .testcases import DockerClientTestCase
 
 
 class ServiceTest(DockerClientTestCase):
     def test_name_validations(self):
-        self.assertRaises(ValueError, lambda: Service(name=''))
+        self.assertRaises(ConfigError, lambda: Service(name=''))
 
-        self.assertRaises(ValueError, lambda: Service(name=' '))
+        self.assertRaises(ConfigError, lambda: Service(name=' '))
-        self.assertRaises(ValueError, lambda: Service(name='/'))
+        self.assertRaises(ConfigError, lambda: Service(name='/'))
-        self.assertRaises(ValueError, lambda: Service(name='!'))
+        self.assertRaises(ConfigError, lambda: Service(name='!'))
-        self.assertRaises(ValueError, lambda: Service(name='\xe2'))
+        self.assertRaises(ConfigError, lambda: Service(name='\xe2'))
-        self.assertRaises(ValueError, lambda: Service(name='_'))
+        self.assertRaises(ConfigError, lambda: Service(name='_'))
-        self.assertRaises(ValueError, lambda: Service(name='____'))
+        self.assertRaises(ConfigError, lambda: Service(name='____'))
-        self.assertRaises(ValueError, lambda: Service(name='foo_bar'))
+        self.assertRaises(ConfigError, lambda: Service(name='foo_bar'))
-        self.assertRaises(ValueError, lambda: Service(name='__foo_bar__'))
+        self.assertRaises(ConfigError, lambda: Service(name='__foo_bar__'))
 
         Service('a')
         Service('foo')
 
     def test_project_validation(self):
-        self.assertRaises(ValueError, lambda: Service(name='foo', project='_'))
+        self.assertRaises(ConfigError, lambda: Service(name='foo', project='_'))
         Service(name='foo', project='bar')
 
+    def test_config_validation(self):
+        self.assertRaises(ConfigError, lambda: Service(name='foo', port=['8000']))
+        Service(name='foo', ports=['8000'])
+
     def test_containers(self):
         foo = self.create_service('foo')
         bar = self.create_service('bar')