Merge pull request #5634 from docker/5632-tls-verify

Fix DOCKER_TLS_VERIFY bug

compose/cli/docker_client.py

@@ -10,6 +10,7 @@ from docker.errors import TLSParameterError
 from docker.tls import TLSConfig
 from docker.utils import kwargs_from_env
 
+from ..config.environment import Environment
 from ..const import HTTP_TIMEOUT
 from .errors import UserError
 from .utils import generate_user_agent
@@ -36,14 +37,18 @@ def get_tls_version(environment):
 
 
 def tls_config_from_options(options, environment=None):
-    environment = environment or {}
+    environment = environment or Environment()
     cert_path = environment.get('DOCKER_CERT_PATH') or None
 
     tls = options.get('--tls', False)
     ca_cert = unquote_path(options.get('--tlscacert'))
     cert = unquote_path(options.get('--tlscert'))
     key = unquote_path(options.get('--tlskey'))
-    verify = options.get('--tlsverify', environment.get('DOCKER_TLS_VERIFY'))
+    # verify is a special case - with docopt `--tlsverify` = False means it
+    # wasn't used, so we set it if either the environment or the flag is True
+    # see https://github.com/docker/compose/issues/5632
+    verify = options.get('--tlsverify') or environment.get_boolean('DOCKER_TLS_VERIFY')
+
     skip_hostname_check = options.get('--skip-hostname-check', False)
     if cert_path is not None and not any((ca_cert, cert, key)):
         # FIXME: Modify TLSConfig to take a cert_path argument and do this internally

tests/unit/cli/docker_client_test.py

@@ -13,6 +13,7 @@ from compose.cli import errors
 from compose.cli.docker_client import docker_client
 from compose.cli.docker_client import get_tls_version
 from compose.cli.docker_client import tls_config_from_options
+from compose.config.environment import Environment
 from tests import mock
 from tests import unittest
 
@@ -163,14 +164,14 @@ class TLSConfigTestCase(unittest.TestCase):
     def test_tls_simple_with_tls_version(self):
         tls_version = 'TLSv1'
         options = {'--tls': True}
-        environment = {'COMPOSE_TLS_VERSION': tls_version}
+        environment = Environment({'COMPOSE_TLS_VERSION': tls_version})
         result = tls_config_from_options(options, environment)
         assert isinstance(result, docker.tls.TLSConfig)
         assert result.ssl_version == ssl.PROTOCOL_TLSv1
 
     def test_tls_mixed_environment_and_flags(self):
         options = {'--tls': True, '--tlsverify': False}
-        environment = {'DOCKER_CERT_PATH': 'tests/fixtures/tls/'}
+        environment = Environment({'DOCKER_CERT_PATH': 'tests/fixtures/tls/'})
         result = tls_config_from_options(options, environment)
         assert isinstance(result, docker.tls.TLSConfig)
         assert result.cert == (self.client_cert, self.key)
@@ -178,15 +179,42 @@ class TLSConfigTestCase(unittest.TestCase):
         assert result.verify is False
 
     def test_tls_flags_override_environment(self):
-        environment = {'DOCKER_TLS_VERIFY': True}
+        environment = Environment({
+            'DOCKER_CERT_PATH': '/completely/wrong/path',
+            'DOCKER_TLS_VERIFY': 'false'
+        })
+        options = {
+            '--tlscacert': '"{0}"'.format(self.ca_cert),
+            '--tlscert': '"{0}"'.format(self.client_cert),
+            '--tlskey': '"{0}"'.format(self.key),
+            '--tlsverify': True
+        }
+
+        result = tls_config_from_options(options, environment)
+        assert isinstance(result, docker.tls.TLSConfig)
+        assert result.cert == (self.client_cert, self.key)
+        assert result.ca_cert == self.ca_cert
+        assert result.verify is True
+
+    def test_tls_verify_flag_no_override(self):
+        environment = Environment({
+            'DOCKER_TLS_VERIFY': 'true',
+            'COMPOSE_TLS_VERSION': 'TLSv1'
+        })
         options = {'--tls': True, '--tlsverify': False}
-        assert tls_config_from_options(options, environment) is True
 
-        environment['COMPOSE_TLS_VERSION'] = 'TLSv1'
         result = tls_config_from_options(options, environment)
         assert isinstance(result, docker.tls.TLSConfig)
         assert result.ssl_version == ssl.PROTOCOL_TLSv1
-        assert result.verify is False
+        # verify is a special case - since `--tlsverify` = False means it
+        # wasn't used, we set it if either the environment or the flag is True
+        # see https://github.com/docker/compose/issues/5632
+        assert result.verify is True
+
+    def test_tls_verify_env_falsy_value(self):
+        environment = Environment({'DOCKER_TLS_VERIFY': '0'})
+        options = {'--tls': True}
+        assert tls_config_from_options(options, environment) is True
 
 
 class TestGetTlsVersion(object):