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 8b9f64f0f93..64a464536bd 100644 --- a/compose/service.py +++ b/compose/service.py @@ -701,6 +701,12 @@ def build(self, no_cache=False, pull=False, force_rm=False): 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 @@ def build(self, no_cache=False, pull=False, force_rm=False): 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 a259c476fb5..ae2cab20838 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -445,7 +445,7 @@ def test_create_container(self): forcerm=False, nocache=False, rm=True, - buildargs=None, + buildargs={}, ) def test_ensure_image_exists_no_build(self): @@ -481,7 +481,33 @@ def test_ensure_image_exists_force_build(self): 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 64a464536bd..8b9f64f0f93 100644 --- a/compose/service.py +++ b/compose/service.py @@ -701,12 +701,6 @@ def build(self, no_cache=False, pull=False, force_rm=False): 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 @@ def build(self, no_cache=False, pull=False, force_rm=False): 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 494beea3415..1e01fcb62f1 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 24ece49946d..3e5a7face61 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -715,7 +715,7 @@ def test_build_args_allow_empty_properties(self): ).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 ae2cab20838..a259c476fb5 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -445,7 +445,7 @@ def test_create_container(self): forcerm=False, nocache=False, rm=True, - buildargs={}, + buildargs=None, ) def test_ensure_image_exists_no_build(self): @@ -481,33 +481,7 @@ def test_ensure_image_exists_force_build(self): 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 1e01fcb62f1..925a8e791a5 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 3e5a7face61..0abb8daea94 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -717,6 +717,34 @@ def test_build_args_allow_empty_properties(self): 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',