From e67bc2569ccc3c811d16cea284eca1f65a227c33 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 23 Nov 2015 13:24:57 -0500 Subject: [PATCH] Properly resolve environment from all sources. Split env resolving into two phases. The first phase is to expand the paths of env_files, which is done before merging extends. Once all files are merged together, the final phase is to read the env_files and use them as the base for environment variables. Signed-off-by: Daniel Nephin --- compose/config/config.py | 34 +++++------- tests/integration/testcases.py | 5 +- tests/unit/config/config_test.py | 94 +++++++++++++++++--------------- 3 files changed, 64 insertions(+), 69 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 9d438ca12..369143311 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -324,16 +324,13 @@ class ServiceExtendsResolver(object): return filename -def resolve_environment(service_config): +def resolve_environment(service_dict): """Unpack any environment variables from an env_file, if set. Interpolate environment values if set. """ - service_dict = service_config.config - env = {} - if 'env_file' in service_dict: - for env_file in get_env_files(service_config.working_dir, service_dict): - env.update(env_vars_from_file(env_file)) + for env_file in service_dict.get('env_file', []): + env.update(env_vars_from_file(env_file)) env.update(parse_environment(service_dict.get('environment'))) return dict(resolve_env_var(k, v) for k, v in six.iteritems(env)) @@ -370,9 +367,11 @@ def process_service(service_config): working_dir = service_config.working_dir service_dict = dict(service_config.config) - if 'environment' in service_dict or 'env_file' in service_dict: - service_dict['environment'] = resolve_environment(service_config) - service_dict.pop('env_file', None) + if 'env_file' in service_dict: + service_dict['env_file'] = [ + expand_path(working_dir, path) + for path in to_list(service_dict['env_file']) + ] if 'volumes' in service_dict and service_dict.get('volume_driver') is None: service_dict['volumes'] = resolve_volume_paths(working_dir, service_dict) @@ -396,6 +395,10 @@ def process_service(service_config): def finalize_service(service_config): service_dict = dict(service_config.config) + if 'environment' in service_dict or 'env_file' in service_dict: + service_dict['environment'] = resolve_environment(service_dict) + service_dict.pop('env_file', None) + if 'volumes_from' in service_dict: service_dict['volumes_from'] = [ VolumeFromSpec.parse(vf) for vf in service_dict['volumes_from']] @@ -440,7 +443,7 @@ def merge_service_dicts(base, override): for field in ['ports', 'expose', 'external_links']: merge_field(field, operator.add, default=[]) - for field in ['dns', 'dns_search']: + for field in ['dns', 'dns_search', 'env_file']: merge_field(field, merge_list_or_string) already_merged_keys = set(d) | {'image', 'build'} @@ -468,17 +471,6 @@ def merge_environment(base, override): return env -def get_env_files(working_dir, options): - if 'env_file' not in options: - return {} - - env_files = options.get('env_file', []) - if not isinstance(env_files, list): - env_files = [env_files] - - return [expand_path(working_dir, path) for path in env_files] - - def parse_environment(environment): if not environment: return {} diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index 334693f70..9ea68e39c 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -7,7 +7,6 @@ from pytest import skip from .. import unittest from compose.cli.docker_client import docker_client from compose.config.config import resolve_environment -from compose.config.config import ServiceConfig from compose.const import LABEL_PROJECT from compose.progress_stream import stream_output from compose.service import Service @@ -39,9 +38,7 @@ class DockerClientTestCase(unittest.TestCase): if 'command' not in kwargs: kwargs['command'] = ["top"] - service_config = ServiceConfig('.', None, name, kwargs) - kwargs['environment'] = resolve_environment(service_config) - + kwargs['environment'] = resolve_environment(kwargs) labels = dict(kwargs.setdefault('labels', {})) labels['com.docker.compose.test-name'] = self.id() diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index a5eeb64f9..2cd26e8f7 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -10,6 +10,7 @@ import py import pytest from compose.config import config +from compose.config.config import resolve_environment from compose.config.errors import ConfigurationError from compose.config.types import VolumeSpec from compose.const import IS_WINDOWS_PLATFORM @@ -973,65 +974,54 @@ class EnvTest(unittest.TestCase): os.environ['FILE_DEF_EMPTY'] = 'E2' os.environ['ENV_DEF'] = 'E3' - service_dict = make_service_dict( - 'foo', { - 'build': '.', - 'environment': { - 'FILE_DEF': 'F1', - 'FILE_DEF_EMPTY': '', - 'ENV_DEF': None, - 'NO_DEF': None - }, + service_dict = { + 'build': '.', + 'environment': { + 'FILE_DEF': 'F1', + 'FILE_DEF_EMPTY': '', + 'ENV_DEF': None, + 'NO_DEF': None }, - 'tests/' - ) - + } self.assertEqual( - service_dict['environment'], + resolve_environment(service_dict), {'FILE_DEF': 'F1', 'FILE_DEF_EMPTY': '', 'ENV_DEF': 'E3', 'NO_DEF': ''}, ) - def test_env_from_file(self): - service_dict = make_service_dict( - 'foo', - {'build': '.', 'env_file': 'one.env'}, - 'tests/fixtures/env', - ) + def test_resolve_environment_from_env_file(self): self.assertEqual( - service_dict['environment'], + resolve_environment({'env_file': ['tests/fixtures/env/one.env']}), {'ONE': '2', 'TWO': '1', 'THREE': '3', 'FOO': 'bar'}, ) - def test_env_from_multiple_files(self): - service_dict = make_service_dict( - 'foo', - {'build': '.', 'env_file': ['one.env', 'two.env']}, - 'tests/fixtures/env', - ) + def test_resolve_environment_with_multiple_env_files(self): + service_dict = { + 'env_file': [ + 'tests/fixtures/env/one.env', + 'tests/fixtures/env/two.env' + ] + } self.assertEqual( - service_dict['environment'], + resolve_environment(service_dict), {'ONE': '2', 'TWO': '1', 'THREE': '3', 'FOO': 'baz', 'DOO': 'dah'}, ) - def test_env_nonexistent_file(self): - options = {'env_file': 'nonexistent.env'} - self.assertRaises( - ConfigurationError, - lambda: make_service_dict('foo', options, 'tests/fixtures/env'), - ) + def test_resolve_environment_nonexistent_file(self): + with pytest.raises(ConfigurationError) as exc: + config.load(build_config_details( + {'foo': {'image': 'example', 'env_file': 'nonexistent.env'}}, + working_dir='tests/fixtures/env')) + + assert 'Couldn\'t find env file' in exc.exconly() + assert 'nonexistent.env' in exc.exconly() @mock.patch.dict(os.environ) - def test_resolve_environment_from_file(self): + def test_resolve_environment_from_env_file_with_empty_values(self): os.environ['FILE_DEF'] = 'E1' os.environ['FILE_DEF_EMPTY'] = 'E2' os.environ['ENV_DEF'] = 'E3' - service_dict = make_service_dict( - 'foo', - {'build': '.', 'env_file': 'resolve.env'}, - 'tests/fixtures/env', - ) self.assertEqual( - service_dict['environment'], + resolve_environment({'env_file': ['tests/fixtures/env/resolve.env']}), { 'FILE_DEF': u'bär', 'FILE_DEF_EMPTY': '', @@ -1378,6 +1368,8 @@ class ExtendsTest(unittest.TestCase): - 'envs' environment: - SECRET + - TEST_ONE=common + - TEST_TWO=common """) tmpdir.join('docker-compose.yml').write(""" ext: @@ -1388,12 +1380,20 @@ class ExtendsTest(unittest.TestCase): - 'envs' environment: - THING + - TEST_ONE=top """) commondir.join('envs').write(""" - COMMON_ENV_FILE=1 + COMMON_ENV_FILE + TEST_ONE=common-env-file + TEST_TWO=common-env-file + TEST_THREE=common-env-file + TEST_FOUR=common-env-file """) tmpdir.join('envs').write(""" - FROM_ENV_FILE=1 + TOP_ENV_FILE + TEST_ONE=top-env-file + TEST_TWO=top-env-file + TEST_THREE=top-env-file """) expected = [ @@ -1402,15 +1402,21 @@ class ExtendsTest(unittest.TestCase): 'image': 'example/app', 'environment': { 'SECRET': 'secret', - 'FROM_ENV_FILE': '1', - 'COMMON_ENV_FILE': '1', + 'TOP_ENV_FILE': 'secret', + 'COMMON_ENV_FILE': 'secret', 'THING': 'thing', + 'TEST_ONE': 'top', + 'TEST_TWO': 'common', + 'TEST_THREE': 'top-env-file', + 'TEST_FOUR': 'common-env-file', }, }, ] with mock.patch.dict(os.environ): os.environ['SECRET'] = 'secret' os.environ['THING'] = 'thing' + os.environ['COMMON_ENV_FILE'] = 'secret' + os.environ['TOP_ENV_FILE'] = 'secret' config = load_from_filename(str(tmpdir.join('docker-compose.yml'))) assert config == expected