Some additional validation for container ports, and a couple extra test cases.

Signed-off-by: Daniel Nephin <[email protected]>

fig/service.py

@@ -245,7 +245,7 @@ class Service(object):
     def start_or_create_containers(self):
         containers = self.containers(stopped=True)
 
-        if len(containers) == 0:
+        if not containers:
             log.info("Creating %s..." % self.next_container_name())
             new_container = self.create_container()
             return [self.start_container(new_container)]
@@ -451,20 +451,20 @@ def build_volume_binding(volume_spec):
 
 
 def split_port(port):
-    port = str(port)
-    external_ip = None
-    if ':' in port:
-        external_port, internal_port = port.rsplit(':', 1)
-        if ':' in external_port:
-            external_ip, external_port = external_port.split(':', 1)
-    else:
-        external_port, internal_port = (None, port)
-    if external_ip:
-        if external_port:
-            external_port = (external_ip, external_port)
-        else:
-            external_port = (external_ip,)
-    return internal_port, external_port
+    parts = str(port).split(':')
+    if not 1 <= len(parts) <= 3:
+        raise ConfigError('Invalid port "%s", should be '
+                          '[[remote_ip:]remote_port:]port[/protocol]' % port)
+
+    if len(parts) == 1:
+        internal_port, = parts
+        return internal_port, None
+    if len(parts) == 2:
+        external_port, internal_port = parts
+        return internal_port, external_port
+
+    external_ip, external_port, internal_port = parts
+    return internal_port, (external_ip, external_port or None)
 
 
 def split_env(env):

tests/unit/service_test.py

@@ -38,23 +38,35 @@ class ServiceTest(unittest.TestCase):
         self.assertRaises(ConfigError, lambda: Service(name='foo', port=['8000']))
         Service(name='foo', ports=['8000'])
 
-    def test_split_port(self):
+    def test_split_port_with_host_ip(self):
         internal_port, external_port = split_port("127.0.0.1:1000:2000")
         self.assertEqual(internal_port, "2000")
         self.assertEqual(external_port, ("127.0.0.1", "1000"))
 
+    def test_split_port_with_protocol(self):
         internal_port, external_port = split_port("127.0.0.1:1000:2000/udp")
         self.assertEqual(internal_port, "2000/udp")
         self.assertEqual(external_port, ("127.0.0.1", "1000"))
 
+    def test_split_port_with_host_ip_no_port(self):
         internal_port, external_port = split_port("127.0.0.1::2000")
         self.assertEqual(internal_port, "2000")
-        self.assertEqual(external_port, ("127.0.0.1",))
+        self.assertEqual(external_port, ("127.0.0.1", None))
 
+    def test_split_port_with_host_port(self):
         internal_port, external_port = split_port("1000:2000")
         self.assertEqual(internal_port, "2000")
         self.assertEqual(external_port, "1000")
 
+    def test_split_port_no_host_port(self):
+        internal_port, external_port = split_port("2000")
+        self.assertEqual(internal_port, "2000")
+        self.assertEqual(external_port, None)
+
+    def test_split_port_invalid(self):
+        with self.assertRaises(ConfigError):
+            split_port("0.0.0.0:1000:2000:tcp")
+
     def test_split_domainname_none(self):
         service = Service('foo',
                 hostname = 'name',