Merge pull request #2948 from dnephin/validate_top_level

Validate that each section of the config is a mapping

compose/config/config.py

@@ -33,11 +33,11 @@ from .types import VolumeSpec
 from .validation import match_named_volumes
 from .validation import validate_against_fields_schema
 from .validation import validate_against_service_schema
+from .validation import validate_config_section
 from .validation import validate_depends_on
 from .validation import validate_extends_file_path
 from .validation import validate_network_mode
 from .validation import validate_top_level_object
-from .validation import validate_top_level_service_objects
 from .validation import validate_ulimits
 
 
@@ -388,22 +388,31 @@ def load_services(working_dir, config_file, service_configs):
     return build_services(service_config)
 
 
-def process_config_file(config_file, service_name=None):
-    service_dicts = config_file.get_service_dicts()
-    validate_top_level_service_objects(config_file.filename, service_dicts)
+def interpolate_config_section(filename, config, section):
+    validate_config_section(filename, config, section)
+    return interpolate_environment_variables(config, section)
 
-    interpolated_config = interpolate_environment_variables(service_dicts, 'service')
+
+def process_config_file(config_file, service_name=None):
+    services = interpolate_config_section(
+        config_file.filename,
+        config_file.get_service_dicts(),
+        'service')
 
     if config_file.version == V2_0:
         processed_config = dict(config_file.config)
-        processed_config['services'] = services = interpolated_config
-        processed_config['volumes'] = interpolate_environment_variables(
-            config_file.get_volumes(), 'volume')
-        processed_config['networks'] = interpolate_environment_variables(
-            config_file.get_networks(), 'network')
+        processed_config['services'] = services
+        processed_config['volumes'] = interpolate_config_section(
+            config_file.filename,
+            config_file.get_volumes(),
+            'volume')
+        processed_config['networks'] = interpolate_config_section(
+            config_file.filename,
+            config_file.get_networks(),
+            'network')
 
     if config_file.version == V1:
-        processed_config = services = interpolated_config
+        processed_config = services
 
     config_file = config_file._replace(config=processed_config)
     validate_against_fields_schema(config_file)

compose/config/interpolation.py

@@ -21,7 +21,7 @@ def interpolate_environment_variables(config, section):
         )
 
     return dict(
-        (name, process_item(name, config_dict))
+        (name, process_item(name, config_dict or {}))
         for name, config_dict in config.items()
     )
 

compose/config/validation.py

@@ -91,29 +91,49 @@ def match_named_volumes(service_dict, project_volumes):
             )
 
 
-def validate_top_level_service_objects(filename, service_dicts):
-    """Perform some high level validation of the service name and value.
+def python_type_to_yaml_type(type_):
+    type_name = type(type_).__name__
+    return {
+        'dict': 'mapping',
+        'list': 'array',
+        'int': 'number',
+        'float': 'number',
+        'bool': 'boolean',
+        'unicode': 'string',
+        'str': 'string',
+        'bytes': 'string',
+    }.get(type_name, type_name)
 
-    This validation must happen before interpolation, which must happen
-    before the rest of validation, which is why it's separate from the
-    rest of the service validation.
+
+def validate_config_section(filename, config, section):
+    """Validate the structure of a configuration section. This must be done
+    before interpolation so it's separate from schema validation.
     """
-    for service_name, service_dict in service_dicts.items():
-        if not isinstance(service_name, six.string_types):
-            raise ConfigurationError(
-                "In file '{}' service name: {} needs to be a string, eg '{}'".format(
-                    filename,
-                    service_name,
-                    service_name))
+    if not isinstance(config, dict):
+        raise ConfigurationError(
+            "In file '{filename}', {section} must be a mapping, not "
+            "{type}.".format(
+                filename=filename,
+                section=section,
+                type=anglicize_json_type(python_type_to_yaml_type(config))))
 
-        if not isinstance(service_dict, dict):
+    for key, value in config.items():
+        if not isinstance(key, six.string_types):
             raise ConfigurationError(
-                "In file '{}' service '{}' doesn\'t have any configuration options. "
-                "All top level keys in your docker-compose.yml must map "
-                "to a dictionary of configuration options.".format(
-                    filename, service_name
-                )
-            )
+                "In file '{filename}', the {section} name {name} must be a "
+                "quoted string, i.e. '{name}'.".format(
+                    filename=filename,
+                    section=section,
+                    name=key))
+
+        if not isinstance(value, (dict, type(None))):
+            raise ConfigurationError(
+                "In file '{filename}', {section} '{name}' must be a mapping not "
+                "{type}.".format(
+                    filename=filename,
+                    section=section,
+                    name=key,
+                    type=anglicize_json_type(python_type_to_yaml_type(value))))
 
 
 def validate_top_level_object(config_file):
@@ -182,10 +202,10 @@ def get_unsupported_config_msg(path, error_key):
     return msg
 
 
-def anglicize_validator(validator):
-    if validator in ["array", "object"]:
-        return 'an ' + validator
-    return 'a ' + validator
+def anglicize_json_type(json_type):
+    if json_type.startswith(('a', 'e', 'i', 'o', 'u')):
+        return 'an ' + json_type
+    return 'a ' + json_type
 
 
 def is_service_dict_schema(schema_id):
@@ -293,14 +313,14 @@ def _parse_valid_types_from_validator(validator):
     a valid type. Parse the valid types and prefix with the correct article.
     """
     if not isinstance(validator, list):
-        return anglicize_validator(validator)
+        return anglicize_json_type(validator)
 
     if len(validator) == 1:
-        return anglicize_validator(validator[0])
+        return anglicize_json_type(validator[0])
 
     return "{}, or {}".format(
-        ", ".join([anglicize_validator(validator[0])] + validator[1:-1]),
-        anglicize_validator(validator[-1]))
+        ", ".join([anglicize_json_type(validator[0])] + validator[1:-1]),
+        anglicize_json_type(validator[-1]))
 
 
 def _parse_oneof_validator(error):

tests/acceptance/cli_test.py

@@ -159,7 +159,7 @@ class CLITestCase(DockerClientTestCase):
             '-f', 'tests/fixtures/invalid-composefile/invalid.yml',
             'config', '-q'
         ], returncode=1)
-        assert "'notaservice' doesn't have any configuration" in result.stderr
+        assert "'notaservice' must be a mapping" in result.stderr
 
     # TODO: this shouldn't be v2-dependent
     @v2_only()

tests/unit/config/config_test.py

@@ -231,7 +231,7 @@ class ConfigTest(unittest.TestCase):
         assert volumes['simple'] == {}
         assert volumes['other'] == {}
 
-    def test_volume_numeric_driver_opt(self):
+    def test_named_volume_numeric_driver_opt(self):
         config_details = build_config_details({
             'version': '2',
             'services': {
@@ -258,6 +258,30 @@ class ConfigTest(unittest.TestCase):
             config.load(config_details)
         assert 'driver_opts.size contains an invalid type' in exc.exconly()
 
+    def test_named_volume_invalid_type_list(self):
+        config_details = build_config_details({
+            'version': '2',
+            'services': {
+                'simple': {'image': 'busybox'}
+            },
+            'volumes': []
+        })
+        with pytest.raises(ConfigurationError) as exc:
+            config.load(config_details)
+        assert "volume must be a mapping, not an array" in exc.exconly()
+
+    def test_networks_invalid_type_list(self):
+        config_details = build_config_details({
+            'version': '2',
+            'services': {
+                'simple': {'image': 'busybox'}
+            },
+            'networks': []
+        })
+        with pytest.raises(ConfigurationError) as exc:
+            config.load(config_details)
+        assert "network must be a mapping, not an array" in exc.exconly()
+
     def test_load_service_with_name_version(self):
         with mock.patch('compose.config.config.log') as mock_logging:
             config_data = config.load(
@@ -368,8 +392,7 @@ class ConfigTest(unittest.TestCase):
             'filename.yml')
         with pytest.raises(ConfigurationError) as exc:
             config.load(config_details)
-        error_msg = "service 'web' doesn't have any configuration options"
-        assert error_msg in exc.exconly()
+        assert "service 'web' must be a mapping not a string." in exc.exconly()
 
     def test_config_integer_service_name_raise_validation_error(self):
         with pytest.raises(ConfigurationError) as excinfo:
@@ -381,8 +404,10 @@ class ConfigTest(unittest.TestCase):
                 )
             )
 
-        assert "In file 'filename.yml' service name: 1 needs to be a string, eg '1'" \
-            in excinfo.exconly()
+        assert (
+            "In file 'filename.yml', the service name 1 must be a quoted string, i.e. '1'" in
+            excinfo.exconly()
+        )
 
     def test_config_integer_service_name_raise_validation_error_v2(self):
         with pytest.raises(ConfigurationError) as excinfo:
@@ -397,8 +422,10 @@ class ConfigTest(unittest.TestCase):
                 )
             )
 
-        assert "In file 'filename.yml' service name: 1 needs to be a string, eg '1'" \
-            in excinfo.exconly()
+        assert (
+            "In file 'filename.yml', the service name 1 must be a quoted string, i.e. '1'." in
+            excinfo.exconly()
+        )
 
     def test_load_with_multiple_files_v1(self):
         base_file = config.ConfigFile(
@@ -532,7 +559,7 @@ class ConfigTest(unittest.TestCase):
 
         with pytest.raises(ConfigurationError) as exc:
             config.load(details)
-        assert "service 'bogus' doesn't have any configuration" in exc.exconly()
+        assert "service 'bogus' must be a mapping not a string." in exc.exconly()
         assert "In file 'override.yaml'" in exc.exconly()
 
     def test_load_sorts_in_dependency_order(self):