diff --git a/compose/config/config.py b/compose/config/config.py index bda086b9b..094c8b3a2 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -16,10 +16,10 @@ from cached_property import cached_property from ..const import COMPOSEFILE_V1 as V1 from ..const import COMPOSEFILE_V2_0 as V2_0 -from ..const import COMPOSEFILE_VERSIONS from .errors import CircularReference from .errors import ComposeFileNotFound from .errors import ConfigurationError +from .errors import VERSION_EXPLANATION from .interpolation import interpolate_environment_variables from .sort_services import get_container_name_from_network_mode from .sort_services import get_service_name_from_network_mode @@ -105,6 +105,7 @@ SUPPORTED_FILENAMES = [ DEFAULT_OVERRIDE_FILENAME = 'docker-compose.override.yml' + log = logging.getLogger(__name__) @@ -131,7 +132,10 @@ class ConfigFile(namedtuple('_ConfigFile', 'filename config')): @cached_property def version(self): - version = self.config.get('version', V1) + if 'version' not in self.config: + return V1 + + version = self.config['version'] if isinstance(version, dict): log.warn("Unexpected type for field 'version', in file {} assuming " @@ -139,12 +143,23 @@ class ConfigFile(namedtuple('_ConfigFile', 'filename config')): "Compose file version 1".format(self.filename)) return V1 + if not isinstance(version, six.string_types): + raise ConfigurationError( + 'Version in "{}" is invalid - it should be a string.' + .format(self.filename)) + + if version == '1': + raise ConfigurationError( + 'Version in "{}" is invalid. {}' + .format(self.filename, VERSION_EXPLANATION)) + if version == '2': version = V2_0 - if version not in COMPOSEFILE_VERSIONS: + if version != V2_0: raise ConfigurationError( - 'Invalid Compose file version: {0}'.format(version)) + 'Version in "{}" is unsupported. {}' + .format(self.filename, VERSION_EXPLANATION)) return version diff --git a/compose/config/errors.py b/compose/config/errors.py index 99129f3de..f94ac7acd 100644 --- a/compose/config/errors.py +++ b/compose/config/errors.py @@ -2,6 +2,14 @@ from __future__ import absolute_import from __future__ import unicode_literals +VERSION_EXPLANATION = ( + 'Either specify a version of "2" (or "2.0") and place your service ' + 'definitions under the `services` key, or omit the `version` key and place ' + 'your service definitions at the root of the file to use version 1.\n' + 'For more on the Compose file format versions, see ' + 'https://docs.docker.com/compose/compose-file/') + + class ConfigurationError(Exception): def __init__(self, msg): self.msg = msg diff --git a/compose/config/validation.py b/compose/config/validation.py index ba1ac5210..6b2401352 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -15,6 +15,7 @@ from jsonschema import RefResolver from jsonschema import ValidationError from .errors import ConfigurationError +from .errors import VERSION_EXPLANATION from .sort_services import get_service_name_from_network_mode @@ -229,11 +230,14 @@ def handle_error_for_schema_with_id(error, path): "A service can either be built to image or use an existing " "image, not both.".format(path_string(path))) - if schema_id == '#/definitions/service': - if error.validator == 'additionalProperties': + if error.validator == 'additionalProperties': + if schema_id == '#/definitions/service': invalid_config_key = parse_key_from_error_msg(error) return get_unsupported_config_msg(path, invalid_config_key) + if not error.path: + return '{}\n{}'.format(error.message, VERSION_EXPLANATION) + def handle_generic_service_error(error, path): msg_format = None diff --git a/compose/const.py b/compose/const.py index d78a5fa72..0e307835c 100644 --- a/compose/const.py +++ b/compose/const.py @@ -17,7 +17,6 @@ LABEL_CONFIG_HASH = 'com.docker.compose.config-hash' COMPOSEFILE_V1 = '1' COMPOSEFILE_V2_0 = '2.0' -COMPOSEFILE_VERSIONS = (COMPOSEFILE_V1, COMPOSEFILE_V2_0) API_VERSIONS = { COMPOSEFILE_V1: '1.21', diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 302f4703f..3c012ea79 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -17,6 +17,7 @@ from compose.config.config import resolve_environment from compose.config.config import V1 from compose.config.config import V2_0 from compose.config.errors import ConfigurationError +from compose.config.errors import VERSION_EXPLANATION from compose.config.types import VolumeSpec from compose.const import IS_WINDOWS_PLATFORM from tests import mock @@ -159,8 +160,8 @@ class ConfigTest(unittest.TestCase): assert list(s['name'] for s in cfg.services) == ['version'] def test_wrong_version_type(self): - for version in [None, 2, 2.0]: - with pytest.raises(ConfigurationError): + for version in [None, 1, 2, 2.0]: + with pytest.raises(ConfigurationError) as excinfo: config.load( build_config_details( {'version': version}, @@ -168,8 +169,11 @@ class ConfigTest(unittest.TestCase): ) ) + assert 'Version in "filename.yml" is invalid - it should be a string.' \ + in excinfo.exconly() + def test_unsupported_version(self): - with pytest.raises(ConfigurationError): + with pytest.raises(ConfigurationError) as excinfo: config.load( build_config_details( {'version': '2.1'}, @@ -177,18 +181,38 @@ class ConfigTest(unittest.TestCase): ) ) - def test_v1_file_with_version_is_invalid(self): - for version in [1, "1"]: - with pytest.raises(ConfigurationError): - config.load( - build_config_details( - { - 'version': version, - 'web': {'image': 'busybox'}, - }, - filename='filename.yml', - ) + assert 'Version in "filename.yml" is unsupported' in excinfo.exconly() + assert VERSION_EXPLANATION in excinfo.exconly() + + def test_version_1_is_invalid(self): + with pytest.raises(ConfigurationError) as excinfo: + config.load( + build_config_details( + { + 'version': '1', + 'web': {'image': 'busybox'}, + }, + filename='filename.yml', ) + ) + + assert 'Version in "filename.yml" is invalid' in excinfo.exconly() + assert VERSION_EXPLANATION in excinfo.exconly() + + def test_v1_file_with_version_is_invalid(self): + with pytest.raises(ConfigurationError) as excinfo: + config.load( + build_config_details( + { + 'version': '2', + 'web': {'image': 'busybox'}, + }, + filename='filename.yml', + ) + ) + + assert 'Additional properties are not allowed' in excinfo.exconly() + assert VERSION_EXPLANATION in excinfo.exconly() def test_named_volume_config_empty(self): config_details = build_config_details({ @@ -226,27 +250,6 @@ class ConfigTest(unittest.TestCase): ]) ) - def test_load_invalid_version(self): - with self.assertRaises(ConfigurationError): - config.load( - build_config_details({ - 'version': 18, - 'services': { - 'foo': {'image': 'busybox'} - } - }, 'working_dir', 'filename.yml') - ) - - with self.assertRaises(ConfigurationError): - config.load( - build_config_details({ - 'version': 'two point oh', - 'services': { - 'foo': {'image': 'busybox'} - } - }, 'working_dir', 'filename.yml') - ) - def test_load_throws_error_when_not_dict(self): with self.assertRaises(ConfigurationError): config.load(