From 92ef1f57022008d0bb5ed47971bccb83ed07afa4 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Tue, 28 Jul 2015 16:48:38 +0100 Subject: [PATCH 1/4] Make compose.config a proper module Signed-off-by: Aanand Prasad --- compose/config/__init__.py | 10 ++++++++++ compose/{ => config}/config.py | 0 tests/integration/testcases.py | 2 +- tests/unit/config_test.py | 2 +- 4 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 compose/config/__init__.py rename compose/{ => config}/config.py (100%) diff --git a/compose/config/__init__.py b/compose/config/__init__.py new file mode 100644 index 0000000000..3907e5b67e --- /dev/null +++ b/compose/config/__init__.py @@ -0,0 +1,10 @@ +from .config import ( + DOCKER_CONFIG_KEYS, + ConfigDetails, + ConfigurationError, + find, + load, + parse_environment, + merge_environment, + get_service_name_from_net, +) # flake8: noqa diff --git a/compose/config.py b/compose/config/config.py similarity index 100% rename from compose/config.py rename to compose/config/config.py diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index 2a7c0a440d..a7929088be 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 ServiceLoader +from compose.config.config import ServiceLoader from compose.const import LABEL_PROJECT from compose.cli.docker_client import docker_client from compose.progress_stream import stream_output diff --git a/tests/unit/config_test.py b/tests/unit/config_test.py index a2c17d7254..3ee754e319 100644 --- a/tests/unit/config_test.py +++ b/tests/unit/config_test.py @@ -4,7 +4,7 @@ import tempfile from .. import unittest -from compose import config +from compose.config import config def make_service_dict(name, service_dict, working_dir): From 31ac3ce22a381061b13046a4231161ed5c1a9eb3 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Tue, 28 Jul 2015 18:05:39 +0100 Subject: [PATCH 2/4] Split out compose.config.errors Signed-off-by: Aanand Prasad --- compose/config/config.py | 36 ++++++------------------------------ compose/config/errors.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 30 deletions(-) create mode 100644 compose/config/errors.py diff --git a/compose/config/config.py b/compose/config/config.py index af8983961f..d36967825c 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -8,6 +8,12 @@ from compose.cli.utils import find_candidates_in_parent_dirs +from .errors import ( + ConfigurationError, + CircularReference, + ComposeFileNotFound, +) + DOCKER_CONFIG_KEYS = [ 'cap_add', @@ -536,33 +542,3 @@ def load_yaml(filename): return yaml.safe_load(fh) except IOError as e: raise ConfigurationError(six.text_type(e)) - - -class ConfigurationError(Exception): - def __init__(self, msg): - self.msg = msg - - def __str__(self): - return self.msg - - -class CircularReference(ConfigurationError): - def __init__(self, trail): - self.trail = trail - - @property - def msg(self): - lines = [ - "{} in {}".format(service_name, filename) - for (filename, service_name) in self.trail - ] - return "Circular reference:\n {}".format("\n extends ".join(lines)) - - -class ComposeFileNotFound(ConfigurationError): - def __init__(self, supported_filenames): - super(ComposeFileNotFound, self).__init__(""" - Can't find a suitable configuration file in this directory or any parent. Are you in the right directory? - - Supported filenames: %s - """ % ", ".join(supported_filenames)) diff --git a/compose/config/errors.py b/compose/config/errors.py new file mode 100644 index 0000000000..037b7ec84d --- /dev/null +++ b/compose/config/errors.py @@ -0,0 +1,28 @@ +class ConfigurationError(Exception): + def __init__(self, msg): + self.msg = msg + + def __str__(self): + return self.msg + + +class CircularReference(ConfigurationError): + def __init__(self, trail): + self.trail = trail + + @property + def msg(self): + lines = [ + "{} in {}".format(service_name, filename) + for (filename, service_name) in self.trail + ] + return "Circular reference:\n {}".format("\n extends ".join(lines)) + + +class ComposeFileNotFound(ConfigurationError): + def __init__(self, supported_filenames): + super(ComposeFileNotFound, self).__init__(""" + Can't find a suitable configuration file in this directory or any parent. Are you in the right directory? + + Supported filenames: %s + """ % ", ".join(supported_filenames)) From 8b5bd945d0883ef71b87ca80e75c57e2636183a9 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Fri, 24 Jul 2015 15:58:18 +0100 Subject: [PATCH 3/4] Interpolate environment variables Signed-off-by: Aanand Prasad --- compose/config/config.py | 9 ++- compose/config/interpolation.py | 69 +++++++++++++++++ docs/yml.md | 31 ++++++++ .../docker-compose.yml | 17 +++++ .../docker-compose.yml | 5 ++ tests/integration/cli_test.py | 15 ++++ tests/integration/service_test.py | 18 ----- tests/unit/config_test.py | 75 +++++++++++++++---- tests/unit/interpolation_test.py | 31 ++++++++ 9 files changed, 235 insertions(+), 35 deletions(-) create mode 100644 compose/config/interpolation.py create mode 100644 tests/fixtures/environment-interpolation/docker-compose.yml create mode 100644 tests/fixtures/volume-path-interpolation/docker-compose.yml create mode 100644 tests/unit/interpolation_test.py diff --git a/compose/config/config.py b/compose/config/config.py index d36967825c..4d3f5faefa 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -8,6 +8,7 @@ from compose.cli.utils import find_candidates_in_parent_dirs +from .interpolation import interpolate_environment_variables from .errors import ( ConfigurationError, CircularReference, @@ -132,11 +133,11 @@ def get_config_path(base_dir): def load(config_details): dictionary, working_dir, filename = config_details + dictionary = interpolate_environment_variables(dictionary) + service_dicts = [] for service_name, service_dict in list(dictionary.items()): - 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) loader = ServiceLoader(working_dir=working_dir, filename=filename) service_dict = loader.make_service_dict(service_name, service_dict) validate_paths(service_dict) @@ -429,9 +430,9 @@ def resolve_volume_paths(volumes, working_dir=None): def resolve_volume_path(volume, working_dir): container_path, host_path = split_path_mapping(volume) - container_path = os.path.expanduser(os.path.expandvars(container_path)) + container_path = os.path.expanduser(container_path) if host_path is not None: - host_path = os.path.expanduser(os.path.expandvars(host_path)) + host_path = os.path.expanduser(host_path) return "%s:%s" % (expand_path(working_dir, host_path), container_path) else: return container_path diff --git a/compose/config/interpolation.py b/compose/config/interpolation.py new file mode 100644 index 0000000000..0d4b96419c --- /dev/null +++ b/compose/config/interpolation.py @@ -0,0 +1,69 @@ +import os +from string import Template +from collections import defaultdict + +import six + +from .errors import ConfigurationError + + +def interpolate_environment_variables(config): + return dict( + (service_name, process_service(service_name, service_dict)) + for (service_name, service_dict) in config.items() + ) + + +def process_service(service_name, service_dict): + 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)) + for (key, val) in service_dict.items() + ) + + +def interpolate_value(service_name, config_key, value): + try: + return recursive_interpolate(value) + except InvalidInterpolation as e: + raise ConfigurationError( + 'Invalid interpolation format for "{config_key}" option ' + 'in service "{service_name}": "{string}"' + .format( + config_key=config_key, + service_name=service_name, + string=e.string, + ) + ) + + +def recursive_interpolate(obj): + if isinstance(obj, six.string_types): + return interpolate(obj, os.environ) + elif isinstance(obj, dict): + return dict( + (key, recursive_interpolate(val)) + for (key, val) in obj.items() + ) + elif isinstance(obj, list): + return map(recursive_interpolate, obj) + else: + return obj + + +def interpolate(string, mapping): + try: + return Template(string).substitute(defaultdict(lambda: "", mapping)) + except ValueError: + raise InvalidInterpolation(string) + + +class InvalidInterpolation(Exception): + def __init__(self, string): + self.string = string diff --git a/docs/yml.md b/docs/yml.md index f89d107bdc..18551bf22f 100644 --- a/docs/yml.md +++ b/docs/yml.md @@ -19,6 +19,10 @@ As with `docker run`, options specified in the Dockerfile (e.g., `CMD`, `EXPOSE`, `VOLUME`, `ENV`) are respected by default - you don't need to specify them again in `docker-compose.yml`. +Values for configuration options can contain environment variables, e.g. +`image: postgres:${POSTGRES_VERSION}`. For more details, see the section on +[variable substitution](#variable-substitution). + ### image Tag or partial image ID. Can be local or remote - Compose will attempt to @@ -369,6 +373,33 @@ Each of these is a single value, analogous to its volume_driver: mydriver ``` +## Variable substitution + +Your configuration options can contain environment variables. Compose uses the +variable values from the shell environment in which `docker-compose` is run. For +example, suppose the shell contains `POSTGRES_VERSION=9.3` and you supply this +configuration: + + db: + image: "postgres:${POSTGRES_VERSION}" + +When you run `docker-compose up` with this configuration, Compose looks for the +`POSTGRES_VERSION` environment variable in the shell and substitutes its value +in. For this example, Compose resolves the `image` to `postgres:9.3` before +running the configuration. + +If an environment variable is not set, Compose substitutes with an empty +string. In the example above, if `POSTGRES_VERSION` is not set, the value for +the `image` option is `postgres:`. + +Both `$VARIABLE` and `${VARIABLE}` syntax are supported. Extended shell-style +features, such as `${VARIABLE-default}` and `${VARIABLE/foo/bar}`, are not +supported. + +If you need to put a literal dollar sign in a configuration value, use a double +dollar sign (`$$`). + + ## Compose documentation - [User guide](/) diff --git a/tests/fixtures/environment-interpolation/docker-compose.yml b/tests/fixtures/environment-interpolation/docker-compose.yml new file mode 100644 index 0000000000..7ed43a812c --- /dev/null +++ b/tests/fixtures/environment-interpolation/docker-compose.yml @@ -0,0 +1,17 @@ +web: + # unbracketed name + image: $IMAGE + + # array element + ports: + - "${HOST_PORT}:8000" + + # dictionary item value + labels: + mylabel: "${LABEL_VALUE}" + + # unset value + hostname: "host-${UNSET_VALUE}" + + # escaped interpolation + command: "$${ESCAPED}" diff --git a/tests/fixtures/volume-path-interpolation/docker-compose.yml b/tests/fixtures/volume-path-interpolation/docker-compose.yml new file mode 100644 index 0000000000..6d4e236af9 --- /dev/null +++ b/tests/fixtures/volume-path-interpolation/docker-compose.yml @@ -0,0 +1,5 @@ +test: + image: busybox + command: top + volumes: + - "~/${VOLUME_NAME}:/container-path" diff --git a/tests/integration/cli_test.py b/tests/integration/cli_test.py index f3b3b9f5fb..0e86c2792f 100644 --- a/tests/integration/cli_test.py +++ b/tests/integration/cli_test.py @@ -488,6 +488,21 @@ def test_env_file_relative_to_compose_file(self): self.assertEqual(len(containers), 1) self.assertIn("FOO=1", containers[0].get('Config.Env')) + @patch.dict(os.environ) + def test_home_and_env_var_in_volume_path(self): + os.environ['VOLUME_NAME'] = 'my-volume' + os.environ['HOME'] = '/tmp/home-dir' + expected_host_path = os.path.join(os.environ['HOME'], os.environ['VOLUME_NAME']) + + self.command.base_dir = 'tests/fixtures/volume-path-interpolation' + self.command.dispatch(['up', '-d'], None) + + container = self.project.containers(stopped=True)[0] + actual_host_path = container.get('Volumes')['/container-path'] + components = actual_host_path.split('/') + self.assertTrue(components[-2:] == ['home-dir', 'my-volume'], + msg="Last two components differ: %s, %s" % (actual_host_path, expected_host_path)) + def test_up_with_extends(self): self.command.base_dir = 'tests/fixtures/extends' self.command.dispatch(['up', '-d'], None) diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 8856d0245f..9bdc12f993 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -273,24 +273,6 @@ def test_duplicate_volume_trailing_slash(self): self.assertEqual(service.containers(stopped=False), [new_container]) - @patch.dict(os.environ) - def test_create_container_with_home_and_env_var_in_volume_path(self): - os.environ['VOLUME_NAME'] = 'my-volume' - os.environ['HOME'] = '/tmp/home-dir' - expected_host_path = os.path.join(os.environ['HOME'], os.environ['VOLUME_NAME']) - - host_path = '~/${VOLUME_NAME}' - container_path = '/container-path' - - service = self.create_service('db', volumes=['%s:%s' % (host_path, container_path)]) - container = service.create_container() - service.start_container(container) - - actual_host_path = container.get('Volumes')[container_path] - components = actual_host_path.split('/') - self.assertTrue(components[-2:] == ['home-dir', 'my-volume'], - msg="Last two components differ: %s, %s" % (actual_host_path, expected_host_path)) - def test_create_container_with_volumes_from(self): volume_service = self.create_service('data') volume_container_1 = volume_service.create_container() diff --git a/tests/unit/config_test.py b/tests/unit/config_test.py index 3ee754e319..b1c22235b9 100644 --- a/tests/unit/config_test.py +++ b/tests/unit/config_test.py @@ -59,11 +59,56 @@ def test_config_validation(self): make_service_dict('foo', {'ports': ['8000']}, 'tests/') -class VolumePathTest(unittest.TestCase): +class InterpolationTest(unittest.TestCase): @mock.patch.dict(os.environ) - def test_volume_binding_with_environ(self): + def test_config_file_with_environment_variable(self): + os.environ.update( + IMAGE="busybox", + HOST_PORT="80", + LABEL_VALUE="myvalue", + ) + + service_dicts = config.load( + config.find('tests/fixtures/environment-interpolation', None), + ) + + self.assertEqual(service_dicts, [ + { + 'name': 'web', + 'image': 'busybox', + 'ports': ['80:8000'], + 'labels': {'mylabel': 'myvalue'}, + 'hostname': 'host-', + 'command': '${ESCAPED}', + } + ]) + + @mock.patch.dict(os.environ) + def test_invalid_interpolation(self): + with self.assertRaises(config.ConfigurationError) as cm: + config.load( + config.ConfigDetails( + {'web': {'image': '${'}}, + 'working_dir', + 'filename.yml' + ) + ) + + self.assertIn('Invalid', cm.exception.msg) + self.assertIn('for "image" option', cm.exception.msg) + self.assertIn('in service "web"', cm.exception.msg) + self.assertIn('"${"', cm.exception.msg) + + @mock.patch.dict(os.environ) + def test_volume_binding_with_environment_variable(self): os.environ['VOLUME_PATH'] = '/host/path' - d = make_service_dict('foo', {'volumes': ['${VOLUME_PATH}:/container/path']}, working_dir='.') + d = config.load( + config.ConfigDetails( + config={'foo': {'volumes': ['${VOLUME_PATH}:/container/path']}}, + working_dir='.', + filename=None, + ) + )[0] self.assertEqual(d['volumes'], ['/host/path:/container/path']) @mock.patch.dict(os.environ) @@ -400,18 +445,22 @@ def test_resolve_path(self): os.environ['HOSTENV'] = '/tmp' os.environ['CONTAINERENV'] = '/host/tmp' - service_dict = make_service_dict( - 'foo', - {'volumes': ['$HOSTENV:$CONTAINERENV']}, - working_dir="tests/fixtures/env" - ) + service_dict = config.load( + config.ConfigDetails( + config={'foo': {'volumes': ['$HOSTENV:$CONTAINERENV']}}, + working_dir="tests/fixtures/env", + filename=None, + ) + )[0] self.assertEqual(set(service_dict['volumes']), set(['/tmp:/host/tmp'])) - service_dict = make_service_dict( - 'foo', - {'volumes': ['/opt${HOSTENV}:/opt${CONTAINERENV}']}, - working_dir="tests/fixtures/env" - ) + service_dict = config.load( + config.ConfigDetails( + config={'foo': {'volumes': ['/opt${HOSTENV}:/opt${CONTAINERENV}']}}, + working_dir="tests/fixtures/env", + filename=None, + ) + )[0] self.assertEqual(set(service_dict['volumes']), set(['/opt/tmp:/opt/host/tmp'])) diff --git a/tests/unit/interpolation_test.py b/tests/unit/interpolation_test.py new file mode 100644 index 0000000000..96c6f9b33a --- /dev/null +++ b/tests/unit/interpolation_test.py @@ -0,0 +1,31 @@ +import unittest + +from compose.config.interpolation import interpolate, InvalidInterpolation + + +class InterpolationTest(unittest.TestCase): + def test_valid_interpolations(self): + self.assertEqual(interpolate('$foo', dict(foo='hi')), 'hi') + self.assertEqual(interpolate('${foo}', dict(foo='hi')), 'hi') + + self.assertEqual(interpolate('${subject} love you', dict(subject='i')), 'i love you') + self.assertEqual(interpolate('i ${verb} you', dict(verb='love')), 'i love you') + self.assertEqual(interpolate('i love ${object}', dict(object='you')), 'i love you') + + def test_empty_value(self): + self.assertEqual(interpolate('${foo}', dict(foo='')), '') + + def test_unset_value(self): + self.assertEqual(interpolate('${foo}', dict()), '') + + def test_escaped_interpolation(self): + self.assertEqual(interpolate('$${foo}', dict(foo='hi')), '${foo}') + + def test_invalid_strings(self): + self.assertRaises(InvalidInterpolation, lambda: interpolate('${', dict())) + self.assertRaises(InvalidInterpolation, lambda: interpolate('$}', dict())) + self.assertRaises(InvalidInterpolation, lambda: interpolate('${}', dict())) + self.assertRaises(InvalidInterpolation, lambda: interpolate('${ }', dict())) + self.assertRaises(InvalidInterpolation, lambda: interpolate('${ foo}', dict())) + self.assertRaises(InvalidInterpolation, lambda: interpolate('${foo }', dict())) + self.assertRaises(InvalidInterpolation, lambda: interpolate('${foo!}', dict())) From ee6ff294a273d07e157af68f0b5f97f36b957676 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 6 Aug 2015 11:31:42 +0100 Subject: [PATCH 4/4] Show a warning when a variable is unset Signed-off-by: Aanand Prasad --- compose/config/interpolation.py | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/compose/config/interpolation.py b/compose/config/interpolation.py index 0d4b96419c..d33e93be49 100644 --- a/compose/config/interpolation.py +++ b/compose/config/interpolation.py @@ -1,11 +1,13 @@ import os from string import Template -from collections import defaultdict import six from .errors import ConfigurationError +import logging +log = logging.getLogger(__name__) + def interpolate_environment_variables(config): return dict( @@ -59,11 +61,26 @@ def recursive_interpolate(obj): def interpolate(string, mapping): try: - return Template(string).substitute(defaultdict(lambda: "", mapping)) + return Template(string).substitute(BlankDefaultDict(mapping)) except ValueError: raise InvalidInterpolation(string) +class BlankDefaultDict(dict): + def __init__(self, mapping): + super(BlankDefaultDict, self).__init__(mapping) + + def __getitem__(self, key): + try: + return super(BlankDefaultDict, self).__getitem__(key) + except KeyError: + log.warn( + "The {} variable is not set. Substituting a blank string." + .format(key) + ) + return "" + + class InvalidInterpolation(Exception): def __init__(self, string): self.string = string