From e3e8a619cce64a127df7d7962a2694116914b566 Mon Sep 17 00:00:00 2001 From: Andrey Devyatkin Date: Fri, 27 May 2016 07:48:13 +0200 Subject: [PATCH 1/3] 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 8b9f64f0f..64a464536 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 a259c476f..ae2cab208 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): From c148849f0e219ff61a7a29164fd88c113faf7ef3 Mon Sep 17 00:00:00 2001 From: Andrey Devyatkin Date: Fri, 27 May 2016 19:59:27 +0200 Subject: [PATCH 2/3] 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): From a67ba5536db72203b22fc989b91f54f598e1d1f9 Mon Sep 17 00:00:00 2001 From: Andrey Devyatkin Date: Sat, 28 May 2016 11:39:41 +0200 Subject: [PATCH 3/3] 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/utils.py | 2 +- tests/unit/config/config_test.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/compose/utils.py b/compose/utils.py index 1e01fcb62..925a8e791 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 if v else '')) for k, v in source_dict.items()) + return dict((k, str(v if v is not None 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 3e5a7face..0abb8daea 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -717,6 +717,34 @@ class ConfigTest(unittest.TestCase): assert 'foo' in service['build']['args'] assert service['build']['args']['foo'] == '' + # If build argument is None then it will be converted to the empty + # string. Make sure that int zero kept as it is, i.e. not converted to + # the empty string + def test_build_args_check_zero_preserved(self): + service = config.load( + build_config_details( + { + 'version': '2', + 'services': { + 'web': { + 'build': { + 'context': '.', + 'dockerfile': 'Dockerfile-alt', + 'args': { + 'foo': 0 + } + } + } + } + }, + 'tests/fixtures/extends', + 'filename.yml' + ) + ).services[0] + assert 'args' in service['build'] + assert 'foo' in service['build']['args'] + assert service['build']['args']['foo'] == '0' + def test_load_with_multiple_files_mismatched_networks_format(self): base_file = config.ConfigFile( 'base.yaml',