From 528bed9ef6921eeea219370d74ec0e6686e10636 Mon Sep 17 00:00:00 2001 From: Aanand Prasad Date: Thu, 12 Mar 2015 13:59:23 +0000 Subject: [PATCH] Fix environment resolution Signed-off-by: Aanand Prasad --- compose/config.py | 60 ++++++++++++++-------- docs/yml.md | 9 +++- tests/fixtures/env-file/docker-compose.yml | 4 ++ tests/fixtures/env-file/test.env | 1 + tests/integration/cli_test.py | 16 ++++++ tests/integration/testcases.py | 2 +- tests/unit/config_test.py | 18 +++---- 7 files changed, 77 insertions(+), 33 deletions(-) create mode 100644 tests/fixtures/env-file/docker-compose.yml create mode 100644 tests/fixtures/env-file/test.env diff --git a/compose/config.py b/compose/config.py index 4376d97cfa..a4e3a991f7 100644 --- a/compose/config.py +++ b/compose/config.py @@ -51,7 +51,8 @@ DOCKER_CONFIG_HINTS = { def load(filename): - return from_dictionary(load_yaml(filename)) + working_dir = os.path.dirname(filename) + return from_dictionary(load_yaml(filename), working_dir=working_dir) def load_yaml(filename): @@ -62,25 +63,26 @@ def load_yaml(filename): raise ConfigurationError(six.text_type(e)) -def from_dictionary(dictionary): +def from_dictionary(dictionary, working_dir=None): 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) - service_dict = make_service_dict(service_name, service_dict) + service_dict = make_service_dict(service_name, service_dict, working_dir=working_dir) service_dicts.append(service_dict) return service_dicts -def make_service_dict(name, options): +def make_service_dict(name, options, working_dir=None): service_dict = options.copy() service_dict['name'] = name - return process_container_options(service_dict) + service_dict = resolve_environment(service_dict, working_dir=working_dir) + return process_container_options(service_dict, working_dir=working_dir) -def process_container_options(service_dict): +def process_container_options(service_dict, working_dir=None): for k in service_dict: if k not in ALLOWED_KEYS: msg = "Unsupported config option for %s service: '%s'" % (service_dict['name'], k) @@ -88,13 +90,6 @@ def process_container_options(service_dict): msg += " (did you mean '%s'?)" % DOCKER_CONFIG_HINTS[k] raise ConfigurationError(msg) - for filename in get_env_files(service_dict): - if not os.path.exists(filename): - raise ConfigurationError("Couldn't find env file for service %s: %s" % (service_dict['name'], filename)) - - if 'environment' in service_dict or 'env_file' in service_dict: - service_dict['environment'] = build_environment(service_dict) - return service_dict @@ -110,21 +105,38 @@ def parse_link(link): return (link, link) -def get_env_files(options): +def get_env_files(options, working_dir=None): + if 'env_file' not in options: + return {} + + if working_dir is None: + raise Exception("No working_dir passed to get_env_files()") + env_files = options.get('env_file', []) if not isinstance(env_files, list): env_files = [env_files] - return env_files + + return [expand_path(working_dir, path) for path in env_files] -def build_environment(options): +def resolve_environment(service_dict, working_dir=None): + service_dict = service_dict.copy() + + if 'environment' not in service_dict and 'env_file' not in service_dict: + return service_dict + env = {} - for f in get_env_files(options): - env.update(env_vars_from_file(f)) + if 'env_file' in service_dict: + for f in get_env_files(service_dict, working_dir=working_dir): + env.update(env_vars_from_file(f)) + del service_dict['env_file'] - env.update(parse_environment(options.get('environment'))) - return dict(resolve_env(k, v) for k, v in six.iteritems(env)) + env.update(parse_environment(service_dict.get('environment'))) + env = dict(resolve_env_var(k, v) for k, v in six.iteritems(env)) + + service_dict['environment'] = env + return service_dict def parse_environment(environment): @@ -150,7 +162,7 @@ def split_env(env): return env, None -def resolve_env(key, val): +def resolve_env_var(key, val): if val is not None: return key, val elif key in os.environ: @@ -163,6 +175,8 @@ def env_vars_from_file(filename): """ Read in a line delimited file of environment variables. """ + if not os.path.exists(filename): + raise ConfigurationError("Couldn't find env file: %s" % filename) env = {} for line in open(filename, 'r'): line = line.strip() @@ -172,6 +186,10 @@ def env_vars_from_file(filename): return env +def expand_path(working_dir, path): + return os.path.abspath(os.path.join(working_dir, path)) + + class ConfigurationError(Exception): def __init__(self, msg): self.msg = msg diff --git a/docs/yml.md b/docs/yml.md index 035a99e921..52be706a07 100644 --- a/docs/yml.md +++ b/docs/yml.md @@ -158,11 +158,18 @@ environment: Add environment variables from a file. Can be a single value or a list. +If you have specified a Compose file with `docker-compose -f FILE`, paths in +`env_file` are relative to the directory that file is in. + Environment variables specified in `environment` override these values. ``` +env_file: .env + env_file: - - .env + - ./common.env + - ./apps/web.env + - /opt/secrets.env ``` ``` diff --git a/tests/fixtures/env-file/docker-compose.yml b/tests/fixtures/env-file/docker-compose.yml new file mode 100644 index 0000000000..d9366ace23 --- /dev/null +++ b/tests/fixtures/env-file/docker-compose.yml @@ -0,0 +1,4 @@ +web: + image: busybox + command: /bin/true + env_file: ./test.env diff --git a/tests/fixtures/env-file/test.env b/tests/fixtures/env-file/test.env new file mode 100644 index 0000000000..c9604dad5b --- /dev/null +++ b/tests/fixtures/env-file/test.env @@ -0,0 +1 @@ +FOO=1 \ No newline at end of file diff --git a/tests/integration/cli_test.py b/tests/integration/cli_test.py index 7cf19be60c..a79b45cfba 100644 --- a/tests/integration/cli_test.py +++ b/tests/integration/cli_test.py @@ -1,5 +1,6 @@ from __future__ import absolute_import import sys +import os from six import StringIO from mock import patch @@ -23,6 +24,12 @@ class CLITestCase(DockerClientTestCase): @property def project(self): + # Hack: allow project to be overridden. This needs refactoring so that + # the project object is built exactly once, by the command object, and + # accessed by the test case object. + if hasattr(self, '_project'): + return self._project + return self.command.get_project(self.command.get_config_path()) def test_help(self): @@ -409,3 +416,12 @@ class CLITestCase(DockerClientTestCase): self.assertEqual(get_port(3000), container.get_local_port(3000)) self.assertEqual(get_port(3001), "0.0.0.0:9999") self.assertEqual(get_port(3002), "") + + def test_env_file_relative_to_compose_file(self): + config_path = os.path.abspath('tests/fixtures/env-file/docker-compose.yml') + self.command.dispatch(['-f', config_path, 'up', '-d'], None) + self._project = self.command.get_project(config_path) + + containers = self.project.containers(stopped=True) + self.assertEqual(len(containers), 1) + self.assertIn("FOO=1", containers[0].get('Config.Env')) diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index 4f49124cf0..d5ca1debc8 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -30,7 +30,7 @@ class DockerClientTestCase(unittest.TestCase): return Service( project='composetest', client=self.client, - **make_service_dict(name, kwargs) + **make_service_dict(name, kwargs, working_dir='.') ) def check_build(self, *args, **kwargs): diff --git a/tests/unit/config_test.py b/tests/unit/config_test.py index 8f59694de9..4ff08a9eff 100644 --- a/tests/unit/config_test.py +++ b/tests/unit/config_test.py @@ -90,7 +90,8 @@ class ConfigTest(unittest.TestCase): def test_env_from_file(self): service_dict = config.make_service_dict( 'foo', - {'env_file': 'tests/fixtures/env/one.env'}, + {'env_file': 'one.env'}, + 'tests/fixtures/env', ) self.assertEqual( service_dict['environment'], @@ -100,12 +101,8 @@ class ConfigTest(unittest.TestCase): def test_env_from_multiple_files(self): service_dict = config.make_service_dict( 'foo', - { - 'env_file': [ - 'tests/fixtures/env/one.env', - 'tests/fixtures/env/two.env', - ], - }, + {'env_file': ['one.env', 'two.env']}, + 'tests/fixtures/env', ) self.assertEqual( service_dict['environment'], @@ -113,10 +110,10 @@ class ConfigTest(unittest.TestCase): ) def test_env_nonexistent_file(self): - options = {'env_file': 'tests/fixtures/env/nonexistent.env'} + options = {'env_file': 'nonexistent.env'} self.assertRaises( config.ConfigurationError, - lambda: config.make_service_dict('foo', options), + lambda: config.make_service_dict('foo', options, 'tests/fixtures/env'), ) @mock.patch.dict(os.environ) @@ -126,7 +123,8 @@ class ConfigTest(unittest.TestCase): os.environ['ENV_DEF'] = 'E3' service_dict = config.make_service_dict( 'foo', - {'env_file': 'tests/fixtures/env/resolve.env'}, + {'env_file': 'resolve.env'}, + 'tests/fixtures/env', ) self.assertEqual( service_dict['environment'],