From e3e8a619cce64a127df7d7962a2694116914b566 Mon Sep 17 00:00:00 2001 From: Andrey Devyatkin Date: Fri, 27 May 2016 07:48:13 +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 build args will not passed to docker engine if they are equal to string None Signed-off-by: Andrey Devyatkin --- compose/service.py | 8 +++++++- tests/unit/service_test.py | 30 ++++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/compose/service.py b/compose/service.py index 8b9f64f0..64a46453 100644 --- a/compose/service.py +++ b/compose/service.py @@ -701,6 +701,12 @@ 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: @@ -715,7 +721,7 @@ class Service(object): pull=pull, nocache=no_cache, dockerfile=build_opts.get('dockerfile', None), - buildargs=build_opts.get('args', None), + buildargs=buildargs, ) try: diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index a259c476..ae2cab20 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=None, + buildargs={}, ) def test_ensure_image_exists_no_build(self): @@ -481,7 +481,33 @@ class ServiceTest(unittest.TestCase): forcerm=False, nocache=False, rm=True, - buildargs=None, + 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'}, ) def test_build_does_not_pull(self):