From b9bae89f1d25c05ddd3438f6968c96ef3a9fb16b Mon Sep 17 00:00:00 2001 From: Guillermo Arribas Date: Wed, 11 Oct 2017 13:56:15 -0300 Subject: [PATCH] Config command generates invalid volumes (fixes #5176) Signed-off-by: Guillermo Arribas --- compose/config/config.py | 19 +++---- compose/config/serialize.py | 4 +- tests/acceptance/cli_test.py | 54 +++++++++++++++++-- .../volumes/external-volumes-v2-x.yml | 17 ++++++ ...al-volumes.yml => external-volumes-v2.yml} | 2 +- .../volumes/external-volumes-v3-4.yml | 17 ++++++ .../volumes/external-volumes-v3-x.yml | 16 ++++++ 7 files changed, 114 insertions(+), 15 deletions(-) create mode 100644 tests/fixtures/volumes/external-volumes-v2-x.yml rename tests/fixtures/volumes/{external-volumes.yml => external-volumes-v2.yml} (92%) create mode 100644 tests/fixtures/volumes/external-volumes-v3-4.yml create mode 100644 tests/fixtures/volumes/external-volumes-v3-x.yml diff --git a/compose/config/config.py b/compose/config/config.py index 68b2be3a6..7bb57076e 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 606dd7614..2b8c73f14 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 b598d99d5..43cc89e36 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 000000000..3b736c5f4 --- /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 similarity index 92% rename from tests/fixtures/volumes/external-volumes.yml rename to tests/fixtures/volumes/external-volumes-v2.yml index 05c6c4844..4025b53b1 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 000000000..76c8421dc --- /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 000000000..903fee647 --- /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