Fixes #2008 - re-use list_or_dict schema for all the types

At the same time, moves extra_hosts validation to the config module.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

compose/config/config.py

@@ -14,6 +14,7 @@ from .errors import CircularReference
 from .errors import ComposeFileNotFound
 from .errors import ConfigurationError
 from .interpolation import interpolate_environment_variables
+from .types import parse_extra_hosts
 from .types import parse_restart_spec
 from .types import VolumeFromSpec
 from .validation import validate_against_fields_schema
@@ -379,6 +380,9 @@ def process_service(service_config):
     if 'labels' in service_dict:
         service_dict['labels'] = parse_labels(service_dict['labels'])
 
+    if 'extra_hosts' in service_dict:
+        service_dict['extra_hosts'] = parse_extra_hosts(service_dict['extra_hosts'])
+
     # TODO: move to a validate_service()
     if 'ulimits' in service_dict:
         validate_ulimits(service_dict['ulimits'])

compose/config/fields_schema.json

@@ -37,22 +37,7 @@
         "domainname": {"type": "string"},
         "entrypoint": {"$ref": "#/definitions/string_or_list"},
         "env_file": {"$ref": "#/definitions/string_or_list"},
-
-        "environment": {
-          "oneOf": [
-            {
-              "type": "object",
-              "patternProperties": {
-                ".+": {
-                  "type": ["string", "number", "boolean", "null"],
-                  "format": "environment"
-                }
-              },
-              "additionalProperties": false
-            },
-            {"type": "array", "items": {"type": "string"}, "uniqueItems": true}
-          ]
-        },
+        "environment": {"$ref": "#/definitions/list_or_dict"},
 
         "expose": {
           "type": "array",
@@ -165,10 +150,18 @@
 
     "list_or_dict": {
       "oneOf": [
-        {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
-        {"type": "object"}
+        {
+          "type": "object",
+          "patternProperties": {
+            ".+": {
+              "type": ["string", "number", "boolean", "null"],
+              "format": "bool-value-in-mapping"
+            }
+          },
+          "additionalProperties": false
+        },
+        {"type": "array", "items": {"type": "string"}, "uniqueItems": true}
       ]
     }
-
   }
 }

compose/config/types.py

@@ -43,3 +43,19 @@ def parse_restart_spec(restart_config):
         max_retry_count = 0
 
     return {'Name': name, 'MaximumRetryCount': int(max_retry_count)}
+
+
+def parse_extra_hosts(extra_hosts_config):
+    if not extra_hosts_config:
+        return {}
+
+    if isinstance(extra_hosts_config, dict):
+        return dict(extra_hosts_config)
+
+    if isinstance(extra_hosts_config, list):
+        extra_hosts_dict = {}
+        for extra_hosts_line in extra_hosts_config:
+            # TODO: validate string contains ':' ?
+            host, ip = extra_hosts_line.split(':')
+            extra_hosts_dict[host.strip()] = ip.strip()
+        return extra_hosts_dict

compose/config/validation.py

@@ -49,7 +49,7 @@ def format_ports(instance):
     return True
 
 
-@FormatChecker.cls_checks(format="environment")
+@FormatChecker.cls_checks(format="bool-value-in-mapping")
 def format_boolean_in_environment(instance):
     """
     Check if there is a boolean in the environment and display a warning.
@@ -273,7 +273,7 @@ def validate_against_fields_schema(config, filename):
     _validate_against_schema(
         config,
         "fields_schema.json",
-        format_checker=["ports", "environment"],
+        format_checker=["ports", "bool-value-in-mapping"],
         filename=filename)
 
 

compose/service.py

@@ -640,6 +640,7 @@ class Service(object):
         pid = options.get('pid', None)
         security_opt = options.get('security_opt', None)
 
+        # TODO: these options are already normalized by config
         dns = options.get('dns', None)
         if isinstance(dns, six.string_types):
             dns = [dns]
@@ -648,9 +649,6 @@ class Service(object):
         if isinstance(dns_search, six.string_types):
             dns_search = [dns_search]
 
-        extra_hosts = build_extra_hosts(options.get('extra_hosts', None))
-        read_only = options.get('read_only', None)
-
         devices = options.get('devices', None)
         cgroup_parent = options.get('cgroup_parent', None)
         ulimits = build_ulimits(options.get('ulimits', None))
@@ -672,8 +670,8 @@ class Service(object):
             memswap_limit=options.get('memswap_limit'),
             ulimits=ulimits,
             log_config=log_config,
-            extra_hosts=extra_hosts,
-            read_only=read_only,
+            extra_hosts=options.get('extra_hosts'),
+            read_only=options.get('read_only'),
             pid_mode=pid,
             security_opt=security_opt,
             ipc_mode=options.get('ipc'),
@@ -1057,31 +1055,3 @@ def build_ulimits(ulimit_config):
             ulimits.append(ulimit_dict)
 
     return ulimits
-
-
-# Extra hosts
-
-
-def build_extra_hosts(extra_hosts_config):
-    if not extra_hosts_config:
-        return {}
-
-    if isinstance(extra_hosts_config, list):
-        extra_hosts_dict = {}
-        for extra_hosts_line in extra_hosts_config:
-            if not isinstance(extra_hosts_line, six.string_types):
-                raise ConfigError(
-                    "extra_hosts_config \"%s\" must be either a list of strings or a string->string mapping," %
-                    extra_hosts_config
-                )
-            host, ip = extra_hosts_line.split(':')
-            extra_hosts_dict.update({host.strip(): ip.strip()})
-        extra_hosts_config = extra_hosts_dict
-
-    if isinstance(extra_hosts_config, dict):
-        return extra_hosts_config
-
-    raise ConfigError(
-        "extra_hosts_config \"%s\" must be either a list of strings or a string->string mapping," %
-        extra_hosts_config
-    )