Merge pull request #3266 from 0mar/reduce_ssl_testing

Test internal_ssl separately

.github/workflows/test.yml

@@ -77,6 +77,10 @@ jobs:
         #   Tests everything when the user instances are started with
         #   jupyter_server instead of notebook.
         #
+        # ssl:
+        #   Tests everything using internal SSL connections instead of
+        #   unencrypted HTTP
+        #
         # main_dependencies:
         #   Tests everything when the we use the latest available dependencies
         #   from: ipytraitlets.
@@ -91,6 +95,8 @@ jobs:
             subdomain: subdomain
           - python: "3.7"
             db: mysql
+          - python: "3.7"
+            ssl: ssl
           - python: "3.8"
             db: postgres
           - python: "3.8"
@@ -111,6 +117,9 @@ jobs:
               echo "MYSQL_HOST=127.0.0.1" >> $GITHUB_ENV
               echo "JUPYTERHUB_TEST_DB_URL=mysql+mysqlconnector://root@127.0.0.1:3306/jupyterhub" >> $GITHUB_ENV
           fi
+          if [ "${{ matrix.ssl }}" == "ssl" ]; then
+              echo "SSL_ENABLED=1" >> $GITHUB_ENV
+          fi
           if [ "${{ matrix.db }}" == "postgres" ]; then
               echo "PGHOST=127.0.0.1" >> $GITHUB_ENV
               echo "PGUSER=test_user" >> $GITHUB_ENV

jupyterhub/proxy.py

@@ -586,6 +586,34 @@ class ConfigurableHTTPProxy(Proxy):
             self.log.debug("PID file %s already removed", self.pid_file)
             pass
 
+    def _get_ssl_options(self):
+        """List of cmd proxy options to use internal SSL"""
+        cmd = []
+        proxy_api = 'proxy-api'
+        proxy_client = 'proxy-client'
+        api_key = self.app.internal_proxy_certs[proxy_api][
+            'keyfile'
+        ]  # Check content in next test and just patch manulaly or in the config of the file
+        api_cert = self.app.internal_proxy_certs[proxy_api]['certfile']
+        api_ca = self.app.internal_trust_bundles[proxy_api + '-ca']
+
+        client_key = self.app.internal_proxy_certs[proxy_client]['keyfile']
+        client_cert = self.app.internal_proxy_certs[proxy_client]['certfile']
+        client_ca = self.app.internal_trust_bundles[proxy_client + '-ca']
+
+        cmd.extend(['--api-ssl-key', api_key])
+        cmd.extend(['--api-ssl-cert', api_cert])
+        cmd.extend(['--api-ssl-ca', api_ca])
+        cmd.extend(['--api-ssl-request-cert'])
+        cmd.extend(['--api-ssl-reject-unauthorized'])
+
+        cmd.extend(['--client-ssl-key', client_key])
+        cmd.extend(['--client-ssl-cert', client_cert])
+        cmd.extend(['--client-ssl-ca', client_ca])
+        cmd.extend(['--client-ssl-request-cert'])
+        cmd.extend(['--client-ssl-reject-unauthorized'])
+        return cmd
+
     async def start(self):
         """Start the proxy process"""
         # check if there is a previous instance still around
@@ -617,27 +645,7 @@ class ConfigurableHTTPProxy(Proxy):
         if self.ssl_cert:
             cmd.extend(['--ssl-cert', self.ssl_cert])
         if self.app.internal_ssl:
-            proxy_api = 'proxy-api'
+            cmd.extend(self._get_ssl_options())
-            proxy_client = 'proxy-client'
-            api_key = self.app.internal_proxy_certs[proxy_api]['keyfile']
-            api_cert = self.app.internal_proxy_certs[proxy_api]['certfile']
-            api_ca = self.app.internal_trust_bundles[proxy_api + '-ca']
-
-            client_key = self.app.internal_proxy_certs[proxy_client]['keyfile']
-            client_cert = self.app.internal_proxy_certs[proxy_client]['certfile']
-            client_ca = self.app.internal_trust_bundles[proxy_client + '-ca']
-
-            cmd.extend(['--api-ssl-key', api_key])
-            cmd.extend(['--api-ssl-cert', api_cert])
-            cmd.extend(['--api-ssl-ca', api_ca])
-            cmd.extend(['--api-ssl-request-cert'])
-            cmd.extend(['--api-ssl-reject-unauthorized'])
-
-            cmd.extend(['--client-ssl-key', client_key])
-            cmd.extend(['--client-ssl-cert', client_cert])
-            cmd.extend(['--client-ssl-ca', client_ca])
-            cmd.extend(['--client-ssl-request-cert'])
-            cmd.extend(['--client-ssl-reject-unauthorized'])
         if self.app.statsd_host:
             cmd.extend(
                 [

jupyterhub/tests/conftest.py

@@ -76,7 +76,9 @@ def ssl_tmpdir(tmpdir_factory):
 def app(request, io_loop, ssl_tmpdir):
     """Mock a jupyterhub app for testing"""
     mocked_app = None
-    ssl_enabled = getattr(request.module, "ssl_enabled", False)
+    ssl_enabled = getattr(
+        request.module, 'ssl_enabled', os.environ.get('SSL_ENABLED', False)
+    )
     kwargs = dict()
     if ssl_enabled:
         kwargs.update(dict(internal_ssl=True, internal_certs_location=str(ssl_tmpdir)))

jupyterhub/tests/test_api.py

@@ -25,7 +25,6 @@ from .utils import async_requests
 from .utils import auth_header
 from .utils import find_user
 
-
 # --------------------
 # Authentication tests
 # --------------------

jupyterhub/tests/test_internal_ssl_api.py

@@ -1,6 +0,0 @@
-"""Tests for the SSL enabled REST API."""
-# Copyright (c) Jupyter Development Team.
-# Distributed under the terms of the Modified BSD License.
-from jupyterhub.tests.test_api import *
-
-ssl_enabled = True

jupyterhub/tests/test_internal_ssl_app.py

@@ -1,7 +0,0 @@
-"""Test the JupyterHub entry point with internal ssl"""
-# Copyright (c) Jupyter Development Team.
-# Distributed under the terms of the Modified BSD License.
-import jupyterhub.tests.mocking
-from jupyterhub.tests.test_app import *
-
-ssl_enabled = True

jupyterhub/tests/test_internal_ssl_spawner.py

@@ -1,6 +0,0 @@
-"""Tests for process spawning with internal_ssl"""
-# Copyright (c) Jupyter Development Team.
-# Distributed under the terms of the Modified BSD License.
-from jupyterhub.tests.test_spawner import *
-
-ssl_enabled = True

jupyterhub/tests/test_proxy.py

@@ -9,12 +9,12 @@ from urllib.parse import urlparse
 import pytest
 from traitlets.config import Config
 
-from .. import orm
 from ..utils import url_path_join as ujoin
 from ..utils import wait_for_http_server
 from .mocking import MockHub
 from .test_api import add_user
 from .test_api import api_request
+from .utils import skip_if_ssl
 
 
 @pytest.fixture
@@ -27,6 +27,7 @@ def disable_check_routes(app):
         app.last_activity_callback.start()
 
 
+@skip_if_ssl
 async def test_external_proxy(request):
     auth_token = 'secret!'
     proxy_ip = '127.0.0.1'

jupyterhub/tests/test_services.py

@@ -15,6 +15,7 @@ from ..utils import url_path_join
 from ..utils import wait_for_http_server
 from .mocking import public_url
 from .utils import async_requests
+from .utils import skip_if_ssl
 
 mockservice_path = os.path.dirname(os.path.abspath(__file__))
 mockservice_py = os.path.join(mockservice_path, 'mockservice.py')
@@ -60,6 +61,7 @@ async def test_managed_service(mockservice):
     assert service.proc.poll() is None
 
 
+@skip_if_ssl
 async def test_proxy_service(app, mockservice_url):
     service = mockservice_url
     name = service.name
@@ -73,6 +75,7 @@ async def test_proxy_service(app, mockservice_url):
     assert r.text.endswith(path)
 
 
+@skip_if_ssl
 async def test_external_service(app):
     name = 'external'
     async with external_service(app, name=name) as env:

jupyterhub/tests/test_services_auth.py

@@ -35,6 +35,7 @@ from .utils import AsyncSession
 
 # mock for sending monotonic counter way into the future
 monotonic_future = mock.patch('time.monotonic', lambda: sys.maxsize)
+ssl_enabled = False
 
 
 def test_expiring_dict():

jupyterhub/tests/utils.py

@@ -1,6 +1,8 @@
 import asyncio
+import os
 from concurrent.futures import ThreadPoolExecutor
 
+import pytest
 import requests
 from certipy import Certipy
 
@@ -53,6 +55,12 @@ def ssl_setup(cert_dir, authority_name):
     return external_certs
 
 
+"""Skip tests that don't work under internal-ssl when testing under internal-ssl"""
+skip_if_ssl = pytest.mark.skipif(
+    os.environ.get('SSL_ENABLED', False), reason="Does not use internal SSL"
+)
+
+
 def check_db_locks(func):
     """Decorator that verifies no locks are held on database upon exit.