diff --git a/compose/config/fields_schema.json b/compose/config/fields_schema.json index 5c7322517d5..6277b57d696 100644 --- a/compose/config/fields_schema.json +++ b/compose/config/fields_schema.json @@ -24,12 +24,7 @@ ] }, "container_name": {"type": "string"}, - "cpu_shares": { - "oneOf": [ - {"type": "number"}, - {"type": "string"} - ] - }, + "cpu_shares": {"type": ["number", "string"]}, "cpuset": {"type": "string"}, "devices": {"type": "array", "items": {"type": "string"}, "uniqueItems": true}, "dns": {"$ref": "#/definitions/string_or_list"}, @@ -74,18 +69,8 @@ "log_opt": {"type": "object"}, "mac_address": {"type": "string"}, - "mem_limit": { - "oneOf": [ - {"type": "number"}, - {"type": "string"} - ] - }, - "memswap_limit": { - "oneOf": [ - {"type": "number"}, - {"type": "string"} - ] - }, + "mem_limit": {"type": ["number", "string"]}, + "memswap_limit": {"type": ["number", "string"]}, "name": {"type": "string"}, "net": {"type": "string"}, "pid": {"type": ["string", "null"]}, diff --git a/compose/config/validation.py b/compose/config/validation.py index 632bdf03bd5..dc630adf279 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -95,6 +95,12 @@ def get_unsupported_config_msg(service_name, error_key): return msg +def anglicize_validator(validator): + if validator in ["array", "object"]: + return 'an ' + validator + return 'a ' + validator + + def process_errors(errors, service_name=None): """ jsonschema gives us an error tree full of information to explain what has @@ -107,15 +113,57 @@ def _parse_key_from_error_msg(error): def _clean_error_message(message): return message.replace("u'", "'") - def _parse_valid_types_from_schema(schema): + def _parse_valid_types_from_validator(validator): """ - Our defined types using $ref in the schema require some extra parsing - retrieve a helpful type for error message display. + 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 '$ref' in schema: - return schema['$ref'].replace("#/definitions/", "").replace("_", " ") + if isinstance(validator, list): + if len(validator) >= 2: + first_type = anglicize_validator(validator[0]) + last_type = anglicize_validator(validator[-1]) + types_from_validator = "{}{}".format(first_type, ", ".join(validator[1:-1])) + + msg = "{} or {}".format( + types_from_validator, + last_type + ) + else: + msg = "{}".format(anglicize_validator(validator[0])) else: - return str(schema['type']) + 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. + """ + 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) + msg = "contains {}, which is an invalid type, it should be {}".format( + 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 = [] @@ -168,27 +216,22 @@ def _parse_valid_types_from_schema(schema): required.append(_clean_error_message(error.message)) elif error.validator == 'oneOf': config_key = error.path[0] + msg = _parse_oneof_validator(error) - valid_types = [_parse_valid_types_from_schema(schema) for schema in error.schema['oneOf']] - valid_type_msg = " or ".join(valid_types) - - type_errors.append("Service '{}' configuration key '{}' contains an invalid type, valid types are {}".format( - service_name, config_key, valid_type_msg) + type_errors.append("Service '{}' configuration key '{}' {}".format( + service_name, config_key, msg) ) elif error.validator == 'type': - msg = "a" - if error.validator_value == "array": - msg = "an" + 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( + "type, it should be {}".format( service_name, config_key, - msg, - error.validator_value)) + msg)) else: root_msgs.append( "Service '{}' doesn\'t have any configuration options. " diff --git a/tests/unit/config_test.py b/tests/unit/config_test.py index 870adcf81e9..f55789207d7 100644 --- a/tests/unit/config_test.py +++ b/tests/unit/config_test.py @@ -183,7 +183,8 @@ def test_invalid_config_not_unique_items(self): ) def test_invalid_list_of_strings_format(self): - expected_error_msg = "'command' contains an invalid type, valid types are string or array" + expected_error_msg = "Service 'web' configuration key 'command' contains 1" + expected_error_msg += ", which is an invalid type, it should be a string" with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): config.load( config.ConfigDetails( @@ -222,7 +223,7 @@ def test_config_extra_hosts_string_raises_validation_error(self): ) def test_config_extra_hosts_list_of_dicts_validation_error(self): - expected_error_msg = "Service 'web' configuration key 'extra_hosts' contains an invalid type" + expected_error_msg = "key 'extra_hosts' contains {'somehost': '162.242.195.82'}, which is an invalid type, it should be a string" with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): config.load( @@ -269,6 +270,21 @@ def test_valid_config_oneof_string_or_list(self): ) self.assertEqual(service[0]['entrypoint'], entrypoint) + def test_validation_message_for_invalid_type_when_multiple_types_allowed(self): + expected_error_msg = "Service 'web' configuration key 'mem_limit' contains an invalid type, it should be a number or a string" + + with self.assertRaisesRegexp(ConfigurationError, expected_error_msg): + config.load( + config.ConfigDetails( + {'web': { + 'image': 'busybox', + 'mem_limit': ['incorrect'] + }}, + 'working_dir', + 'filename.yml' + ) + ) + class InterpolationTest(unittest.TestCase): @mock.patch.dict(os.environ)