From f7d808769405e52e4c84acdcf15875614cb71993 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 12 Nov 2015 12:40:36 -0500 Subject: [PATCH 1/4] Add ids to config schemas Also enforce a max complexity for functions and add some new tests for config. Signed-off-by: Daniel Nephin --- compose/config/fields_schema.json | 6 ++++-- compose/config/service_schema.json | 11 ++++------- compose/config/validation.py | 5 ++++- tests/unit/config/config_test.py | 24 ++++++++++++++++-------- tox.ini | 2 ++ 5 files changed, 30 insertions(+), 18 deletions(-) diff --git a/compose/config/fields_schema.json b/compose/config/fields_schema.json index 7723f2fb..a174ba87 100644 --- a/compose/config/fields_schema.json +++ b/compose/config/fields_schema.json @@ -2,15 +2,18 @@ "$schema": "http://json-schema.org/draft-04/schema#", "type": "object", + "id": "fields_schema.json", "patternProperties": { "^[a-zA-Z0-9._-]+$": { "$ref": "#/definitions/service" } }, + "additionalProperties": false, "definitions": { "service": { + "id": "#/definitions/service", "type": "object", "properties": { @@ -167,6 +170,5 @@ ] } - }, - "additionalProperties": false + } } diff --git a/compose/config/service_schema.json b/compose/config/service_schema.json index 221c5d8d..05774efd 100644 --- a/compose/config/service_schema.json +++ b/compose/config/service_schema.json @@ -1,15 +1,17 @@ { "$schema": "http://json-schema.org/draft-04/schema#", + "id": "service_schema.json", "type": "object", "allOf": [ {"$ref": "fields_schema.json#/definitions/service"}, - {"$ref": "#/definitions/service_constraints"} + {"$ref": "#/definitions/constraints"} ], "definitions": { - "service_constraints": { + "constraints": { + "id": "#/definitions/constraints", "anyOf": [ { "required": ["build"], @@ -21,13 +23,8 @@ {"required": ["build"]}, {"required": ["dockerfile"]} ]} - }, - { - "required": ["extends"], - "not": {"required": ["build", "image"]} } ] } } - } diff --git a/compose/config/validation.py b/compose/config/validation.py index d3bcb35c..962d41e2 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -307,7 +307,10 @@ def _validate_against_schema(config, schema_filename, format_checker=[], service schema = json.load(schema_fh) resolver = RefResolver(resolver_full_path, schema) - validation_output = Draft4Validator(schema, resolver=resolver, format_checker=FormatChecker(format_checker)) + validation_output = Draft4Validator( + schema, + resolver=resolver, + format_checker=FormatChecker(format_checker)) errors = [error for error in sorted(validation_output.iter_errors(config), key=str)] if errors: diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index add7a5a4..03e338cb 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -78,14 +78,12 @@ class ConfigTest(unittest.TestCase): def test_config_invalid_service_names(self): for invalid_name in ['?not?allowed', ' ', '', '!', '/', '\xe2']: - with pytest.raises(ConfigurationError): - config.load( - build_config_details( - {invalid_name: {'image': 'busybox'}}, - 'working_dir', - 'filename.yml' - ) - ) + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details( + {invalid_name: {'image': 'busybox'}}, + 'working_dir', + 'filename.yml')) + assert 'Invalid service name \'%s\'' % invalid_name in exc.exconly() def test_load_with_invalid_field_name(self): config_details = build_config_details( @@ -97,6 +95,16 @@ class ConfigTest(unittest.TestCase): error_msg = "Unsupported config option for 'web' service: 'name'" assert error_msg in exc.exconly() + def test_load_invalid_service_definition(self): + config_details = build_config_details( + {'web': 'wrong'}, + 'working_dir', + '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() + def test_config_integer_service_name_raise_validation_error(self): expected_error_msg = "Service name: 1 needs to be a string, eg '1'" with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): diff --git a/tox.ini b/tox.ini index f05c5ed2..d1098a55 100644 --- a/tox.ini +++ b/tox.ini @@ -43,4 +43,6 @@ directory = coverage-html [flake8] # Allow really long lines for now max-line-length = 140 +# Set this high for now +max-complexity = 20 exclude = compose/packages From fa96484d2835b8711e560d0c22626c67b99b2407 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 12 Nov 2015 12:43:29 -0500 Subject: [PATCH 2/4] Refactor process_errors into smaller functions So that it passed new max-complexity requirement Signed-off-by: Daniel Nephin --- compose/config/validation.py | 316 +++++++++++++++---------------- tests/unit/config/config_test.py | 2 +- 2 files changed, 149 insertions(+), 169 deletions(-) diff --git a/compose/config/validation.py b/compose/config/validation.py index 962d41e2..2928238c 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -109,189 +109,169 @@ def anglicize_validator(validator): return 'a ' + validator -def process_errors(errors, service_name=None): +def handle_error_for_schema_with_id(error, service_name): + schema_id = error.schema['id'] + + if schema_id == 'fields_schema.json' and error.validator == 'additionalProperties': + return "Invalid service name '{}' - only {} characters are allowed".format( + # The service_name is the key to the json object + list(error.instance)[0], + VALID_NAME_CHARS) + + if schema_id == '#/definitions/constraints': + if 'image' in error.instance and 'build' in error.instance: + return ( + "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)) + if 'image' not in error.instance and 'build' not in error.instance: + return ( + "Service '{}' has neither an image nor a build path " + "specified. Exactly one must be provided.".format(service_name)) + if 'image' in error.instance and 'dockerfile' in error.instance: + return ( + "Service '{}' has both an image and alternate Dockerfile. " + "A service can either be built to image or use an existing " + "image, not both.".format(service_name)) + + if schema_id == '#/definitions/service': + if error.validator == 'additionalProperties': + invalid_config_key = parse_key_from_error_msg(error) + return get_unsupported_config_msg(service_name, invalid_config_key) + + +def handle_generic_service_error(error, service_name): + config_key = " ".join("'%s'" % k for k in error.path) + msg_format = None + error_msg = error.message + + if error.validator == 'oneOf': + msg_format = "Service '{}' configuration key {} {}" + error_msg = _parse_oneof_validator(error) + + elif error.validator == 'type': + msg_format = ("Service '{}' configuration key {} contains an invalid " + "type, it should be {}") + error_msg = _parse_valid_types_from_validator(error.validator_value) + + # TODO: no test case for this branch, there are no config options + # which exercise this branch + elif error.validator == 'required': + msg_format = "Service '{}' configuration key '{}' is invalid, {}" + + elif error.validator == 'dependencies': + msg_format = "Service '{}' configuration key '{}' is invalid: {}" + config_key = list(error.validator_value.keys())[0] + required_keys = ",".join(error.validator_value[config_key]) + error_msg = "when defining '{}' you must set '{}' as well".format( + config_key, + required_keys) + + elif error.path: + msg_format = "Service '{}' configuration key {} value {}" + + if msg_format: + return msg_format.format(service_name, config_key, error_msg) + + return error.message + + +def parse_key_from_error_msg(error): + return error.message.split("'")[1] + + +def _parse_valid_types_from_validator(validator): + """A validator value can be either an array of valid types or a string of + a valid type. Parse the valid types and prefix with the correct article. """ - jsonschema gives us an error tree full of information to explain what has + if not isinstance(validator, list): + return anglicize_validator(validator) + + if len(validator) == 1: + return anglicize_validator(validator[0]) + + return "{}, or {}".format( + ", ".join([anglicize_validator(validator[0])] + validator[1:-1]), + anglicize_validator(validator[-1])) + + +def _parse_oneof_validator(error): + """oneOf has multiple schemas, so we need to reason about which schema, sub + schema or constraint the validation is failing on. + Inspecting the context value of a ValidationError gives us information about + which sub schema failed and which kind of error it is. + """ + types = [] + for context in error.context: + + if context.validator == 'required': + return context.message + + if context.validator == 'additionalProperties': + invalid_config_key = parse_key_from_error_msg(context) + return "contains unsupported option: '{}'".format(invalid_config_key) + + if context.path: + invalid_config_key = " ".join( + "'{}' ".format(fragment) for fragment in context.path + if isinstance(fragment, six.string_types) + ) + return "{}contains {}, which is an invalid type, it should be {}".format( + invalid_config_key, + context.instance, + _parse_valid_types_from_validator(context.validator_value)) + + if context.validator == 'uniqueItems': + return "contains non unique items, please remove duplicates from {}".format( + context.instance) + + if context.validator == 'type': + types.append(context.validator_value) + + valid_types = _parse_valid_types_from_validator(types) + return "contains an invalid type, it should be {}".format(valid_types) + + +def process_errors(errors, service_name=None): + """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] + def format_error_message(error, service_name): + if not service_name and error.path: + # field_schema errors will have service name on the path + service_name = error.path.popleft() - def _clean_error_message(message): - return message.replace("u'", "'") + if 'id' in error.schema: + error_msg = handle_error_for_schema_with_id(error, service_name) + if error_msg: + return error_msg - def _parse_valid_types_from_validator(validator): - """ - A validator value can be either an array of valid types or a string of - a valid type. Parse the valid types and prefix with the correct article. - """ - if isinstance(validator, list): - if len(validator) >= 2: - first_type = anglicize_validator(validator[0]) - last_type = anglicize_validator(validator[-1]) - types_from_validator = ", ".join([first_type] + validator[1:-1]) + return handle_generic_service_error(error, service_name) - msg = "{} or {}".format( - types_from_validator, - last_type - ) - else: - msg = "{}".format(anglicize_validator(validator[0])) - else: - msg = "{}".format(anglicize_validator(validator)) - - return msg - - def _parse_oneof_validator(error): - """ - oneOf has multiple schemas, so we need to reason about which schema, sub - schema or constraint the validation is failing on. - Inspecting the context value of a ValidationError gives us information about - which sub schema failed and which kind of error it is. - """ - required = [context for context in error.context if context.validator == 'required'] - if required: - return required[0].message - - additionalProperties = [context for context in error.context if context.validator == 'additionalProperties'] - if additionalProperties: - invalid_config_key = _parse_key_from_error_msg(additionalProperties[0]) - return "contains unsupported option: '{}'".format(invalid_config_key) - - constraint = [context for context in error.context if len(context.path) > 0] - if constraint: - valid_types = _parse_valid_types_from_validator(constraint[0].validator_value) - invalid_config_key = "".join( - "'{}' ".format(fragment) for fragment in constraint[0].path - if isinstance(fragment, six.string_types) - ) - msg = "{}contains {}, which is an invalid type, it should be {}".format( - invalid_config_key, - constraint[0].instance, - valid_types - ) - return msg - - uniqueness = [context for context in error.context if context.validator == 'uniqueItems'] - if uniqueness: - msg = "contains non unique items, please remove duplicates from {}".format( - uniqueness[0].instance - ) - return msg - - types = [context.validator_value for context in error.context if context.validator == 'type'] - valid_types = _parse_valid_types_from_validator(types) - - msg = "contains an invalid type, it should be {}".format(valid_types) - - return msg - - root_msgs = [] - invalid_keys = [] - required = [] - type_errors = [] - other_errors = [] - - for error in errors: - # handle root level errors - if len(error.path) == 0 and not service_name: - 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(_clean_error_message(error.message)) - - else: - if not service_name: - # field_schema errors will have service name on the path - service_name = error.path[0] - error.path.popleft() - else: - # service_schema errors have the service name passed in, as that - # is not available on error.path or necessarily error.instance - service_name = service_name - - 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)) - elif 'image' in error.instance and 'dockerfile' in error.instance: - required.append( - "Service '{}' has both an image and alternate Dockerfile. " - "A service can either be built to image or use an existing " - "image, not both.".format(service_name)) - else: - required.append(_clean_error_message(error.message)) - elif error.validator == 'oneOf': - config_key = error.path[0] - msg = _parse_oneof_validator(error) - - type_errors.append("Service '{}' configuration key '{}' {}".format( - service_name, config_key, msg) - ) - elif error.validator == 'type': - msg = _parse_valid_types_from_validator(error.validator_value) - - if len(error.path) > 0: - config_key = " ".join(["'%s'" % k for k in error.path]) - type_errors.append( - "Service '{}' configuration key {} contains an invalid " - "type, it should be {}".format( - service_name, - config_key, - msg)) - else: - 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(service_name)) - elif error.validator == 'required': - config_key = error.path[0] - required.append( - "Service '{}' option '{}' is invalid, {}".format( - service_name, - config_key, - _clean_error_message(error.message))) - elif error.validator == 'dependencies': - dependency_key = list(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)) - else: - config_key = " ".join(["'%s'" % k for k in error.path]) - err_msg = "Service '{}' configuration key {} value {}".format(service_name, config_key, error.message) - other_errors.append(err_msg) - - return "\n".join(root_msgs + invalid_keys + required + type_errors + other_errors) + return '\n'.join(format_error_message(error, service_name) for error in errors) def validate_against_fields_schema(config): - schema_filename = "fields_schema.json" - format_checkers = ["ports", "environment"] - return _validate_against_schema(config, schema_filename, format_checkers) + return _validate_against_schema( + config, + "fields_schema.json", + ["ports", "environment"]) def validate_against_service_schema(config, service_name): - schema_filename = "service_schema.json" - format_checkers = ["ports"] - return _validate_against_schema(config, schema_filename, format_checkers, service_name) + return _validate_against_schema( + config, + "service_schema.json", + ["ports"], + service_name) -def _validate_against_schema(config, schema_filename, format_checker=[], service_name=None): +def _validate_against_schema( + config, + schema_filename, + format_checker=(), + service_name=None): config_source_dir = os.path.dirname(os.path.abspath(__file__)) if sys.platform == "win32": diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 03e338cb..9abc58e4 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -867,7 +867,7 @@ class MemoryOptionsTest(unittest.TestCase): a mem_limit """ expected_error_msg = ( - "Invalid 'memswap_limit' configuration for 'foo' service: when " + "Service 'foo' configuration key 'memswap_limit' is invalid: when " "defining 'memswap_limit' you must set 'mem_limit' as well" ) with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): From 589755d03448b04dff441895949116d647b64bc1 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 9 Nov 2015 20:01:20 -0500 Subject: [PATCH 3/4] Inclide the filename in validation errors. Signed-off-by: Daniel Nephin --- compose/config/config.py | 4 +- compose/config/interpolation.py | 7 --- compose/config/validation.py | 60 ++++++++++++------ tests/unit/config/config_test.py | 104 +++++++++++++++---------------- 4 files changed, 95 insertions(+), 80 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 21788551..2c1fdeb9 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -229,9 +229,9 @@ def load(config_details): def process_config_file(config_file, service_name=None): - validate_top_level_object(config_file.config) + validate_top_level_object(config_file) processed_config = interpolate_environment_variables(config_file.config) - validate_against_fields_schema(processed_config) + validate_against_fields_schema(processed_config, config_file.filename) if service_name and service_name not in processed_config: raise ConfigurationError( diff --git a/compose/config/interpolation.py b/compose/config/interpolation.py index f8e1da61..ba7e35c1 100644 --- a/compose/config/interpolation.py +++ b/compose/config/interpolation.py @@ -18,13 +18,6 @@ def interpolate_environment_variables(config): def process_service(service_name, service_dict, mapping): - if not isinstance(service_dict, dict): - raise ConfigurationError( - 'Service "%s" doesn\'t have any configuration options. ' - 'All top level keys in your docker-compose.yml must map ' - 'to a dictionary of configuration options.' % service_name - ) - return dict( (key, interpolate_value(service_name, key, val, mapping)) for (key, val) in service_dict.items() diff --git a/compose/config/validation.py b/compose/config/validation.py index 2928238c..38866b0f 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -66,21 +66,38 @@ def format_boolean_in_environment(instance): return True -def validate_service_names(config): - for service_name in config.keys(): +def validate_top_level_service_objects(config_file): + """Perform some high level validation of the service name and value. + + 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. + """ + for service_name, service_dict in config_file.config.items(): if not isinstance(service_name, six.string_types): raise ConfigurationError( - "Service name: {} needs to be a string, eg '{}'".format( + "In file '{}' service name: {} needs to be a string, eg '{}'".format( + config_file.filename, service_name, service_name)) + if not isinstance(service_dict, dict): + 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( + config_file.filename, + service_name)) -def validate_top_level_object(config): - if not isinstance(config, dict): + +def validate_top_level_object(config_file): + if not isinstance(config_file.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) + "Top level object in '{}' needs to be an object not '{}'. Check " + "that you have defined a service at the top level.".format( + config_file.filename, + type(config_file.config))) + validate_top_level_service_objects(config_file) def validate_extends_file_path(service_name, extends_options, filename): @@ -252,26 +269,28 @@ def process_errors(errors, service_name=None): return '\n'.join(format_error_message(error, service_name) for error in errors) -def validate_against_fields_schema(config): - return _validate_against_schema( +def validate_against_fields_schema(config, filename): + _validate_against_schema( config, "fields_schema.json", - ["ports", "environment"]) + format_checker=["ports", "environment"], + filename=filename) def validate_against_service_schema(config, service_name): - return _validate_against_schema( + _validate_against_schema( config, "service_schema.json", - ["ports"], - service_name) + format_checker=["ports"], + service_name=service_name) def _validate_against_schema( config, schema_filename, format_checker=(), - service_name=None): + service_name=None, + filename=None): config_source_dir = os.path.dirname(os.path.abspath(__file__)) if sys.platform == "win32": @@ -293,6 +312,11 @@ def _validate_against_schema( format_checker=FormatChecker(format_checker)) errors = [error for error in sorted(validation_output.iter_errors(config), key=str)] - if errors: - error_msg = process_errors(errors, service_name) - raise ConfigurationError("Validation failed, reason(s):\n{}".format(error_msg)) + if not errors: + return + + error_msg = process_errors(errors, service_name) + file_msg = " in file '{}'".format(filename) if filename else '' + raise ConfigurationError("Validation failed{}, reason(s):\n{}".format( + file_msg, + error_msg)) diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 9abc58e4..84ed4943 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -94,6 +94,7 @@ class ConfigTest(unittest.TestCase): config.load(config_details) error_msg = "Unsupported config option for 'web' service: 'name'" assert error_msg in exc.exconly() + assert "Validation failed in file 'filename.yml'" in exc.exconly() def test_load_invalid_service_definition(self): config_details = build_config_details( @@ -102,11 +103,12 @@ 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" + error_msg = "service 'web' doesn't have any configuration options" assert error_msg in exc.exconly() def test_config_integer_service_name_raise_validation_error(self): - expected_error_msg = "Service name: 1 needs to be a string, eg '1'" + expected_error_msg = ("In file 'filename.yml' service name: 1 needs to " + "be a string, eg '1'") with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): config.load( build_config_details( @@ -156,25 +158,26 @@ class ConfigTest(unittest.TestCase): def test_load_with_multiple_files_and_empty_override(self): base_file = config.ConfigFile( - 'base.yaml', + 'base.yml', {'web': {'image': 'example/web'}}) - override_file = config.ConfigFile('override.yaml', None) + override_file = config.ConfigFile('override.yml', 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() + error_msg = "Top level object in 'override.yml' needs to be an object" + assert error_msg in exc.exconly() def test_load_with_multiple_files_and_empty_base(self): - base_file = config.ConfigFile('base.yaml', None) + base_file = config.ConfigFile('base.yml', None) override_file = config.ConfigFile( - 'override.yaml', + 'override.yml', {'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() + assert "Top level object in 'base.yml' needs to be an object" in exc.exconly() def test_load_with_multiple_files_and_extends_in_override_file(self): base_file = config.ConfigFile( @@ -225,17 +228,17 @@ 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' doesn't have any configuration" in exc.exconly() + assert "In file 'override.yaml'" in exc.exconly() def test_config_valid_service_names(self): for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']: - config.load( + services = config.load( build_config_details( {valid_name: {'image': 'busybox'}}, 'tests/fixtures/extends', - 'common.yml' - ) - ) + 'common.yml')) + assert services[0]['name'] == valid_name def test_config_invalid_ports_format_validation(self): expected_error_msg = "Service 'web' configuration key 'ports' contains an invalid type" @@ -300,7 +303,8 @@ class ConfigTest(unittest.TestCase): ) def test_invalid_config_not_a_dictionary(self): - expected_error_msg = "Top level object needs to be a dictionary." + expected_error_msg = ("Top level object in 'filename.yml' needs to be " + "an object.") with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): config.load( build_config_details( @@ -382,12 +386,13 @@ class ConfigTest(unittest.TestCase): ) def test_config_ulimits_invalid_keys_validation_error(self): - expected_error_msg = "Service 'web' configuration key 'ulimits' contains unsupported option: 'not_soft_or_hard'" + expected = ("Service 'web' configuration key 'ulimits' 'nofile' contains " + "unsupported option: 'not_soft_or_hard'") - with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): - config.load( - build_config_details( - {'web': { + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details( + { + 'web': { 'image': 'busybox', 'ulimits': { 'nofile': { @@ -396,50 +401,43 @@ class ConfigTest(unittest.TestCase): "hard": 20000, } } - }}, - 'working_dir', - 'filename.yml' - ) - ) + } + }, + 'working_dir', + 'filename.yml')) + assert expected in exc.exconly() def test_config_ulimits_required_keys_validation_error(self): - expected_error_msg = "Service 'web' configuration key 'ulimits' u?'hard' is a required property" - with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): - config.load( - build_config_details( - {'web': { + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details( + { + 'web': { 'image': 'busybox', - 'ulimits': { - 'nofile': { - "soft": 10000, - } - } - }}, - 'working_dir', - 'filename.yml' - ) - ) + 'ulimits': {'nofile': {"soft": 10000}} + } + }, + 'working_dir', + 'filename.yml')) + assert "Service 'web' configuration key 'ulimits' 'nofile'" in exc.exconly() + assert "'hard' is a required property" in exc.exconly() def test_config_ulimits_soft_greater_than_hard_error(self): - expected_error_msg = "cannot contain a 'soft' value higher than 'hard' value" + expected = "cannot contain a 'soft' value higher than 'hard' value" - with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): - config.load( - build_config_details( - {'web': { + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details( + { + 'web': { 'image': 'busybox', 'ulimits': { - 'nofile': { - "soft": 10000, - "hard": 1000 - } + 'nofile': {"soft": 10000, "hard": 1000} } - }}, - 'working_dir', - 'filename.yml' - ) - ) + } + }, + 'working_dir', + 'filename.yml')) + assert expected in exc.exconly() def test_valid_config_which_allows_two_type_definitions(self): expose_values = [["8000"], [8000]] From 9c305ac10f6c6900e432602ccefa5e0bdd83f9e1 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 12 Nov 2015 13:26:13 -0500 Subject: [PATCH 4/4] Remove name field from the list of ALLOWED_KEYS Signed-off-by: Daniel Nephin --- compose/config/config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/compose/config/config.py b/compose/config/config.py index 2c1fdeb9..20126620 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -65,7 +65,6 @@ ALLOWED_KEYS = DOCKER_CONFIG_KEYS + [ 'dockerfile', 'expose', 'external_links', - 'name', ]