Fix environment resolution

Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>

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):
+    if 'env_file' in service_dict:
-        env.update(env_vars_from_file(f))
+        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')))
+    env.update(parse_environment(service_dict.get('environment')))
-    return dict(resolve_env(k, v) for k, v in six.iteritems(env))
+    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

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
 ```
 
 ```

tests/fixtures/env-file/docker-compose.yml

@@ -0,0 +1,4 @@
+web:
+  image: busybox
+  command: /bin/true
+  env_file: ./test.env

tests/fixtures/env-file/test.env

@@ -0,0 +1 @@
+FOO=1

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'))

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):

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': ['one.env', 'two.env']},
-                'env_file': [
+            'tests/fixtures/env',
-                    'tests/fixtures/env/one.env',
-                    'tests/fixtures/env/two.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'],