From adb64ef8d5406c26834b2bd0a4dfab1de47ff9ca Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 5 Feb 2016 20:07:04 -0500 Subject: [PATCH 1/3] Merge v2 config jsonschemas into a single file. Signed-off-by: Daniel Nephin --- compose/config/config.py | 8 +- ...hema_v2.0.json => config_schema_v2.0.json} | 116 ++++++++++++++++-- compose/config/fields_schema_v2.0.json | 96 --------------- compose/config/validation.py | 14 +-- docker-compose.spec | 9 +- 5 files changed, 115 insertions(+), 128 deletions(-) rename compose/config/{service_schema_v2.0.json => config_schema_v2.0.json} (74%) delete mode 100644 compose/config/fields_schema_v2.0.json diff --git a/compose/config/config.py b/compose/config/config.py index 4e91a3af2..3994a3323 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -31,12 +31,12 @@ from .types import ServiceLink from .types import VolumeFromSpec 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_against_config_schema from .validation import validate_depends_on from .validation import validate_extends_file_path from .validation import validate_network_mode +from .validation import validate_service_constraints from .validation import validate_top_level_object from .validation import validate_ulimits @@ -415,7 +415,7 @@ def process_config_file(config_file, service_name=None): processed_config = services config_file = config_file._replace(config=processed_config) - validate_against_fields_schema(config_file) + validate_against_config_schema(config_file) if service_name and service_name not in services: raise ConfigurationError( @@ -548,7 +548,7 @@ def validate_extended_service_dict(service_dict, filename, service): def validate_service(service_config, service_names, version): service_dict, service_name = service_config.config, service_config.name - validate_against_service_schema(service_dict, service_name, version) + validate_service_constraints(service_dict, service_name, version) validate_paths(service_dict) validate_ulimits(service_config) diff --git a/compose/config/service_schema_v2.0.json b/compose/config/config_schema_v2.0.json similarity index 74% rename from compose/config/service_schema_v2.0.json rename to compose/config/config_schema_v2.0.json index edccedc66..e8ceb4c2b 100644 --- a/compose/config/service_schema_v2.0.json +++ b/compose/config/config_schema_v2.0.json @@ -1,15 +1,50 @@ { "$schema": "http://json-schema.org/draft-04/schema#", - "id": "service_schema_v2.0.json", - + "id": "config_schema_v2.0.json", "type": "object", - "allOf": [ - {"$ref": "#/definitions/service"}, - {"$ref": "#/definitions/constraints"} - ], + "properties": { + "version": { + "type": "string" + }, + + "services": { + "id": "#/properties/services", + "type": "object", + "patternProperties": { + "^[a-zA-Z0-9._-]+$": { + "$ref": "#/definitions/service" + } + }, + "additionalProperties": false + }, + + "networks": { + "id": "#/properties/networks", + "type": "object", + "patternProperties": { + "^[a-zA-Z0-9._-]+$": { + "$ref": "#/definitions/network" + } + } + }, + + "volumes": { + "id": "#/properties/volumes", + "type": "object", + "patternProperties": { + "^[a-zA-Z0-9._-]+$": { + "$ref": "#/definitions/volume" + } + }, + "additionalProperties": false + } + }, + + "additionalProperties": false, "definitions": { + "service": { "id": "#/definitions/service", "type": "object", @@ -193,6 +228,60 @@ "additionalProperties": false }, + "network": { + "id": "#/definitions/network", + "type": "object", + "properties": { + "driver": {"type": "string"}, + "driver_opts": { + "type": "object", + "patternProperties": { + "^.+$": {"type": ["string", "number"]} + } + }, + "ipam": { + "type": "object", + "properties": { + "driver": {"type": "string"}, + "config": { + "type": "array" + } + }, + "additionalProperties": false + }, + "external": { + "type": ["boolean", "object"], + "properties": { + "name": {"type": "string"} + }, + "additionalProperties": false + } + }, + "additionalProperties": false + }, + + "volume": { + "id": "#/definitions/volume", + "type": ["object", "null"], + "properties": { + "driver": {"type": "string"}, + "driver_opts": { + "type": "object", + "patternProperties": { + "^.+$": {"type": ["string", "number"]} + } + }, + "external": { + "type": ["boolean", "object"], + "properties": { + "name": {"type": "string"} + } + }, + "additionalProperties": false + }, + "additionalProperties": false + }, + "string_or_list": { "oneOf": [ {"type": "string"}, @@ -221,15 +310,18 @@ {"type": "array", "items": {"type": "string"}, "uniqueItems": true} ] }, + "constraints": { - "id": "#/definitions/constraints", - "anyOf": [ + "services": { + "id": "#/definitions/services/constraints", + "anyOf": [ {"required": ["build"]}, {"required": ["image"]} - ], - "properties": { - "build": { - "required": ["context"] + ], + "properties": { + "build": { + "required": ["context"] + } } } } diff --git a/compose/config/fields_schema_v2.0.json b/compose/config/fields_schema_v2.0.json deleted file mode 100644 index 7703adcd0..000000000 --- a/compose/config/fields_schema_v2.0.json +++ /dev/null @@ -1,96 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-04/schema#", - "type": "object", - "id": "fields_schema_v2.0.json", - - "properties": { - "version": { - "type": "string" - }, - "services": { - "id": "#/properties/services", - "type": "object", - "patternProperties": { - "^[a-zA-Z0-9._-]+$": { - "$ref": "service_schema_v2.0.json#/definitions/service" - } - }, - "additionalProperties": false - }, - "networks": { - "id": "#/properties/networks", - "type": "object", - "patternProperties": { - "^[a-zA-Z0-9._-]+$": { - "$ref": "#/definitions/network" - } - } - }, - "volumes": { - "id": "#/properties/volumes", - "type": "object", - "patternProperties": { - "^[a-zA-Z0-9._-]+$": { - "$ref": "#/definitions/volume" - } - }, - "additionalProperties": false - } - }, - - "definitions": { - "network": { - "id": "#/definitions/network", - "type": "object", - "properties": { - "driver": {"type": "string"}, - "driver_opts": { - "type": "object", - "patternProperties": { - "^.+$": {"type": ["string", "number"]} - } - }, - "ipam": { - "type": "object", - "properties": { - "driver": {"type": "string"}, - "config": { - "type": "array" - } - }, - "additionalProperties": false - }, - "external": { - "type": ["boolean", "object"], - "properties": { - "name": {"type": "string"} - }, - "additionalProperties": false - } - }, - "additionalProperties": false - }, - "volume": { - "id": "#/definitions/volume", - "type": ["object", "null"], - "properties": { - "driver": {"type": "string"}, - "driver_opts": { - "type": "object", - "patternProperties": { - "^.+$": {"type": ["string", "number"]} - } - }, - "external": { - "type": ["boolean", "object"], - "properties": { - "name": {"type": "string"} - } - }, - "additionalProperties": false - }, - "additionalProperties": false - } - }, - "additionalProperties": false -} diff --git a/compose/config/validation.py b/compose/config/validation.py index 60ee5c930..d7ca270ca 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -385,21 +385,17 @@ def process_errors(errors, path_prefix=None): return '\n'.join(format_error_message(error) for error in errors) -def validate_against_fields_schema(config_file): - schema_filename = "fields_schema_v{0}.json".format(config_file.version) +def validate_against_config_schema(config_file): _validate_against_schema( config_file.config, - schema_filename, + "service_schema_v{0}.json".format(config_file.version), format_checker=["ports", "expose", "bool-value-in-mapping"], filename=config_file.filename) -def validate_against_service_schema(config, service_name, version): - _validate_against_schema( - config, - "service_schema_v{0}.json".format(version), - format_checker=["ports"], - path_prefix=[service_name]) +def validate_service_constraints(config, service_name, version): + # TODO: + pass def _validate_against_schema( diff --git a/docker-compose.spec b/docker-compose.spec index b3d8db399..4282400ed 100644 --- a/docker-compose.spec +++ b/docker-compose.spec @@ -22,19 +22,14 @@ exe = EXE(pyz, 'compose/config/fields_schema_v1.json', 'DATA' ), - ( - 'compose/config/fields_schema_v2.0.json', - 'compose/config/fields_schema_v2.0.json', - 'DATA' - ), ( 'compose/config/service_schema_v1.json', 'compose/config/service_schema_v1.json', 'DATA' ), ( - 'compose/config/service_schema_v2.0.json', - 'compose/config/service_schema_v2.0.json', + 'compose/config/config_schema_v2.0.json', + 'compose/config/config_schema_v2.0.json', 'DATA' ), ( From be554c3a74c137e835209b7a1f27b76bd64aa54f Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 5 Feb 2016 20:21:58 -0500 Subject: [PATCH 2/3] Merge v1 config jsonschemas into a single file. Signed-off-by: Daniel Nephin --- ...e_schema_v1.json => config_schema_v1.json} | 44 +++++++++++-------- compose/config/fields_schema_v1.json | 13 ------ compose/config/validation.py | 2 +- docker-compose.spec | 9 +--- 4 files changed, 28 insertions(+), 40 deletions(-) rename compose/config/{service_schema_v1.json => config_schema_v1.json} (89%) delete mode 100644 compose/config/fields_schema_v1.json diff --git a/compose/config/service_schema_v1.json b/compose/config/config_schema_v1.json similarity index 89% rename from compose/config/service_schema_v1.json rename to compose/config/config_schema_v1.json index 4d974d710..affc19f0f 100644 --- a/compose/config/service_schema_v1.json +++ b/compose/config/config_schema_v1.json @@ -1,13 +1,16 @@ { "$schema": "http://json-schema.org/draft-04/schema#", - "id": "service_schema_v1.json", + "id": "config_schema_v1.json", "type": "object", - "allOf": [ - {"$ref": "#/definitions/service"}, - {"$ref": "#/definitions/constraints"} - ], + "patternProperties": { + "^[a-zA-Z0-9._-]+$": { + "$ref": "#/definitions/service" + } + }, + + "additionalProperties": false, "definitions": { "service": { @@ -162,21 +165,24 @@ {"type": "array", "items": {"type": "string"}, "uniqueItems": true} ] }, + "constraints": { - "id": "#/definitions/constraints", - "anyOf": [ - { - "required": ["build"], - "not": {"required": ["image"]} - }, - { - "required": ["image"], - "not": {"anyOf": [ - {"required": ["build"]}, - {"required": ["dockerfile"]} - ]} - } - ] + "services": { + "id": "#/definitions/services/constraints", + "anyOf": [ + { + "required": ["build"], + "not": {"required": ["image"]} + }, + { + "required": ["image"], + "not": {"anyOf": [ + {"required": ["build"]}, + {"required": ["dockerfile"]} + ]} + } + ] + } } } } diff --git a/compose/config/fields_schema_v1.json b/compose/config/fields_schema_v1.json deleted file mode 100644 index 8f6a8c0ad..000000000 --- a/compose/config/fields_schema_v1.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-04/schema#", - - "type": "object", - "id": "fields_schema_v1.json", - - "patternProperties": { - "^[a-zA-Z0-9._-]+$": { - "$ref": "service_schema_v1.json#/definitions/service" - } - }, - "additionalProperties": false -} diff --git a/compose/config/validation.py b/compose/config/validation.py index d7ca270ca..07ec04ef7 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -388,7 +388,7 @@ def process_errors(errors, path_prefix=None): def validate_against_config_schema(config_file): _validate_against_schema( config_file.config, - "service_schema_v{0}.json".format(config_file.version), + "config_schema_v{0}.json".format(config_file.version), format_checker=["ports", "expose", "bool-value-in-mapping"], filename=config_file.filename) diff --git a/docker-compose.spec b/docker-compose.spec index 4282400ed..3a165dd67 100644 --- a/docker-compose.spec +++ b/docker-compose.spec @@ -18,13 +18,8 @@ exe = EXE(pyz, a.datas, [ ( - 'compose/config/fields_schema_v1.json', - 'compose/config/fields_schema_v1.json', - 'DATA' - ), - ( - 'compose/config/service_schema_v1.json', - 'compose/config/service_schema_v1.json', + 'compose/config/config_schema_v1.json', + 'compose/config/config_schema_v1.json', 'DATA' ), ( From dc3a5ce624fb0a0bf2d0c701aaa34df63b6fc5bd Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 10 Feb 2016 16:05:15 -0500 Subject: [PATCH 3/3] Refactor config validation to support constraints in the same jsonschema Reworked the two schema validation functions to read from the same schema but use different parts of it. Error handling is now split as well by the schema that is being used to validate. Signed-off-by: Daniel Nephin --- compose/config/config.py | 2 +- compose/config/config_schema_v1.json | 4 +- compose/config/config_schema_v2.0.json | 4 +- compose/config/validation.py | 153 ++++++++++++------------- tests/unit/config/config_test.py | 19 ++- 5 files changed, 87 insertions(+), 95 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 3994a3323..850af31c9 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -31,8 +31,8 @@ from .types import ServiceLink from .types import VolumeFromSpec from .types import VolumeSpec from .validation import match_named_volumes -from .validation import validate_config_section from .validation import validate_against_config_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 diff --git a/compose/config/config_schema_v1.json b/compose/config/config_schema_v1.json index affc19f0f..cde8c8e56 100644 --- a/compose/config/config_schema_v1.json +++ b/compose/config/config_schema_v1.json @@ -167,8 +167,8 @@ }, "constraints": { - "services": { - "id": "#/definitions/services/constraints", + "service": { + "id": "#/definitions/constraints/service", "anyOf": [ { "required": ["build"], diff --git a/compose/config/config_schema_v2.0.json b/compose/config/config_schema_v2.0.json index e8ceb4c2b..54bfc978d 100644 --- a/compose/config/config_schema_v2.0.json +++ b/compose/config/config_schema_v2.0.json @@ -312,8 +312,8 @@ }, "constraints": { - "services": { - "id": "#/definitions/services/constraints", + "service": { + "id": "#/definitions/constraints/service", "anyOf": [ {"required": ["build"]}, {"required": ["image"]} diff --git a/compose/config/validation.py b/compose/config/validation.py index 07ec04ef7..4eafe7b5c 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -14,6 +14,7 @@ from jsonschema import FormatChecker from jsonschema import RefResolver from jsonschema import ValidationError +from ..const import COMPOSEFILE_V1 as V1 from .errors import ConfigurationError from .errors import VERSION_EXPLANATION from .sort_services import get_service_name_from_network_mode @@ -209,7 +210,7 @@ def anglicize_json_type(json_type): def is_service_dict_schema(schema_id): - return schema_id == 'fields_schema_v1.json' or schema_id == '#/properties/services' + return schema_id in ('config_schema_v1.json', '#/properties/services') def handle_error_for_schema_with_id(error, path): @@ -221,35 +222,6 @@ def handle_error_for_schema_with_id(error, path): list(error.instance)[0], VALID_NAME_CHARS) - if schema_id == '#/definitions/constraints': - # Build context could in 'build' or 'build.context' and dockerfile could be - # in 'dockerfile' or 'build.dockerfile' - context = False - dockerfile = 'dockerfile' in error.instance - if 'build' in error.instance: - if isinstance(error.instance['build'], six.string_types): - context = True - else: - context = 'context' in error.instance['build'] - dockerfile = dockerfile or 'dockerfile' in error.instance['build'] - - # TODO: only applies to v1 - if 'image' in error.instance and context: - return ( - "{} has both an image and build path specified. " - "A service can either be built to image or use an existing " - "image, not both.".format(path_string(path))) - if 'image' not in error.instance and not context: - return ( - "{} has neither an image nor a build path specified. " - "At least one must be provided.".format(path_string(path))) - # TODO: only applies to v1 - if 'image' in error.instance and dockerfile: - return ( - "{} has both an image and alternate Dockerfile. " - "A service can either be built to image or use an existing " - "image, not both.".format(path_string(path))) - if error.validator == 'additionalProperties': if schema_id == '#/definitions/service': invalid_config_key = parse_key_from_error_msg(error) @@ -259,7 +231,7 @@ def handle_error_for_schema_with_id(error, path): return '{}\n{}'.format(error.message, VERSION_EXPLANATION) -def handle_generic_service_error(error, path): +def handle_generic_error(error, path): msg_format = None error_msg = error.message @@ -365,71 +337,94 @@ def _parse_oneof_validator(error): return (None, "contains an invalid type, it should be {}".format(valid_types)) -def process_errors(errors, path_prefix=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. - """ - path_prefix = path_prefix or [] +def process_service_constraint_errors(error, service_name, version): + if version == V1: + 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)) - def format_error_message(error): - path = path_prefix + list(error.path) + 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 'id' in error.schema: - error_msg = handle_error_for_schema_with_id(error, path) - if error_msg: - return error_msg + if 'image' not in error.instance and 'build' not in error.instance: + return ( + "Service {} has neither an image nor a build context specified. " + "At least one must be provided.".format(service_name)) - return handle_generic_service_error(error, path) - return '\n'.join(format_error_message(error) for error in errors) +def process_config_schema_errors(error): + path = list(error.path) + + if 'id' in error.schema: + error_msg = handle_error_for_schema_with_id(error, path) + if error_msg: + return error_msg + + return handle_generic_error(error, path) def validate_against_config_schema(config_file): - _validate_against_schema( - config_file.config, - "config_schema_v{0}.json".format(config_file.version), - format_checker=["ports", "expose", "bool-value-in-mapping"], - filename=config_file.filename) + schema = load_jsonschema(config_file.version) + format_checker = FormatChecker(["ports", "expose", "bool-value-in-mapping"]) + validator = Draft4Validator( + schema, + resolver=RefResolver(get_resolver_path(), schema), + format_checker=format_checker) + handle_errors( + validator.iter_errors(config_file.config), + process_config_schema_errors, + config_file.filename) def validate_service_constraints(config, service_name, version): - # TODO: - pass + def handler(errors): + return process_service_constraint_errors(errors, service_name, version) + + schema = load_jsonschema(version) + validator = Draft4Validator(schema['definitions']['constraints']['service']) + handle_errors(validator.iter_errors(config), handler, None) -def _validate_against_schema( - config, - schema_filename, - format_checker=(), - path_prefix=None, - filename=None): - config_source_dir = os.path.dirname(os.path.abspath(__file__)) +def get_schema_path(): + return os.path.dirname(os.path.abspath(__file__)) + +def load_jsonschema(version): + filename = os.path.join( + get_schema_path(), + "config_schema_v{0}.json".format(version)) + + with open(filename, "r") as fh: + return json.load(fh) + + +def get_resolver_path(): + schema_path = get_schema_path() if sys.platform == "win32": - file_pre_fix = "///" - config_source_dir = config_source_dir.replace('\\', '/') + scheme = "///" + # TODO: why is this necessary? + schema_path = schema_path.replace('\\', '/') else: - file_pre_fix = "//" + scheme = "//" + return "file:{}{}/".format(scheme, schema_path) - resolver_full_path = "file:{}{}/".format(file_pre_fix, config_source_dir) - schema_file = os.path.join(config_source_dir, schema_filename) - with open(schema_file, "r") as schema_fh: - schema = json.load(schema_fh) - - resolver = RefResolver(resolver_full_path, schema) - validation_output = Draft4Validator( - schema, - resolver=resolver, - format_checker=FormatChecker(format_checker)) - - errors = [error for error in sorted(validation_output.iter_errors(config), key=str)] +def handle_errors(errors, format_error_func, filename): + """jsonschema returns 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. + """ + errors = list(sorted(errors, key=str)) if not errors: return - error_msg = process_errors(errors, path_prefix=path_prefix) - file_msg = " in file '{}'".format(filename) if filename else '' - raise ConfigurationError("Validation failed{}, reason(s):\n{}".format( - file_msg, - error_msg)) + error_msg = '\n'.join(format_error_func(error) for error in errors) + raise ConfigurationError( + "Validation failed{file_msg}, reason(s):\n{error_msg}".format( + file_msg=" in file '{}'".format(filename) if filename else "", + error_msg=error_msg)) diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 11bc7f0b7..f8f224a08 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -342,20 +342,17 @@ class ConfigTest(unittest.TestCase): for invalid_name in ['?not?allowed', ' ', '', '!', '/', '\xe2']: with pytest.raises(ConfigurationError) as exc: config.load(build_config_details( - {invalid_name: {'image': 'busybox'}}, - 'working_dir', - 'filename.yml')) + {invalid_name: {'image': 'busybox'}})) assert 'Invalid service name \'%s\'' % invalid_name in exc.exconly() - def test_config_invalid_service_names_v2(self): + def test_load_config_invalid_service_names_v2(self): for invalid_name in ['?not?allowed', ' ', '', '!', '/', '\xe2']: with pytest.raises(ConfigurationError) as exc: - config.load( - build_config_details({ + config.load(build_config_details( + { 'version': '2', - 'services': {invalid_name: {'image': 'busybox'}} - }, 'working_dir', 'filename.yml') - ) + 'services': {invalid_name: {'image': 'busybox'}}, + })) assert 'Invalid service name \'%s\'' % invalid_name in exc.exconly() def test_load_with_invalid_field_name(self): @@ -1317,7 +1314,7 @@ class ConfigTest(unittest.TestCase): }) with pytest.raises(ConfigurationError) as exc: config.load(config_details) - assert 'one.build is invalid, context is required.' in exc.exconly() + assert 'has neither an image nor a build context' in exc.exconly() class NetworkModeTest(unittest.TestCase): @@ -2269,7 +2266,7 @@ class ExtendsTest(unittest.TestCase): with pytest.raises(ConfigurationError) as exc: load_from_filename('tests/fixtures/extends/service-with-invalid-schema.yml') assert ( - "myweb has neither an image nor a build path specified" in + "myweb has neither an image nor a build context specified" in exc.exconly() )