Config command generates invalid volumes (fixes #5176)

Signed-off-by: Guillermo Arribas <garribas@gmail.com>

7 files changed, 114 insertions, 15 deletions

diff --git a/compose/config/config.py b/compose/config/config.py
index 68b2be3a..7bb57076 100644
--- a/compose/config/config.py
+++ b/compose/config/config.py
@@ -15,6 +15,9 @@ from cached_property import cached_property
 from . import types
 from .. import const
 from ..const import COMPOSEFILE_V1 as V1
+from ..const import COMPOSEFILE_V2_1 as V2_1
+from ..const import COMPOSEFILE_V3_0 as V3_0
+from ..const import COMPOSEFILE_V3_4 as V3_4
 from ..utils import build_string_dict
 from ..utils import parse_bytes
 from ..utils import parse_nanoseconds_int
@@ -405,7 +408,7 @@ def load_mapping(config_files, get_func, entity_type, working_dir=None):
             external = config.get('external')
             if external:
                 name_field = 'name' if entity_type == 'Volume' else 'external_name'
-                validate_external(entity_type, name, config)
+                validate_external(entity_type, name, config, config_file.version)
                 if isinstance(external, dict):
                     config[name_field] = external.get('name')
                 elif not config.get('name'):
@@ -425,14 +428,12 @@ def load_mapping(config_files, get_func, entity_type, working_dir=None):
     return mapping
 
 
-def validate_external(entity_type, name, config):
-    if len(config.keys()) <= 1:
-        return
-
-    raise ConfigurationError(
-        "{} {} declared as external but specifies additional attributes "
-        "({}).".format(
-            entity_type, name, ', '.join(k for k in config if k != 'external')))
+def validate_external(entity_type, name, config, version):
+    if (version < V2_1 or (version >= V3_0 and version < V3_4)) and len(config.keys()) > 1:
+        raise ConfigurationError(
+            "{} {} declared as external but specifies additional attributes "
+            "({}).".format(
+                entity_type, name, ', '.join(k for k in config if k != 'external')))
 
 
 def load_services(config_details, config_file):
diff --git a/compose/config/serialize.py b/compose/config/serialize.py
index 606dd761..2b8c73f1 100644
--- a/compose/config/serialize.py
+++ b/compose/config/serialize.py
@@ -9,7 +9,7 @@ from compose.const import COMPOSEFILE_V1 as V1
 from compose.const import COMPOSEFILE_V2_1 as V2_1
 from compose.const import COMPOSEFILE_V3_0 as V3_0
 from compose.const import COMPOSEFILE_V3_2 as V3_2
-from compose.const import COMPOSEFILE_V3_2 as V3_4
+from compose.const import COMPOSEFILE_V3_4 as V3_4
 
 
 def serialize_config_type(dumper, data):
@@ -67,7 +67,7 @@ def denormalize_config(config, image_digests=None):
                 del conf['external_name']
 
             if 'name' in conf:
-                if config.version < V2_1 or (config.version > V3_0 and config.version < V3_4):
+                if config.version < V2_1 or (config.version >= V3_0 and config.version < V3_4):
                     del conf['name']
                 elif 'external' in conf:
                     conf['external'] = True
diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py
index b598d99d..43cc89e3 100644
--- a/tests/acceptance/cli_test.py
+++ b/tests/acceptance/cli_test.py
@@ -285,15 +285,63 @@ class CLITestCase(DockerClientTestCase):
             }
         }
 
-    def test_config_external_volume(self):
+    def test_config_external_volume_v2(self):
         self.base_dir = 'tests/fixtures/volumes'
-        result = self.dispatch(['-f', 'external-volumes.yml', 'config'])
+        result = self.dispatch(['-f', 'external-volumes-v2.yml', 'config'])
         json_result = yaml.load(result.stdout)
         assert 'volumes' in json_result
         assert json_result['volumes'] == {
             'foo': {
                 'external': True,
-                'name': 'foo',
+            },
+            'bar': {
+                'external': {
+                    'name': 'some_bar',
+                },
+            }
+        }
+
+    def test_config_external_volume_v2_x(self):
+        self.base_dir = 'tests/fixtures/volumes'
+        result = self.dispatch(['-f', 'external-volumes-v2-x.yml', 'config'])
+        json_result = yaml.load(result.stdout)
+        assert 'volumes' in json_result
+        assert json_result['volumes'] == {
+            'foo': {
+                'external': True,
+                'name': 'some_foo',
+            },
+            'bar': {
+                'external': True,
+                'name': 'some_bar',
+            }
+        }
+
+    def test_config_external_volume_v3_x(self):
+        self.base_dir = 'tests/fixtures/volumes'
+        result = self.dispatch(['-f', 'external-volumes-v3-x.yml', 'config'])
+        json_result = yaml.load(result.stdout)
+        assert 'volumes' in json_result
+        assert json_result['volumes'] == {
+            'foo': {
+                'external': True,
+            },
+            'bar': {
+                'external': {
+                    'name': 'some_bar',
+                },
+            }
+        }
+
+    def test_config_external_volume_v3_4(self):
+        self.base_dir = 'tests/fixtures/volumes'
+        result = self.dispatch(['-f', 'external-volumes-v3-4.yml', 'config'])
+        json_result = yaml.load(result.stdout)
+        assert 'volumes' in json_result
+        assert json_result['volumes'] == {
+            'foo': {
+                'external': True,
+                'name': 'some_foo',
             },
             'bar': {
                 'external': True,
diff --git a/tests/fixtures/volumes/external-volumes-v2-x.yml b/tests/fixtures/volumes/external-volumes-v2-x.yml
new file mode 100644
index 00000000..3b736c5f
--- /dev/null
+++ b/tests/fixtures/volumes/external-volumes-v2-x.yml
@@ -0,0 +1,17 @@
+version: "2.1"
+
+services:
+  web:
+    image: busybox
+    command: top
+    volumes:
+      - foo:/var/lib/
+      - bar:/etc/
+
+volumes:
+  foo:
+    external: true
+    name: some_foo
+  bar:
+    external:
+      name: some_bar
diff --git a/tests/fixtures/volumes/external-volumes.yml b/tests/fixtures/volumes/external-volumes-v2.yml
index 05c6c484..4025b53b 100644
--- a/tests/fixtures/volumes/external-volumes.yml
+++ b/tests/fixtures/volumes/external-volumes-v2.yml
@@ -1,4 +1,4 @@
-version: "2.1"
+version: "2"
 
 services:
   web:
diff --git a/tests/fixtures/volumes/external-volumes-v3-4.yml b/tests/fixtures/volumes/external-volumes-v3-4.yml
new file mode 100644
index 00000000..76c8421d
--- /dev/null
+++ b/tests/fixtures/volumes/external-volumes-v3-4.yml
@@ -0,0 +1,17 @@
+version: "3.4"
+
+services:
+  web:
+    image: busybox
+    command: top
+    volumes:
+      - foo:/var/lib/
+      - bar:/etc/
+
+volumes:
+  foo:
+    external: true
+    name: some_foo
+  bar:
+    external:
+      name: some_bar
diff --git a/tests/fixtures/volumes/external-volumes-v3-x.yml b/tests/fixtures/volumes/external-volumes-v3-x.yml
new file mode 100644
index 00000000..903fee64
--- /dev/null
+++ b/tests/fixtures/volumes/external-volumes-v3-x.yml
@@ -0,0 +1,16 @@
+version: "3.0"
+
+services:
+  web:
+    image: busybox
+    command: top
+    volumes:
+      - foo:/var/lib/
+      - bar:/etc/
+
+volumes:
+  foo:
+    external: true
+  bar:
+    external:
+      name: some_bar