diff --git a/compose/config/config.py b/compose/config/config.py index 9b03ea4ff..55adcaf28 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -14,6 +14,7 @@ from .errors import CircularReference from .errors import ComposeFileNotFound from .errors import ConfigurationError from .interpolation import interpolate_environment_variables +from .types import parse_extra_hosts from .types import parse_restart_spec from .types import VolumeFromSpec from .validation import validate_against_fields_schema @@ -379,6 +380,9 @@ def process_service(service_config): if 'labels' in service_dict: service_dict['labels'] = parse_labels(service_dict['labels']) + if 'extra_hosts' in service_dict: + service_dict['extra_hosts'] = parse_extra_hosts(service_dict['extra_hosts']) + # TODO: move to a validate_service() if 'ulimits' in service_dict: validate_ulimits(service_dict['ulimits']) diff --git a/compose/config/fields_schema.json b/compose/config/fields_schema.json index ca3b3a502..9cbcfd1b2 100644 --- a/compose/config/fields_schema.json +++ b/compose/config/fields_schema.json @@ -37,22 +37,7 @@ "domainname": {"type": "string"}, "entrypoint": {"$ref": "#/definitions/string_or_list"}, "env_file": {"$ref": "#/definitions/string_or_list"}, - - "environment": { - "oneOf": [ - { - "type": "object", - "patternProperties": { - ".+": { - "type": ["string", "number", "boolean", "null"], - "format": "environment" - } - }, - "additionalProperties": false - }, - {"type": "array", "items": {"type": "string"}, "uniqueItems": true} - ] - }, + "environment": {"$ref": "#/definitions/list_or_dict"}, "expose": { "type": "array", @@ -165,10 +150,18 @@ "list_or_dict": { "oneOf": [ - {"type": "array", "items": {"type": "string"}, "uniqueItems": true}, - {"type": "object"} + { + "type": "object", + "patternProperties": { + ".+": { + "type": ["string", "number", "boolean", "null"], + "format": "bool-value-in-mapping" + } + }, + "additionalProperties": false + }, + {"type": "array", "items": {"type": "string"}, "uniqueItems": true} ] } - } } diff --git a/compose/config/types.py b/compose/config/types.py index 0ab53c825..b6add0894 100644 --- a/compose/config/types.py +++ b/compose/config/types.py @@ -43,3 +43,19 @@ def parse_restart_spec(restart_config): max_retry_count = 0 return {'Name': name, 'MaximumRetryCount': int(max_retry_count)} + + +def parse_extra_hosts(extra_hosts_config): + if not extra_hosts_config: + return {} + + if isinstance(extra_hosts_config, dict): + return dict(extra_hosts_config) + + if isinstance(extra_hosts_config, list): + extra_hosts_dict = {} + for extra_hosts_line in extra_hosts_config: + # TODO: validate string contains ':' ? + host, ip = extra_hosts_line.split(':') + extra_hosts_dict[host.strip()] = ip.strip() + return extra_hosts_dict diff --git a/compose/config/validation.py b/compose/config/validation.py index 38866b0f4..38020366d 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -49,7 +49,7 @@ def format_ports(instance): return True -@FormatChecker.cls_checks(format="environment") +@FormatChecker.cls_checks(format="bool-value-in-mapping") def format_boolean_in_environment(instance): """ Check if there is a boolean in the environment and display a warning. @@ -273,7 +273,7 @@ def validate_against_fields_schema(config, filename): _validate_against_schema( config, "fields_schema.json", - format_checker=["ports", "environment"], + format_checker=["ports", "bool-value-in-mapping"], filename=filename) diff --git a/compose/service.py b/compose/service.py index 33d9a7bec..2bb0030f0 100644 --- a/compose/service.py +++ b/compose/service.py @@ -640,6 +640,7 @@ class Service(object): pid = options.get('pid', None) security_opt = options.get('security_opt', None) + # TODO: these options are already normalized by config dns = options.get('dns', None) if isinstance(dns, six.string_types): dns = [dns] @@ -648,9 +649,6 @@ class Service(object): if isinstance(dns_search, six.string_types): dns_search = [dns_search] - extra_hosts = build_extra_hosts(options.get('extra_hosts', None)) - read_only = options.get('read_only', None) - devices = options.get('devices', None) cgroup_parent = options.get('cgroup_parent', None) ulimits = build_ulimits(options.get('ulimits', None)) @@ -672,8 +670,8 @@ class Service(object): memswap_limit=options.get('memswap_limit'), ulimits=ulimits, log_config=log_config, - extra_hosts=extra_hosts, - read_only=read_only, + extra_hosts=options.get('extra_hosts'), + read_only=options.get('read_only'), pid_mode=pid, security_opt=security_opt, ipc_mode=options.get('ipc'), @@ -1057,31 +1055,3 @@ def build_ulimits(ulimit_config): ulimits.append(ulimit_dict) return ulimits - - -# Extra hosts - - -def build_extra_hosts(extra_hosts_config): - if not extra_hosts_config: - return {} - - if isinstance(extra_hosts_config, list): - extra_hosts_dict = {} - for extra_hosts_line in extra_hosts_config: - if not isinstance(extra_hosts_line, six.string_types): - raise ConfigError( - "extra_hosts_config \"%s\" must be either a list of strings or a string->string mapping," % - extra_hosts_config - ) - host, ip = extra_hosts_line.split(':') - extra_hosts_dict.update({host.strip(): ip.strip()}) - extra_hosts_config = extra_hosts_dict - - if isinstance(extra_hosts_config, dict): - return extra_hosts_config - - raise ConfigError( - "extra_hosts_config \"%s\" must be either a list of strings or a string->string mapping," % - extra_hosts_config - ) diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 15d8ca072..27a290050 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -22,8 +22,6 @@ from compose.const import LABEL_PROJECT from compose.const import LABEL_SERVICE from compose.const import LABEL_VERSION from compose.container import Container -from compose.service import build_extra_hosts -from compose.service import ConfigError from compose.service import ConvergencePlan from compose.service import ConvergenceStrategy from compose.service import Net @@ -139,37 +137,6 @@ class ServiceTest(DockerClientTestCase): container.start() self.assertEqual(container.get('HostConfig.CpuShares'), 73) - def test_build_extra_hosts(self): - # string - self.assertRaises(ConfigError, lambda: build_extra_hosts("www.example.com: 192.168.0.17")) - - # list of strings - self.assertEqual(build_extra_hosts( - ["www.example.com:192.168.0.17"]), - {'www.example.com': '192.168.0.17'}) - self.assertEqual(build_extra_hosts( - ["www.example.com: 192.168.0.17"]), - {'www.example.com': '192.168.0.17'}) - self.assertEqual(build_extra_hosts( - ["www.example.com: 192.168.0.17", - "static.example.com:192.168.0.19", - "api.example.com: 192.168.0.18"]), - {'www.example.com': '192.168.0.17', - 'static.example.com': '192.168.0.19', - 'api.example.com': '192.168.0.18'}) - - # list of dictionaries - self.assertRaises(ConfigError, lambda: build_extra_hosts( - [{'www.example.com': '192.168.0.17'}, - {'api.example.com': '192.168.0.18'}])) - - # dictionaries - self.assertEqual(build_extra_hosts( - {'www.example.com': '192.168.0.17', - 'api.example.com': '192.168.0.18'}), - {'www.example.com': '192.168.0.17', - 'api.example.com': '192.168.0.18'}) - def test_create_container_with_extra_hosts_list(self): extra_hosts = ['somehost:162.242.195.82', 'otherhost:50.31.209.229'] service = self.create_service('db', extra_hosts=extra_hosts) diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index c69e34306..f923fb370 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -32,7 +32,7 @@ def service_sort(services): return sorted(services, key=itemgetter('name')) -def build_config_details(contents, working_dir, filename): +def build_config_details(contents, working_dir='working_dir', filename='filename.yml'): return config.ConfigDetails( working_dir, [config.ConfigFile(filename, contents)]) @@ -512,6 +512,29 @@ class ConfigTest(unittest.TestCase): assert 'line 3, column 32' in exc.exconly() + def test_validate_extra_hosts_invalid(self): + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details({ + 'web': { + 'image': 'alpine', + 'extra_hosts': "www.example.com: 192.168.0.17", + } + })) + assert "'extra_hosts' contains an invalid type" in exc.exconly() + + def test_validate_extra_hosts_invalid_list(self): + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details({ + 'web': { + 'image': 'alpine', + 'extra_hosts': [ + {'www.example.com': '192.168.0.17'}, + {'api.example.com': '192.168.0.18'} + ], + } + })) + assert "which is an invalid type" in exc.exconly() + class InterpolationTest(unittest.TestCase): @mock.patch.dict(os.environ) diff --git a/tests/unit/config/types_test.py b/tests/unit/config/types_test.py new file mode 100644 index 000000000..25692ca37 --- /dev/null +++ b/tests/unit/config/types_test.py @@ -0,0 +1,29 @@ +from compose.config.types import parse_extra_hosts + + +def test_parse_extra_hosts_list(): + expected = {'www.example.com': '192.168.0.17'} + assert parse_extra_hosts(["www.example.com:192.168.0.17"]) == expected + + expected = {'www.example.com': '192.168.0.17'} + assert parse_extra_hosts(["www.example.com: 192.168.0.17"]) == expected + + assert parse_extra_hosts([ + "www.example.com: 192.168.0.17", + "static.example.com:192.168.0.19", + "api.example.com: 192.168.0.18" + ]) == { + 'www.example.com': '192.168.0.17', + 'static.example.com': '192.168.0.19', + 'api.example.com': '192.168.0.18' + } + + +def test_parse_extra_hosts_dict(): + assert parse_extra_hosts({ + 'www.example.com': '192.168.0.17', + 'api.example.com': '192.168.0.18' + }) == { + 'www.example.com': '192.168.0.17', + 'api.example.com': '192.168.0.18' + }