Merge pull request #2655 from shin-/updated_volume_definition

Properly validate volume definition

compose/config/config.py

@@ -273,9 +273,21 @@ def load_volumes(config_files):
     volumes = {}
     for config_file in config_files:
         for name, volume_config in config_file.config.get('volumes', {}).items():
+            if volume_config is None:
+                volumes.update({name: {}})
+                continue
+
             volumes.update({name: volume_config})
             external = volume_config.get('external')
             if external:
+                if len(volume_config.keys()) > 1:
+                    raise ConfigurationError(
+                        'Volume {0} declared as external but specifies'
+                        ' additional attributes ({1}). '.format(
+                            name,
+                            ', '.join([k for k in volume_config.keys() if k != 'external'])
+                        )
+                    )
                 if isinstance(external, dict):
                     volume_config['external_name'] = external.get('name')
                 else:

compose/config/fields_schema_v2.json

@@ -32,32 +32,24 @@
   "definitions": {
     "volume": {
       "id": "#/definitions/volume",
-      "oneOf": [{
-        "type": "object",
-        "properties": {
-          "driver": {"type": "string"},
-          "driver_opts": {
-            "type": "object",
-            "patternProperties": {
-              "^.+$": {"type": ["string", "number"]}
-            },
-            "additionalProperties": false
+      "type": ["object", "null"],
+      "properties": {
+        "driver": {"type": "string"},
+        "driver_opts": {
+          "type": "object",
+          "patternProperties": {
+            "^.+$": {"type": ["string", "number"]}
           }
         },
-        "additionalProperties": false
-      }, {
-        "type": "object",
-        "properties": {
-          "external": {
-            "type": ["boolean", "object"],
-            "properties": {
-              "name": {"type": "string"}
-            },
-            "additionalProperties": false
+        "external": {
+          "type": ["boolean", "object"],
+          "properties": {
+            "name": {"type": "string"}
           }
         },
         "additionalProperties": false
-      }]
+      },
+      "additionalProperties": false
     }
   },
   "additionalProperties": false

tests/unit/config/config_test.py

@@ -112,6 +112,23 @@ class ConfigTest(unittest.TestCase):
             }
         })
 
+    def test_named_volume_config_empty(self):
+        config_details = build_config_details({
+            'version': 2,
+            'services': {
+                'simple': {'image': 'busybox'}
+            },
+            'volumes': {
+                'simple': None,
+                'other': {},
+            }
+        })
+        config_result = config.load(config_details)
+        volumes = config_result.volumes
+        assert 'simple' in volumes
+        assert volumes['simple'] == {}
+        assert volumes['other'] == {}
+
     def test_load_service_with_name_version(self):
         config_data = config.load(
             build_config_details({