diff --git a/compose/config/config.py b/compose/config/config.py index 31e5e916..c1cfdb73 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -3,9 +3,6 @@ import os import sys import yaml from collections import namedtuple -import json -from jsonschema import Draft4Validator, FormatChecker, ValidationError - import six from compose.cli.utils import find_candidates_in_parent_dirs @@ -16,10 +13,9 @@ from .errors import ( CircularReference, ComposeFileNotFound, ) +from .validation import validate_against_schema -VALID_NAME_CHARS = '[a-zA-Z0-9\._\-]' - DOCKER_CONFIG_KEYS = [ 'cap_add', 'cap_drop', @@ -69,22 +65,6 @@ ALLOWED_KEYS = DOCKER_CONFIG_KEYS + [ 'name', ] -DOCKER_CONFIG_HINTS = { - 'cpu_share': 'cpu_shares', - 'add_host': 'extra_hosts', - 'hosts': 'extra_hosts', - 'extra_host': 'extra_hosts', - 'device': 'devices', - 'link': 'links', - 'memory_swap': 'memswap_limit', - 'port': 'ports', - 'privilege': 'privileged', - 'priviliged': 'privileged', - 'privilige': 'privileged', - 'volume': 'volumes', - 'workdir': 'working_dir', -} - SUPPORTED_FILENAMES = [ 'docker-compose.yml', @@ -135,122 +115,18 @@ def get_config_path(base_dir): return os.path.join(path, winner) -@FormatChecker.cls_checks(format="ports", raises=ValidationError("Ports is incorrectly formatted.")) -def format_ports(instance): - def _is_valid(port): - if ':' in port or '/' in port: - return True - try: - int(port) - return True - except ValueError: - return False - return False - - if isinstance(instance, list): - for port in instance: - if not _is_valid(port): - return False - return True - elif isinstance(instance, str): - return _is_valid(instance) - return False - - -def get_unsupported_config_msg(service_name, error_key): - msg = "Unsupported config option for '{}' service: '{}'".format(service_name, error_key) - if error_key in DOCKER_CONFIG_HINTS: - msg += " (did you mean '{}'?)".format(DOCKER_CONFIG_HINTS[error_key]) - return msg - - -def process_errors(errors): - """ - jsonschema gives us an error tree full of information to explain what has - gone wrong. Process each error and pull out relevant information and re-write - helpful error messages that are relevant. - """ - def _parse_key_from_error_msg(error): - return error.message.split("'")[1] - - root_msgs = [] - invalid_keys = [] - required = [] - type_errors = [] - - for error in errors: - # handle root level errors - if len(error.path) == 0: - if error.validator == 'type': - msg = "Top level object needs to be a dictionary. Check your .yml file that you have defined a service at the top level." - root_msgs.append(msg) - elif error.validator == 'additionalProperties': - invalid_service_name = _parse_key_from_error_msg(error) - msg = "Invalid service name '{}' - only {} characters are allowed".format(invalid_service_name, VALID_NAME_CHARS) - root_msgs.append(msg) - else: - root_msgs.append(error.message) - - else: - # handle service level errors - service_name = error.path[0] - - if error.validator == 'additionalProperties': - invalid_config_key = _parse_key_from_error_msg(error) - invalid_keys.append(get_unsupported_config_msg(service_name, invalid_config_key)) - elif error.validator == 'anyOf': - if 'image' in error.instance and 'build' in error.instance: - required.append("Service '{}' has both an image and build path specified. A service can either be built to image or use an existing image, not both.".format(service_name)) - elif 'image' not in error.instance and 'build' not in error.instance: - required.append("Service '{}' has neither an image nor a build path specified. Exactly one must be provided.".format(service_name)) - else: - required.append(error.message) - elif error.validator == 'type': - msg = "a" - if error.validator_value == "array": - msg = "an" - - try: - config_key = error.path[1] - type_errors.append("Service '{}' has an invalid value for '{}', it should be {} {}".format(service_name, config_key, msg, error.validator_value)) - except IndexError: - config_key = error.path[0] - root_msgs.append("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(config_key)) - elif error.validator == 'required': - config_key = error.path[1] - required.append("Service '{}' option '{}' is invalid, {}".format(service_name, config_key, error.message)) - elif error.validator == 'dependencies': - dependency_key = error.validator_value.keys()[0] - required_keys = ",".join(error.validator_value[dependency_key]) - required.append("Invalid '{}' configuration for '{}' service: when defining '{}' you must set '{}' as well".format( - dependency_key, service_name, dependency_key, required_keys)) - - return "\n".join(root_msgs + invalid_keys + required + type_errors) - - -def validate_against_schema(config): - config_source_dir = os.path.dirname(os.path.abspath(__file__)) - schema_file = os.path.join(config_source_dir, "schema.json") - - with open(schema_file, "r") as schema_fh: - schema = json.load(schema_fh) - - validation_output = Draft4Validator(schema, format_checker=FormatChecker(["ports"])) - - errors = [error for error in sorted(validation_output.iter_errors(config), key=str)] - if errors: - error_msg = process_errors(errors) - raise ConfigurationError("Validation failed, reason(s):\n{}".format(error_msg)) - - def load(config_details): config, working_dir, filename = config_details + 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." + ) + config = interpolate_environment_variables(config) + validate_against_schema(config) service_dicts = [] - validate_against_schema(config) - for service_name, service_dict in list(config.items()): loader = ServiceLoader(working_dir=working_dir, filename=filename) service_dict = loader.make_service_dict(service_name, service_dict) @@ -347,13 +223,6 @@ def validate_extended_service_dict(service_dict, filename, service): def process_container_options(service_dict, working_dir=None): - for k in service_dict: - if k not in ALLOWED_KEYS: - msg = "Unsupported config option for %s service: '%s'" % (service_dict['name'], k) - if k in DOCKER_CONFIG_HINTS: - msg += " (did you mean '%s'?)" % DOCKER_CONFIG_HINTS[k] - raise ConfigurationError(msg) - service_dict = service_dict.copy() if 'volumes' in service_dict and service_dict.get('volume_driver') is None: diff --git a/compose/schema.json b/compose/config/schema.json similarity index 100% rename from compose/schema.json rename to compose/config/schema.json diff --git a/compose/config/validation.py b/compose/config/validation.py new file mode 100644 index 00000000..ba5803de --- /dev/null +++ b/compose/config/validation.py @@ -0,0 +1,134 @@ +import os + +import json +from jsonschema import Draft4Validator, FormatChecker, ValidationError + +from .errors import ConfigurationError + + +DOCKER_CONFIG_HINTS = { + 'cpu_share': 'cpu_shares', + 'add_host': 'extra_hosts', + 'hosts': 'extra_hosts', + 'extra_host': 'extra_hosts', + 'device': 'devices', + 'link': 'links', + 'memory_swap': 'memswap_limit', + 'port': 'ports', + 'privilege': 'privileged', + 'priviliged': 'privileged', + 'privilige': 'privileged', + 'volume': 'volumes', + 'workdir': 'working_dir', +} + + +VALID_NAME_CHARS = '[a-zA-Z0-9\._\-]' + + +@FormatChecker.cls_checks(format="ports", raises=ValidationError("Ports is incorrectly formatted.")) +def format_ports(instance): + def _is_valid(port): + if ':' in port or '/' in port: + return True + try: + int(port) + return True + except ValueError: + return False + return False + + if isinstance(instance, list): + for port in instance: + if not _is_valid(port): + return False + return True + elif isinstance(instance, str): + return _is_valid(instance) + return False + + +def get_unsupported_config_msg(service_name, error_key): + msg = "Unsupported config option for '{}' service: '{}'".format(service_name, error_key) + if error_key in DOCKER_CONFIG_HINTS: + msg += " (did you mean '{}'?)".format(DOCKER_CONFIG_HINTS[error_key]) + return msg + + +def process_errors(errors): + """ + jsonschema gives us an error tree full of information to explain what has + gone wrong. Process each error and pull out relevant information and re-write + helpful error messages that are relevant. + """ + def _parse_key_from_error_msg(error): + return error.message.split("'")[1] + + root_msgs = [] + invalid_keys = [] + required = [] + type_errors = [] + + for error in errors: + # handle root level errors + if len(error.path) == 0: + if error.validator == 'type': + msg = "Top level object needs to be a dictionary. Check your .yml file that you have defined a service at the top level." + root_msgs.append(msg) + elif error.validator == 'additionalProperties': + invalid_service_name = _parse_key_from_error_msg(error) + msg = "Invalid service name '{}' - only {} characters are allowed".format(invalid_service_name, VALID_NAME_CHARS) + root_msgs.append(msg) + else: + root_msgs.append(error.message) + + else: + # handle service level errors + service_name = error.path[0] + + if error.validator == 'additionalProperties': + invalid_config_key = _parse_key_from_error_msg(error) + invalid_keys.append(get_unsupported_config_msg(service_name, invalid_config_key)) + elif error.validator == 'anyOf': + if 'image' in error.instance and 'build' in error.instance: + required.append("Service '{}' has both an image and build path specified. A service can either be built to image or use an existing image, not both.".format(service_name)) + elif 'image' not in error.instance and 'build' not in error.instance: + required.append("Service '{}' has neither an image nor a build path specified. Exactly one must be provided.".format(service_name)) + else: + required.append(error.message) + elif error.validator == 'type': + msg = "a" + if error.validator_value == "array": + msg = "an" + + try: + config_key = error.path[1] + type_errors.append("Service '{}' has an invalid value for '{}', it should be {} {}".format(service_name, config_key, msg, error.validator_value)) + except IndexError: + config_key = error.path[0] + root_msgs.append("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(config_key)) + elif error.validator == 'required': + config_key = error.path[1] + required.append("Service '{}' option '{}' is invalid, {}".format(service_name, config_key, error.message)) + elif error.validator == 'dependencies': + dependency_key = error.validator_value.keys()[0] + required_keys = ",".join(error.validator_value[dependency_key]) + required.append("Invalid '{}' configuration for '{}' service: when defining '{}' you must set '{}' as well".format( + dependency_key, service_name, dependency_key, required_keys)) + + return "\n".join(root_msgs + invalid_keys + required + type_errors) + + +def validate_against_schema(config): + config_source_dir = os.path.dirname(os.path.abspath(__file__)) + schema_file = os.path.join(config_source_dir, "schema.json") + + with open(schema_file, "r") as schema_fh: + schema = json.load(schema_fh) + + validation_output = Draft4Validator(schema, format_checker=FormatChecker(["ports"])) + + errors = [error for error in sorted(validation_output.iter_errors(config), key=str)] + if errors: + error_msg = process_errors(errors) + raise ConfigurationError("Validation failed, reason(s):\n{}".format(error_msg)) diff --git a/compose/service.py b/compose/service.py index 103840c3..9b5e5928 100644 --- a/compose/service.py +++ b/compose/service.py @@ -12,7 +12,7 @@ from docker.errors import APIError from docker.utils import create_host_config, LogConfig from . import __version__ -from .config import DOCKER_CONFIG_KEYS, merge_environment, VALID_NAME_CHARS +from .config import DOCKER_CONFIG_KEYS, merge_environment from .const import ( DEFAULT_TIMEOUT, LABEL_CONTAINER_NUMBER, @@ -26,6 +26,7 @@ from .container import Container from .legacy import check_for_legacy_containers from .progress_stream import stream_output, StreamOutputError from .utils import json_hash, parallel_execute +from .config.validation import VALID_NAME_CHARS log = logging.getLogger(__name__) diff --git a/tests/unit/config_test.py b/tests/unit/config_test.py index 15657f87..9f690f11 100644 --- a/tests/unit/config_test.py +++ b/tests/unit/config_test.py @@ -5,6 +5,7 @@ import tempfile from .. import unittest from compose.config import config +from compose.config.errors import ConfigurationError def make_service_dict(name, service_dict, working_dir): @@ -43,7 +44,7 @@ class ConfigTest(unittest.TestCase): ) def test_load_throws_error_when_not_dict(self): - with self.assertRaises(config.ConfigurationError): + with self.assertRaises(ConfigurationError): config.load( config.ConfigDetails( {'web': 'busybox:latest'}, @@ -52,15 +53,8 @@ class ConfigTest(unittest.TestCase): ) ) - def test_config_validation(self): - self.assertRaises( - config.ConfigurationError, - lambda: make_service_dict('foo', {'port': ['8000']}, 'tests/') - ) - make_service_dict('foo', {'ports': ['8000']}, 'tests/') - def test_config_invalid_service_names(self): - with self.assertRaises(config.ConfigurationError): + with self.assertRaises(ConfigurationError): for invalid_name in ['?not?allowed', ' ', '', '!', '/', '\xe2']: config.load( config.ConfigDetails( @@ -81,7 +75,7 @@ class ConfigTest(unittest.TestCase): ) def test_config_invalid_ports_format_validation(self): - with self.assertRaises(config.ConfigurationError): + with self.assertRaises(ConfigurationError): for invalid_ports in [{"1": "8000"}, "whatport"]: config.load( config.ConfigDetails( @@ -104,7 +98,7 @@ class ConfigTest(unittest.TestCase): def test_config_hint(self): expected_error_msg = "(did you mean 'privileged'?)" - with self.assertRaisesRegexp(config.ConfigurationError, expected_error_msg): + with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): config.load( config.ConfigDetails( { @@ -117,7 +111,7 @@ class ConfigTest(unittest.TestCase): def test_invalid_config_build_and_image_specified(self): expected_error_msg = "Service 'foo' has both an image and build path specified." - with self.assertRaisesRegexp(config.ConfigurationError, expected_error_msg): + with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): config.load( config.ConfigDetails( { @@ -130,7 +124,7 @@ class ConfigTest(unittest.TestCase): def test_invalid_config_type_should_be_an_array(self): expected_error_msg = "Service 'foo' has an invalid value for 'links', it should be an array" - with self.assertRaisesRegexp(config.ConfigurationError, expected_error_msg): + with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): config.load( config.ConfigDetails( { @@ -143,7 +137,7 @@ class ConfigTest(unittest.TestCase): def test_invalid_config_not_a_dictionary(self): expected_error_msg = "Top level object needs to be a dictionary." - with self.assertRaisesRegexp(config.ConfigurationError, expected_error_msg): + with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): config.load( config.ConfigDetails( ['foo', 'lol'], @@ -198,7 +192,7 @@ class InterpolationTest(unittest.TestCase): os.environ['VOLUME_PATH'] = '/host/path' d = config.load( config.ConfigDetails( - config={'foo': {'volumes': ['${VOLUME_PATH}:/container/path']}}, + config={'foo': {'build': '.', 'volumes': ['${VOLUME_PATH}:/container/path']}}, working_dir='.', filename=None, ) @@ -422,7 +416,7 @@ class MemoryOptionsTest(unittest.TestCase): a mem_limit """ expected_error_msg = "Invalid 'memswap_limit' configuration for 'foo' service: when defining 'memswap_limit' you must set 'mem_limit' as well" - with self.assertRaisesRegexp(config.ConfigurationError, expected_error_msg): + with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): config.load( config.ConfigDetails( { @@ -465,7 +459,7 @@ class EnvTest(unittest.TestCase): self.assertEqual(config.parse_environment(environment), environment) def test_parse_environment_invalid(self): - with self.assertRaises(config.ConfigurationError): + with self.assertRaises(ConfigurationError): config.parse_environment('a=b') def test_parse_environment_empty(self): @@ -519,7 +513,7 @@ class EnvTest(unittest.TestCase): def test_env_nonexistent_file(self): options = {'env_file': 'nonexistent.env'} self.assertRaises( - config.ConfigurationError, + ConfigurationError, lambda: make_service_dict('foo', options, 'tests/fixtures/env'), ) @@ -545,7 +539,7 @@ class EnvTest(unittest.TestCase): service_dict = config.load( config.ConfigDetails( - config={'foo': {'volumes': ['$HOSTENV:$CONTAINERENV']}}, + config={'foo': {'build': '.', 'volumes': ['$HOSTENV:$CONTAINERENV']}}, working_dir="tests/fixtures/env", filename=None, ) @@ -554,7 +548,7 @@ class EnvTest(unittest.TestCase): service_dict = config.load( config.ConfigDetails( - config={'foo': {'volumes': ['/opt${HOSTENV}:/opt${CONTAINERENV}']}}, + config={'foo': {'build': '.', 'volumes': ['/opt${HOSTENV}:/opt${CONTAINERENV}']}}, working_dir="tests/fixtures/env", filename=None, ) @@ -652,7 +646,7 @@ class ExtendsTest(unittest.TestCase): ) def test_extends_validation_empty_dictionary(self): - with self.assertRaisesRegexp(config.ConfigurationError, 'service'): + with self.assertRaisesRegexp(ConfigurationError, 'service'): config.load( config.ConfigDetails( { @@ -664,7 +658,7 @@ class ExtendsTest(unittest.TestCase): ) def test_extends_validation_missing_service_key(self): - with self.assertRaisesRegexp(config.ConfigurationError, "u'service' is a required property"): + with self.assertRaisesRegexp(ConfigurationError, "u'service' is a required property"): config.load( config.ConfigDetails( { @@ -677,7 +671,7 @@ class ExtendsTest(unittest.TestCase): def test_extends_validation_invalid_key(self): expected_error_msg = "Unsupported config option for 'web' service: 'rogue_key'" - with self.assertRaisesRegexp(config.ConfigurationError, expected_error_msg): + with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): config.load( config.ConfigDetails( { @@ -701,7 +695,7 @@ class ExtendsTest(unittest.TestCase): def load_config(): return make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends') - self.assertRaisesRegexp(config.ConfigurationError, 'file', load_config) + self.assertRaisesRegexp(ConfigurationError, 'file', load_config) def test_extends_validation_valid_config(self): service = config.load( @@ -750,19 +744,19 @@ class ExtendsTest(unittest.TestCase): } }, '.') - with self.assertRaisesRegexp(config.ConfigurationError, 'links'): + with self.assertRaisesRegexp(ConfigurationError, 'links'): other_config = {'web': {'links': ['db']}} with mock.patch.object(config, 'load_yaml', return_value=other_config): print load_config() - with self.assertRaisesRegexp(config.ConfigurationError, 'volumes_from'): + with self.assertRaisesRegexp(ConfigurationError, 'volumes_from'): other_config = {'web': {'volumes_from': ['db']}} with mock.patch.object(config, 'load_yaml', return_value=other_config): print load_config() - with self.assertRaisesRegexp(config.ConfigurationError, 'net'): + with self.assertRaisesRegexp(ConfigurationError, 'net'): other_config = {'web': {'net': 'container:db'}} with mock.patch.object(config, 'load_yaml', return_value=other_config): @@ -804,7 +798,7 @@ class BuildPathTest(unittest.TestCase): self.abs_context_path = os.path.join(os.getcwd(), 'tests/fixtures/build-ctx') def test_nonexistent_path(self): - with self.assertRaises(config.ConfigurationError): + with self.assertRaises(ConfigurationError): config.load( config.ConfigDetails( {