FileConfig consistency with dockerpy

jupyterhub/remoteappmanager_config.py

@@ -15,23 +15,19 @@
 # # TLS configuration
 # # -----------------
 # #
-# # Set this to True only if your docker machine has a certificate signed by
-# # a recognised CA.
-# # If using self-signed certificates, using tls as True will produce an error
-# # of incorrect CA validation. As a consequence, the default tls setting is
-# # False, and tls_verify is according to the current environment (likely True
-# # with default setup on OSX), honoring docker documentation.
-# # See https://docs.docker.com/engine/security/https/ for additional details
+# # Set this to True to enable TLS connection with the docker client
 #
 # tls = True
 #
 # # Enables verification of the certificates. By default, this is the
-# # result of the DOCKER_TLS_VERIFY envvar
+# # result of the DOCKER_TLS_VERIFY envvar. Set to False to skip verification/
 #
 # tls_verify = True
 #
 # # Full paths of the CA certificate, certificate and key of the docker
-# # machine. Normally these are computed from the DOCKER_CERT_PATH
+# # machine. Normally these are computed from the DOCKER_CERT_PATH.
+# # If you want to use a recognised CA for verification, set the tls_ca to
+# # an empty string
 #
 # tls_ca = "/path/to/ca.pem"
 # tls_cert = "/path/to/cert.pem"

remoteappmanager/file_config.py

@@ -15,19 +15,12 @@ class FileConfig(HasTraits):
 
     ##########
     # Configuration file options. All of these come from the config file.
-
-    #: Enable tls, with a twist. If self-signed certificates are used,
-    #: setting tls as True will produce an error of incorrect CA validation.
-    #: As a consequence, defaults to False. TLS secure connection will still
-    #: happen thanks to tls_verify and tls[_cert|_key|_ca] being defined.
-    #: See https://docs.docker.com/engine/security/https/
-    #: Verification can be enabled simply by issuing tls=True in the
-    #: config file
     tls = Bool(False,
-               help="If True, connect to docker with --tls")
+               help="If True, connect to docker with tls")
 
-    tls_verify = Bool(False,
-                      help="If True, connect to docker with --tlsverify")
+    tls_verify = Bool(True,
+                      help="If True, verify the CA certificate against a "
+                           "known or self-signed CA certificate")
 
     tls_ca = Unicode("", help="Path to CA certificate for docker TLS")
 
@@ -39,7 +32,7 @@ class FileConfig(HasTraits):
 
     accounting_class = Unicode(
         default_value="remoteappmanager.db.orm.AppAccounting",
-        help="The import path to a subclass of ABCAccounting.")
+        help="The import path to a subclass of ABCAccounting")
 
     accounting_kwargs = Dict(
         default_value={'url': 'sqlite:///remoteappmanager.db'},
@@ -48,7 +41,7 @@ class FileConfig(HasTraits):
     login_url = Unicode(default_value="/hub",
                         help=("The url to be redirected to if the user is not "
                               "authenticated for pages that require "
-                              "authentication."))
+                              "authentication"))
 
     # The network timeout for any async operation we have to perform,
     # in seconds. 30 seconds is plenty enough.
@@ -77,18 +70,18 @@ def __init__(self, *args, **kwargs):
             self.docker_host = "unix://var/run/docker.sock"
 
         self.tls_verify = (env.get("DOCKER_TLS_VERIFY", "") != "")
+        # We don't have an envvar way of saying TLS = True, so we rely on
+        # TLS_VERIFY set status
+        self.tls = self.tls_verify
 
-        # Note that certificate paths can still be present even if tls_verify
-        # is false: that is the case of using certificates signed by an
-        # authoritative CA.
         cert_path = env.get("DOCKER_CERT_PATH",
                             os.path.join(os.path.expanduser("~"), ".docker"))
 
         self.tls_cert = os.path.join(cert_path, 'cert.pem')
         self.tls_key = os.path.join(cert_path, 'key.pem')
         self.tls_ca = os.path.join(cert_path, 'ca.pem')
 
-        if self.tls_verify or self.tls:
+        if self.tls:
             self.docker_host = self.docker_host.replace('tcp://', 'https://')
 
     # -------------------------------------------------------------------------
@@ -142,17 +135,18 @@ def docker_config(self):
         # present at the specified paths.
         # Note that the tls flag takes precedence against tls verify.
         # This is docker behavior.
-        if self.tls:
-            params["tls"] = tls.TLSConfig(
-                client_cert=(self.tls_cert, self.tls_key),
-                )
-        elif self.tls_verify:
-            params["tls"] = tls.TLSConfig(
-                client_cert=(self.tls_cert, self.tls_key),
-                ca_cert=self.tls_ca,
-                verify=True,
-            )
-
         params["version"] = "auto"
 
+        if not self.tls:
+            return params
+
+        tls_kwargs = {}
+        tls_kwargs["client_cert"] = (self.tls_cert, self.tls_key)
+        tls_kwargs["verify"] = self.tls_verify
+
+        if self.tls_verify and self.tls_ca:
+            tls_kwargs["ca_cert"] = self.tls_ca
+
+        params["tls"] = tls.TLSConfig(**tls_kwargs)
+
         return params

tests/test_file_config.py

@@ -83,7 +83,7 @@ def test_tls_no_verify(self):
         config.parse_config(self.config_file)
         docker_dict = config.docker_config()
         self.assertIn("tls", docker_dict)
-        self.assertEqual(docker_dict["tls"].verify, None)
+        self.assertEqual(docker_dict["tls"].verify, False)
         self.assertEqual(docker_dict["tls"].ca_cert, None)
 
     def test_initialization_with_good_accounting(self):
@@ -106,8 +106,7 @@ def test_initialization_on_nonlocal_docker_machine(self):
 
         with envvars(envs):
             config = FileConfig()
-            # False by default, as we don't want CA verification
-            self.assertEqual(config.tls, False)
+            self.assertEqual(config.tls, True)
             self.assertEqual(config.tls_verify, True)
             self.assertEqual(config.tls_ca,
                              os.path.join(self.tempdir, "ca.pem"))
@@ -141,9 +140,9 @@ def test_overriding(self):
 
         with envvars(envs):
             config = FileConfig()
+            config.tls = False
             config.tls_verify = False
             config.docker_host = "tcp://192.168.99.100:31337"
-            # False by default, as we don't want CA verification
             docker_config = config.docker_config()
             self.assertNotIn("tls", docker_config)
             self.assertEqual(docker_config["base_url"],