diff --git a/compose/config.py b/compose/config.py index f1883e69..7fceb50c 100644 --- a/compose/config.py +++ b/compose/config.py @@ -134,20 +134,20 @@ def load(config_details): return service_dicts -def make_service_dict(name, service_dict, working_dir=None): - return ServiceLoader(working_dir=working_dir).make_service_dict(name, service_dict) - - class ServiceLoader(object): def __init__(self, working_dir, filename=None, already_seen=None): - self.working_dir = working_dir - self.filename = filename + self.working_dir = os.path.abspath(working_dir) + if filename: + self.filename = os.path.abspath(filename) + else: + self.filename = filename self.already_seen = already_seen or [] - def make_service_dict(self, name, service_dict): + def detect_cycle(self, name): if self.signature(name) in self.already_seen: - raise CircularReference(self.already_seen) + raise CircularReference(self.already_seen + [self.signature(name)]) + def make_service_dict(self, name, service_dict): service_dict = service_dict.copy() service_dict['name'] = name service_dict = resolve_environment(service_dict, working_dir=self.working_dir) @@ -158,12 +158,17 @@ class ServiceLoader(object): if 'extends' not in service_dict: return service_dict - extends_options = process_extends_options(service_dict['name'], service_dict['extends']) + extends_options = self.validate_extends_options(service_dict['name'], service_dict['extends']) if self.working_dir is None: raise Exception("No working_dir passed to ServiceLoader()") - other_config_path = expand_path(self.working_dir, extends_options['file']) + if 'file' in extends_options: + extends_from_filename = extends_options['file'] + other_config_path = expand_path(self.working_dir, extends_from_filename) + else: + other_config_path = self.filename + other_working_dir = os.path.dirname(other_config_path) other_already_seen = self.already_seen + [self.signature(service_dict['name'])] other_loader = ServiceLoader( @@ -174,6 +179,7 @@ class ServiceLoader(object): other_config = load_yaml(other_config_path) other_service_dict = other_config[extends_options['service']] + other_loader.detect_cycle(extends_options['service']) other_service_dict = other_loader.make_service_dict( service_dict['name'], other_service_dict, @@ -189,25 +195,29 @@ class ServiceLoader(object): def signature(self, name): return (self.filename, name) + def validate_extends_options(self, service_name, extends_options): + error_prefix = "Invalid 'extends' configuration for %s:" % service_name -def process_extends_options(service_name, extends_options): - error_prefix = "Invalid 'extends' configuration for %s:" % service_name + if not isinstance(extends_options, dict): + raise ConfigurationError("%s must be a dictionary" % error_prefix) - if not isinstance(extends_options, dict): - raise ConfigurationError("%s must be a dictionary" % error_prefix) - - if 'service' not in extends_options: - raise ConfigurationError( - "%s you need to specify a service, e.g. 'service: web'" % error_prefix - ) - - for k, _ in extends_options.items(): - if k not in ['file', 'service']: + if 'service' not in extends_options: raise ConfigurationError( - "%s unsupported configuration option '%s'" % (error_prefix, k) + "%s you need to specify a service, e.g. 'service: web'" % error_prefix ) - return extends_options + if 'file' not in extends_options and self.filename is None: + raise ConfigurationError( + "%s you need to specify a 'file', e.g. 'file: something.yml'" % error_prefix + ) + + for k, _ in extends_options.items(): + if k not in ['file', 'service']: + raise ConfigurationError( + "%s unsupported configuration option '%s'" % (error_prefix, k) + ) + + return extends_options def validate_extended_service_dict(service_dict, filename, service): diff --git a/tests/fixtures/extends/no-file-specified.yml b/tests/fixtures/extends/no-file-specified.yml new file mode 100644 index 00000000..40e43c4b --- /dev/null +++ b/tests/fixtures/extends/no-file-specified.yml @@ -0,0 +1,9 @@ +myweb: + extends: + service: web + environment: + - "BAR=1" +web: + image: busybox + environment: + - "BAZ=3" diff --git a/tests/fixtures/extends/specify-file-as-self.yml b/tests/fixtures/extends/specify-file-as-self.yml new file mode 100644 index 00000000..7e249976 --- /dev/null +++ b/tests/fixtures/extends/specify-file-as-self.yml @@ -0,0 +1,16 @@ +myweb: + extends: + file: specify-file-as-self.yml + service: web + environment: + - "BAR=1" +web: + extends: + file: specify-file-as-self.yml + service: otherweb + image: busybox + environment: + - "BAZ=3" +otherweb: + environment: + - "YEP=1" diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index 98c5876e..2a7c0a44 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -1,7 +1,7 @@ from __future__ import unicode_literals from __future__ import absolute_import from compose.service import Service -from compose.config import make_service_dict +from compose.config import ServiceLoader from compose.const import LABEL_PROJECT from compose.cli.docker_client import docker_client from compose.progress_stream import stream_output @@ -30,10 +30,12 @@ class DockerClientTestCase(unittest.TestCase): if 'command' not in kwargs: kwargs['command'] = ["top"] + options = ServiceLoader(working_dir='.').make_service_dict(name, kwargs) + return Service( project='composetest', client=self.client, - **make_service_dict(name, kwargs, working_dir='.') + **options ) def check_build(self, *args, **kwargs): diff --git a/tests/unit/config_test.py b/tests/unit/config_test.py index 5a61ad52..d56b2a18 100644 --- a/tests/unit/config_test.py +++ b/tests/unit/config_test.py @@ -7,6 +7,13 @@ from .. import unittest from compose import config +def make_service_dict(name, service_dict, working_dir): + """ + Test helper function to contruct a ServiceLoader + """ + return config.ServiceLoader(working_dir=working_dir).make_service_dict(name, service_dict) + + class ConfigTest(unittest.TestCase): def test_load(self): service_dicts = config.load( @@ -47,22 +54,22 @@ class ConfigTest(unittest.TestCase): def test_config_validation(self): self.assertRaises( config.ConfigurationError, - lambda: config.make_service_dict('foo', {'port': ['8000']}) + lambda: make_service_dict('foo', {'port': ['8000']}, 'tests/') ) - config.make_service_dict('foo', {'ports': ['8000']}) + make_service_dict('foo', {'ports': ['8000']}, 'tests/') class VolumePathTest(unittest.TestCase): @mock.patch.dict(os.environ) def test_volume_binding_with_environ(self): os.environ['VOLUME_PATH'] = '/host/path' - d = config.make_service_dict('foo', {'volumes': ['${VOLUME_PATH}:/container/path']}, working_dir='.') + d = make_service_dict('foo', {'volumes': ['${VOLUME_PATH}:/container/path']}, working_dir='.') self.assertEqual(d['volumes'], ['/host/path:/container/path']) @mock.patch.dict(os.environ) def test_volume_binding_with_home(self): os.environ['HOME'] = '/home/user' - d = config.make_service_dict('foo', {'volumes': ['~:/container/path']}, working_dir='.') + d = make_service_dict('foo', {'volumes': ['~:/container/path']}, working_dir='.') self.assertEqual(d['volumes'], ['/home/user:/container/path']) @@ -219,36 +226,36 @@ class MergeLabelsTest(unittest.TestCase): def test_no_override(self): service_dict = config.merge_service_dicts( - config.make_service_dict('foo', {'labels': ['foo=1', 'bar']}), - config.make_service_dict('foo', {}), + make_service_dict('foo', {'labels': ['foo=1', 'bar']}, 'tests/'), + make_service_dict('foo', {}, 'tests/'), ) self.assertEqual(service_dict['labels'], {'foo': '1', 'bar': ''}) def test_no_base(self): service_dict = config.merge_service_dicts( - config.make_service_dict('foo', {}), - config.make_service_dict('foo', {'labels': ['foo=2']}), + make_service_dict('foo', {}, 'tests/'), + make_service_dict('foo', {'labels': ['foo=2']}, 'tests/'), ) self.assertEqual(service_dict['labels'], {'foo': '2'}) def test_override_explicit_value(self): service_dict = config.merge_service_dicts( - config.make_service_dict('foo', {'labels': ['foo=1', 'bar']}), - config.make_service_dict('foo', {'labels': ['foo=2']}), + make_service_dict('foo', {'labels': ['foo=1', 'bar']}, 'tests/'), + make_service_dict('foo', {'labels': ['foo=2']}, 'tests/'), ) self.assertEqual(service_dict['labels'], {'foo': '2', 'bar': ''}) def test_add_explicit_value(self): service_dict = config.merge_service_dicts( - config.make_service_dict('foo', {'labels': ['foo=1', 'bar']}), - config.make_service_dict('foo', {'labels': ['bar=2']}), + make_service_dict('foo', {'labels': ['foo=1', 'bar']}, 'tests/'), + make_service_dict('foo', {'labels': ['bar=2']}, 'tests/'), ) self.assertEqual(service_dict['labels'], {'foo': '1', 'bar': '2'}) def test_remove_explicit_value(self): service_dict = config.merge_service_dicts( - config.make_service_dict('foo', {'labels': ['foo=1', 'bar=2']}), - config.make_service_dict('foo', {'labels': ['bar']}), + make_service_dict('foo', {'labels': ['foo=1', 'bar=2']}, 'tests/'), + make_service_dict('foo', {'labels': ['bar']}, 'tests/'), ) self.assertEqual(service_dict['labels'], {'foo': '1', 'bar': ''}) @@ -286,7 +293,7 @@ class EnvTest(unittest.TestCase): os.environ['FILE_DEF_EMPTY'] = 'E2' os.environ['ENV_DEF'] = 'E3' - service_dict = config.make_service_dict( + service_dict = make_service_dict( 'foo', { 'environment': { 'FILE_DEF': 'F1', @@ -295,6 +302,7 @@ class EnvTest(unittest.TestCase): 'NO_DEF': None }, }, + 'tests/' ) self.assertEqual( @@ -303,7 +311,7 @@ class EnvTest(unittest.TestCase): ) def test_env_from_file(self): - service_dict = config.make_service_dict( + service_dict = make_service_dict( 'foo', {'env_file': 'one.env'}, 'tests/fixtures/env', @@ -314,7 +322,7 @@ class EnvTest(unittest.TestCase): ) def test_env_from_multiple_files(self): - service_dict = config.make_service_dict( + service_dict = make_service_dict( 'foo', {'env_file': ['one.env', 'two.env']}, 'tests/fixtures/env', @@ -328,7 +336,7 @@ class EnvTest(unittest.TestCase): options = {'env_file': 'nonexistent.env'} self.assertRaises( config.ConfigurationError, - lambda: config.make_service_dict('foo', options, 'tests/fixtures/env'), + lambda: make_service_dict('foo', options, 'tests/fixtures/env'), ) @mock.patch.dict(os.environ) @@ -336,7 +344,7 @@ class EnvTest(unittest.TestCase): os.environ['FILE_DEF'] = 'E1' os.environ['FILE_DEF_EMPTY'] = 'E2' os.environ['ENV_DEF'] = 'E3' - service_dict = config.make_service_dict( + service_dict = make_service_dict( 'foo', {'env_file': 'resolve.env'}, 'tests/fixtures/env', @@ -351,14 +359,14 @@ class EnvTest(unittest.TestCase): os.environ['HOSTENV'] = '/tmp' os.environ['CONTAINERENV'] = '/host/tmp' - service_dict = config.make_service_dict( + service_dict = make_service_dict( 'foo', {'volumes': ['$HOSTENV:$CONTAINERENV']}, working_dir="tests/fixtures/env" ) self.assertEqual(set(service_dict['volumes']), set(['/tmp:/host/tmp'])) - service_dict = config.make_service_dict( + service_dict = make_service_dict( 'foo', {'volumes': ['/opt${HOSTENV}:/opt${CONTAINERENV}']}, working_dir="tests/fixtures/env" @@ -413,6 +421,33 @@ class ExtendsTest(unittest.TestCase): }, ]) + def test_self_referencing_file(self): + """ + We specify a 'file' key that is the filename we're already in. + """ + service_dicts = load_from_filename('tests/fixtures/extends/specify-file-as-self.yml') + self.assertEqual(service_dicts, [ + { + 'environment': + { + 'YEP': '1', 'BAR': '1', 'BAZ': '3' + }, + 'image': 'busybox', + 'name': 'myweb' + }, + { + 'environment': + {'YEP': '1'}, + 'name': 'otherweb' + }, + { + 'environment': + {'YEP': '1', 'BAZ': '3'}, + 'image': 'busybox', + 'name': 'web' + } + ]) + def test_circular(self): try: load_from_filename('tests/fixtures/extends/circle-1.yml') @@ -427,29 +462,81 @@ class ExtendsTest(unittest.TestCase): ], ) - def test_extends_validation(self): + def test_extends_validation_empty_dictionary(self): dictionary = {'extends': None} def load_config(): - return config.make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends') + return make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends') self.assertRaisesRegexp(config.ConfigurationError, 'dictionary', load_config) dictionary['extends'] = {} self.assertRaises(config.ConfigurationError, load_config) - dictionary['extends']['file'] = 'common.yml' + def test_extends_validation_missing_service_key(self): + dictionary = {'extends': {'file': 'common.yml'}} + + def load_config(): + return make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends') + self.assertRaisesRegexp(config.ConfigurationError, 'service', load_config) - dictionary['extends']['service'] = 'web' + def test_extends_validation_invalid_key(self): + dictionary = { + 'extends': + { + 'service': 'web', 'file': 'common.yml', 'what': 'is this' + } + } + + def load_config(): + return make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends') + + self.assertRaisesRegexp(config.ConfigurationError, 'what', load_config) + + def test_extends_validation_no_file_key_no_filename_set(self): + dictionary = {'extends': {'service': 'web'}} + + def load_config(): + return make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends') + + self.assertRaisesRegexp(config.ConfigurationError, 'file', load_config) + + def test_extends_validation_valid_config(self): + dictionary = {'extends': {'service': 'web', 'file': 'common.yml'}} + + def load_config(): + return make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends') + self.assertIsInstance(load_config(), dict) - dictionary['extends']['what'] = 'is this' - self.assertRaisesRegexp(config.ConfigurationError, 'what', load_config) + def test_extends_file_defaults_to_self(self): + """ + Test not specifying a file in our extends options that the + config is valid and correctly extends from itself. + """ + service_dicts = load_from_filename('tests/fixtures/extends/no-file-specified.yml') + self.assertEqual(service_dicts, [ + { + 'name': 'myweb', + 'image': 'busybox', + 'environment': { + "BAR": "1", + "BAZ": "3", + } + }, + { + 'name': 'web', + 'image': 'busybox', + 'environment': { + "BAZ": "3", + } + } + ]) def test_blacklisted_options(self): def load_config(): - return config.make_service_dict('myweb', { + return make_service_dict('myweb', { 'extends': { 'file': 'whatever', 'service': 'web', @@ -523,7 +610,7 @@ class BuildPathTest(unittest.TestCase): def test_relative_path(self): relative_build_path = '../build-ctx/' - service_dict = config.make_service_dict( + service_dict = make_service_dict( 'relpath', {'build': relative_build_path}, working_dir='tests/fixtures/build-path' @@ -531,7 +618,7 @@ class BuildPathTest(unittest.TestCase): self.assertEquals(service_dict['build'], self.abs_context_path) def test_absolute_path(self): - service_dict = config.make_service_dict( + service_dict = make_service_dict( 'abspath', {'build': self.abs_context_path}, working_dir='tests/fixtures/build-path'