From c148849f0e219ff61a7a29164fd88c113faf7ef3 Mon Sep 17 00:00:00 2001 From: Andrey Devyatkin Date: Fri, 27 May 2016 19:59:27 +0200 Subject: [PATCH] Fix #3281: Unexpected result when using build args with default values Fix the issue when build arg is set to None instead of empty string. Usecase: cat docker-compose.yml .... args: - http_proxy - https_proxy - no_proxy If http_proxy, https_proxy, no_proxy environment variables are not defined then http_proxy, https_proxy, no_proxy build args will be set to string None which breaks all downloads With this change undefined build args will be set to empty string instead of string None Signed-off-by: Andrey Devyatkin --- compose/service.py | 8 +------- compose/utils.py | 2 +- tests/unit/config/config_test.py | 2 +- tests/unit/service_test.py | 30 ++---------------------------- 4 files changed, 5 insertions(+), 37 deletions(-) diff --git a/compose/service.py b/compose/service.py index 64a464536..8b9f64f0f 100644 --- a/compose/service.py +++ b/compose/service.py @@ -701,12 +701,6 @@ class Service(object): build_opts = self.options.get('build', {}) path = build_opts.get('context') - # If build argument is not defined and there is no environment variable - # with the same name then build argument value will be None - # Moreover it will be sent to the docker engine as None and then - # interpreted as string None which in many cases will fail the build - # That is why we filter out all pairs with value equal to None - buildargs = {k: v for k, v in build_opts.get('args', {}).items() if v != 'None'} # python2 os.path() doesn't support unicode, so we need to encode it to # a byte string if not six.PY3: @@ -721,7 +715,7 @@ class Service(object): pull=pull, nocache=no_cache, dockerfile=build_opts.get('dockerfile', None), - buildargs=buildargs, + buildargs=build_opts.get('args', None), ) try: diff --git a/compose/utils.py b/compose/utils.py index 494beea34..1e01fcb62 100644 --- a/compose/utils.py +++ b/compose/utils.py @@ -95,4 +95,4 @@ def microseconds_from_time_nano(time_nano): def build_string_dict(source_dict): - return dict((k, str(v)) for k, v in source_dict.items()) + return dict((k, str(v if v else '')) for k, v in source_dict.items()) diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 24ece4994..3e5a7face 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -715,7 +715,7 @@ class ConfigTest(unittest.TestCase): ).services[0] assert 'args' in service['build'] assert 'foo' in service['build']['args'] - assert service['build']['args']['foo'] == 'None' + assert service['build']['args']['foo'] == '' def test_load_with_multiple_files_mismatched_networks_format(self): base_file = config.ConfigFile( diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index ae2cab208..a259c476f 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -445,7 +445,7 @@ class ServiceTest(unittest.TestCase): forcerm=False, nocache=False, rm=True, - buildargs={}, + buildargs=None, ) def test_ensure_image_exists_no_build(self): @@ -481,33 +481,7 @@ class ServiceTest(unittest.TestCase): forcerm=False, nocache=False, rm=True, - buildargs={}, - ) - - def test_ensure_filter_out_empty_build_args(self): - args = {u'no_proxy': 'None', u'https_proxy': 'something'} - service = Service('foo', - client=self.mock_client, - build={'context': '.', 'args': args}) - self.mock_client.inspect_image.return_value = {'Id': 'abc123'} - self.mock_client.build.return_value = [ - '{"stream": "Successfully built abcd"}', - ] - - with mock.patch('compose.service.log', autospec=True) as mock_log: - service.ensure_image_exists(do_build=BuildAction.force) - - assert not mock_log.warn.called - self.mock_client.build.assert_called_once_with( - tag='default_foo', - dockerfile=None, - stream=True, - path='.', - pull=False, - forcerm=False, - nocache=False, - rm=True, - buildargs={u'https_proxy': 'something'}, + buildargs=None, ) def test_build_does_not_pull(self):