From 24d4a1045a6317a612f856af3cc99e1b301eae49 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 16 Oct 2015 15:45:56 -0400 Subject: [PATCH] Fixes #2203 - properly validate files when multiple files are used. Remove the single-use decorators so the functionality can be used directly as a function. Signed-off-by: Daniel Nephin --- compose/config/config.py | 22 +++++++------------- compose/config/validation.py | 35 +++++++++++++------------------- tests/unit/config/config_test.py | 22 ++++++++++++++++++++ 3 files changed, 43 insertions(+), 36 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 59b98f609..05e572585 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -14,7 +14,6 @@ from .validation import validate_against_fields_schema from .validation import validate_against_service_schema from .validation import validate_extended_service_exists from .validation import validate_extends_file_path -from .validation import validate_service_names from .validation import validate_top_level_object @@ -165,16 +164,6 @@ def find_candidates_in_parent_dirs(filenames, path): return (candidates, path) -@validate_top_level_object -@validate_service_names -def pre_process_config(config): - """ - Pre validation checks and processing of the config file to interpolate env - vars returning a config dict ready to be tested against the schema. - """ - return interpolate_environment_variables(config) - - def load(config_details): """Load the configuration from a working directory and a list of configuration files. Files are loaded in order, and merged on top @@ -194,7 +183,7 @@ def load(config_details): return service_dict def load_file(filename, config): - processed_config = pre_process_config(config) + processed_config = interpolate_environment_variables(config) validate_against_fields_schema(processed_config) return [ build_service(filename, name, service_config) @@ -209,7 +198,10 @@ def load(config_details): } config_file = config_details.config_files[0] + validate_top_level_object(config_file.config) for next_file in config_details.config_files[1:]: + validate_top_level_object(next_file.config) + config_file = ConfigFile( config_file.filename, merge_services(config_file.config, next_file.config)) @@ -283,9 +275,9 @@ class ServiceLoader(object): ) self.extended_service_name = extends['service'] - full_extended_config = pre_process_config( - load_yaml(self.extended_config_path) - ) + config = load_yaml(self.extended_config_path) + validate_top_level_object(config) + full_extended_config = interpolate_environment_variables(config) validate_extended_service_exists( self.extended_service_name, diff --git a/compose/config/validation.py b/compose/config/validation.py index 46b891b71..aea722864 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -2,7 +2,6 @@ import json import logging import os import sys -from functools import wraps import six from docker.utils.ports import split_port @@ -65,27 +64,21 @@ def format_boolean_in_environment(instance): return True -def validate_service_names(func): - @wraps(func) - def func_wrapper(config): - for service_name in config.keys(): - if type(service_name) is int: - raise ConfigurationError( - "Service name: {} needs to be a string, eg '{}'".format(service_name, service_name) - ) - return func(config) - return func_wrapper - - -def validate_top_level_object(func): - @wraps(func) - def func_wrapper(config): - if not isinstance(config, dict): +def validate_service_names(config): + for service_name in config.keys(): + if not isinstance(service_name, six.string_types): raise ConfigurationError( - "Top level object needs to be a dictionary. Check your .yml file that you have defined a service at the top level." - ) - return func(config) - return func_wrapper + "Service name: {} needs to be a string, eg '{}'".format( + service_name, + service_name)) + + +def validate_top_level_object(config): + if not isinstance(config, dict): + raise ConfigurationError( + "Top level object needs to be a dictionary. Check your .yml file " + "that you have defined a service at the top level.") + validate_service_names(config) def validate_extends_file_path(service_name, extends_options, filename): diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index b4bd9c712..e8caea814 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -134,6 +134,28 @@ class ConfigTest(unittest.TestCase): ] self.assertEqual(service_sort(service_dicts), service_sort(expected)) + def test_load_with_multiple_files_and_empty_override(self): + base_file = config.ConfigFile( + 'base.yaml', + {'web': {'image': 'example/web'}}) + override_file = config.ConfigFile('override.yaml', None) + details = config.ConfigDetails('.', [base_file, override_file]) + + with pytest.raises(ConfigurationError) as exc: + config.load(details) + assert 'Top level object needs to be a dictionary' in exc.exconly() + + def test_load_with_multiple_files_and_empty_base(self): + base_file = config.ConfigFile('base.yaml', None) + override_file = config.ConfigFile( + 'override.yaml', + {'web': {'image': 'example/web'}}) + details = config.ConfigDetails('.', [base_file, override_file]) + + with pytest.raises(ConfigurationError) as exc: + config.load(details) + assert 'Top level object needs to be a dictionary' in exc.exconly() + def test_config_valid_service_names(self): for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']: config.load(