From b03137fe0c26eb493724ba920643742846e4f72f Mon Sep 17 00:00:00 2001 From: Min RK Date: Fri, 3 Mar 2023 14:53:34 +0100 Subject: [PATCH 1/7] restore proxy tests --- tests/test_proxy.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_proxy.py b/tests/test_proxy.py index 850dfe7d..1cf45857 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -1,6 +1,7 @@ """Tests for the base traefik proxy""" import pytest +from proxytest import * # noqa from jupyterhub_traefik_proxy.proxy import TraefikProxy From b4f978099d4ec7e55aea1e96d7bdc3a0c0ca8690 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 6 Mar 2023 08:17:26 +0100 Subject: [PATCH 2/7] add explanatory comment to proxy tests Co-authored-by: Simon Li --- tests/test_proxy.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_proxy.py b/tests/test_proxy.py index 1cf45857..04371ea6 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -1,6 +1,8 @@ """Tests for the base traefik proxy""" import pytest + +# Some proxy tests are defined in `proxytest` so that they can be used in external repositories from proxytest import * # noqa from jupyterhub_traefik_proxy.proxy import TraefikProxy From 174be9a882be7fef786f38697010f7fc99987320 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 6 Mar 2023 08:26:11 +0100 Subject: [PATCH 3/7] etcdctl txn needs two trailing newlines on stdin and make sure to check the returncode --- tests/conftest.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 1c012ac0..76b38054 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -393,8 +393,12 @@ def _config_etcd(*extra_args): proc = subprocess.Popen( data_store_cmd, stdin=subprocess.PIPE, env=Config.etcdctl_env ) - proc.communicate(txns.encode()) + # need two trailing newlines for etcdctl txn to complete + proc.communicate(txns.encode() + b'\n\n') proc.wait() + assert ( + proc.returncode == 0 + ), f"{data_store_cmd} exited with status {proc.returncode}" @pytest.fixture From c875c349a37134686beb8776d22a5e304351bc43 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 6 Mar 2023 08:36:29 +0100 Subject: [PATCH 4/7] merge proxy tests into test_proxy if we want re-usable tests, let's move them to pytest-jupyterhub --- tests/proxytest.py | 486 ------------------------------------------- tests/test_proxy.py | 492 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 486 insertions(+), 492 deletions(-) delete mode 100644 tests/proxytest.py diff --git a/tests/proxytest.py b/tests/proxytest.py deleted file mode 100644 index 0a9c8e6c..00000000 --- a/tests/proxytest.py +++ /dev/null @@ -1,486 +0,0 @@ -"""Tests for the base traefik proxy""" - -import asyncio -import copy -import inspect -import pprint -import subprocess -import sys -from contextlib import contextmanager -from os.path import abspath, dirname, join -from random import randint -from unittest.mock import Mock -from urllib.parse import quote, urlparse - -import pytest -import utils -import websockets -from jupyterhub.objects import Hub, Server -from jupyterhub.user import User -from jupyterhub.utils import exponential_backoff, url_path_join -from tornado.httpclient import AsyncHTTPClient, HTTPClientError, HTTPRequest - -pp = pprint.PrettyPrinter(indent=2) - - -class MockApp: - def __init__(self): - self.hub = Hub(routespec="/") - - -class MockSpawner: - name = "" - server = None - pending = None - - def __init__(self, name="", *, user, **kwargs): - self.name = name - self.user = user - for key, value in kwargs.items(): - setattr(self, key, value) - self.proxy_spec = url_path_join(self.user.proxy_spec, name, "/") - - def start(self): - self.server = Server.from_url("http://127.0.0.1:%i" % randint(1025, 65535)) - - def stop(self): - self.server = None - - @property - def ready(self): - """Is this server ready to use? - - A server is not ready if an event is pending. - """ - return self.server is not None - - @property - def active(self): - """Return True if the server is active. - - This includes fully running and ready or any pending start/stop event. - """ - return self.ready - - -class MockUser(User): - """Mock User for use in proxytest""" - - def __init__(self, name): - orm_user = Mock() - orm_user.name = name - orm_user.orm_spawners = "" - super().__init__(orm_user=orm_user, db=Mock()) - - def _new_spawner(self, spawner_name, **kwargs): - return MockSpawner(spawner_name, user=self, **kwargs) - - -def assert_equal(value, expected): - try: - assert value == expected - except AssertionError: - pp.pprint({'value': value}) - pp.pprint({"expected": expected}) - raise - - -@pytest.fixture -def launch_backend(): - dummy_server_path = abspath(join(dirname(__file__), "dummy_http_server.py")) - running_backends = [] - - def _launch_backend(port, proto="http"): - backend = subprocess.Popen( - [sys.executable, dummy_server_path, str(port), proto] - ) - running_backends.append(backend) - - yield _launch_backend - - for proc in running_backends: - proc.terminate() - for proc in running_backends: - proc.communicate() - for proc in running_backends: - proc.wait() - - -async def wait_for_services(urls): - # Wait until traefik and the backend are ready - await exponential_backoff( - utils.check_services_ready, "Service not reacheable", urls=urls - ) - - -@pytest.mark.parametrize( - "routespec, existing_routes", - [ - # default route - ( - "/", - [ - "/abc", - "/has%20space/", - "/has%20space/foo/", - "/missing-trailing/", - "/missing-trailing/slash", - "/has/", - "/has/@/", - "host.name/", - "host.name/path/", - "other.host/", - "other.host/path/", - "other.host/path/no/", - "other.host/path/no/slash", - ], - ), - ("/has%20space/foo/", ["/", "/has%20space/", "/has%20space/foo/abc/"]), - ( - "/missing-trailing/slash", - ["/", "/missing-trailing/", "/missing-trailing/slash/abc"], - ), - ("/has/@/", ["/", "/has/", "/has/@/abc/"]), - ( - "/has/" + quote("üñîçø∂é"), - ["/", "/has/", "/has/" + quote("üñîçø∂é") + "/abc/"], - ), - ("host.name/path/", ["/", "host.name/", "host.name/path/abc/"]), - ( - "other.host/path/no/slash", - [ - "/", - "other.host/", - "other.host/path/", - "other.host/path/no/", - "other.host/path/no/slash/abc/", - ], - ), - ], -) -async def test_add_get_delete( - request, proxy, launch_backend, routespec, existing_routes, event_loop -): - default_target = "http://127.0.0.1:9000" - data = {"test": "test1", "user": "username"} - - default_backend = urlparse(default_target) - extra_backends = [] - - proxy_url = proxy.public_url.rstrip("/") - - def normalize_spec(spec): - return proxy.validate_routespec(spec) - - def expected_output(spec, url): - return { - "routespec": normalize_spec(spec), - "target": url, - "data": data, - } - - # just use existing Jupyterhub check instead of making own one - def expect_value_error(spec): - try: - normalize_spec(spec) - except ValueError: - return True - - return False - - @contextmanager - def context(spec): - if expect_value_error(spec): - with pytest.raises(ValueError): - yield - else: - yield - - async def test_route_exist(spec, backend): - with context(spec): - route = await proxy.get_route(spec) - - if not expect_value_error(spec): - try: - del route["data"]["last_activity"] # CHP - except TypeError as e: - raise TypeError(f"{e}\nRoute got:{route}") - except KeyError: - pass - - assert_equal(route, expected_output(spec, backend.geturl())) - - # Test the actual routing - responding_backend1 = await utils.get_responding_backend_port( - proxy_url, normalize_spec(spec) - ) - responding_backend2 = await utils.get_responding_backend_port( - proxy_url, normalize_spec(spec) + "something" - ) - assert responding_backend1 == backend.port - assert responding_backend2 == backend.port - - for i, spec in enumerate(existing_routes, start=1): - backend = default_backend._replace( - netloc=f"{default_backend.hostname}:{default_backend.port+i}" - ) - launch_backend(backend.port, backend.scheme) - extra_backends.append(backend) - - launch_backend(default_backend.port, default_backend.scheme) - await wait_for_services( - [proxy.public_url, default_backend.geturl()] - + [backend.geturl() for backend in extra_backends] - ) - - # Create existing routes - futures = [] - for i, spec in enumerate(existing_routes): - f = proxy.add_route(spec, extra_backends[i].geturl(), copy.copy(data)) - futures.append(f) - - if futures: - await asyncio.gather(*futures) - - def finalizer(): - async def cleanup(): - """Cleanup""" - futures = [] - for spec in existing_routes: - try: - f = proxy.delete_route(spec) - futures.append(f) - except Exception: - pass - if futures: - await asyncio.gather(*futures) - - event_loop.run_until_complete(cleanup()) - - request.addfinalizer(finalizer) - - # Test add - with context(routespec): - await proxy.add_route(routespec, default_backend.geturl(), copy.copy(data)) - - # Test get - await test_route_exist(routespec, default_backend) - for i, spec in enumerate(existing_routes): - await test_route_exist(spec, extra_backends[i]) - - # Test delete - with context(routespec): - await proxy.delete_route(routespec) - route = await proxy.get_route(routespec) - - # Test that deleted route does not exist anymore - if not expect_value_error(routespec): - assert_equal(route, None) - - async def _wait_for_deletion(): - deleted = 0 - for spec in [ - normalize_spec(routespec), - normalize_spec(routespec) + "something", - ]: - try: - result = await utils.get_responding_backend_port(proxy_url, spec) - if result != default_backend.port: - deleted += 1 - except HTTPClientError: - deleted += 1 - - return deleted == 2 - - # If this raises a TimeoutError, the route wasn't properly deleted, - # thus the proxy still has a route for the given routespec - await exponential_backoff(_wait_for_deletion, "Route still exists") - - # Test that other routes are still exist - for i, spec in enumerate(existing_routes): - await test_route_exist(spec, extra_backends[i]) - - -async def test_get_all_routes(proxy, launch_backend): - routespecs = ["/proxy/path1", "/proxy/path2/", "/proxy/path3/"] - targets = [ - "http://127.0.0.1:9900", - "http://127.0.0.1:9090", - "http://127.0.0.1:9999", - ] - datas = [{"test": "test1"}, {}, {"test": "test2"}] - - expected_output = { - routespecs[0] - + "/": { - "routespec": routespecs[0] + "/", - "target": targets[0], - "data": datas[0], - }, - routespecs[1]: { - "routespec": routespecs[1], - "target": targets[1], - "data": datas[1], - }, - routespecs[2]: { - "routespec": routespecs[2], - "target": targets[2], - "data": datas[2], - }, - } - - for target in targets: - launch_backend(urlparse(target).port) - - await wait_for_services([proxy.public_url] + targets) - - futures = [] - for routespec, target, data in zip(routespecs, targets, datas): - f = proxy.add_route(routespec, target, copy.copy(data)) - futures.append(f) - if futures: - await asyncio.gather(*futures) - - routes = await proxy.get_all_routes() - try: - for route_key in routes.keys(): - del routes[route_key]["data"]["last_activity"] # CHP - except KeyError: - pass - - assert_equal(routes, expected_output) - - -async def test_host_origin_headers(proxy, launch_backend): - routespec = "/user/username/" - target = "http://127.0.0.1:9000" - data = {} - - proxy_url = urlparse(proxy.public_url) - traefik_port = proxy_url.port - traefik_host = proxy_url.hostname - default_backend_port = urlparse(target).port - launch_backend(default_backend_port) - - # wait for traefik to be reachable - await exponential_backoff( - utils.check_host_up_http, - "Traefik not reacheable", - url=proxy_url.geturl(), - ) - - # wait for backend to be reachable - await exponential_backoff( - utils.check_host_up_http, - "Backends not reacheable", - url=target, - ) - - # Add route to default_backend - await proxy.add_route(routespec, target, data) - - req_url = proxy.public_url.rstrip("/") + routespec - - expected_host_header = traefik_host + ":" + str(traefik_port) - expected_origin_header = proxy.public_url + routespec - - req = HTTPRequest( - req_url, - method="GET", - headers={"Host": expected_host_header, "Origin": expected_origin_header}, - validate_cert=False, - ) - resp = await AsyncHTTPClient().fetch(req) - - assert resp.headers["Host"] == expected_host_header - assert resp.headers["Origin"] == expected_origin_header - - -@pytest.mark.parametrize("username", ["zoe", "50fia", "秀樹", "~TestJH", "has@"]) -async def test_check_routes(proxy, username): - # fill out necessary attributes for check_routes - proxy.app = MockApp() - proxy.hub = proxy.app.hub - - users = {} - services = {} - # run initial check first, to ensure that `/` is in the routes - await proxy.check_routes(users, services) - routes = await proxy.get_all_routes() - assert sorted(routes) == ["/"] - - users[username] = test_user = MockUser(username) - spawner = test_user.spawners[""] - f = spawner.start() - if inspect.isawaitable(f): - await f - assert spawner.ready - assert spawner.active - await proxy.add_user(test_user, "") - - # check a valid route exists for user - routes = await proxy.get_all_routes() - before = sorted(routes) - assert test_user.proxy_spec in before - - # check if a route is removed when user deleted - await proxy.check_routes(users, services) - await proxy.delete_user(test_user) - routes = await proxy.get_all_routes() - during = sorted(routes) - assert test_user.proxy_spec not in during - - # check if a route exists for user - await proxy.check_routes(users, services) - routes = await proxy.get_all_routes() - after = sorted(routes) - assert test_user.proxy_spec in after - - # check that before and after state are the same - assert_equal(before, after) - - -async def test_websockets(proxy, launch_backend): - import ssl - - routespec = "/user/username/" - target = "http://127.0.0.1:9000" - data = {} - - proxy_url = urlparse(proxy.public_url) - proxy_url.port - proxy_url.hostname - default_backend_port = urlparse(target).port - launch_backend(default_backend_port, "ws") - - # wait for traefik to be reachable - await exponential_backoff( - utils.check_host_up_http, - "Traefik cannot be reached", - url=proxy_url.geturl(), - ) - - # wait for backend to be reachable - await exponential_backoff( - utils.check_host_up_http, - "Backends cannot be reached", - url=target, - ) - - # Add route to default_backend - await proxy.add_route(routespec, target, data) - - if proxy.is_https: - kwargs = {'ssl': ssl._create_unverified_context()} - scheme = "wss://" - else: - kwargs = {} - scheme = "ws://" - req_url = scheme + proxy_url.netloc + routespec - - # Don't validate the ssl certificate, it's self-signed by traefik - print(f"Connecting with websockets to {req_url}") - async with websockets.connect(req_url, **kwargs) as websocket: - port = await websocket.recv() - - assert port == str(default_backend_port) diff --git a/tests/test_proxy.py b/tests/test_proxy.py index 04371ea6..c90f8a8a 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -1,14 +1,115 @@ -"""Tests for the base traefik proxy""" +"""Tests for the treafik proxy implementations""" -import pytest +import asyncio +import copy +import inspect +import pprint +import ssl +import subprocess +import sys +from contextlib import contextmanager +from os.path import abspath, dirname, join +from random import randint +from unittest.mock import Mock +from urllib.parse import quote, urlparse -# Some proxy tests are defined in `proxytest` so that they can be used in external repositories -from proxytest import * # noqa +import pytest +import utils +import websockets +from jupyterhub.objects import Hub, Server +from jupyterhub.user import User +from jupyterhub.utils import exponential_backoff, url_path_join +from tornado.httpclient import AsyncHTTPClient, HTTPClientError, HTTPRequest from jupyterhub_traefik_proxy.proxy import TraefikProxy -# Mark all tests in this file as asyncio and slow -pytestmark = [pytest.mark.asyncio, pytest.mark.slow] +# Mark all tests in this file as slow +pytestmark = [pytest.mark.slow] + +pp = pprint.PrettyPrinter(indent=2) + + +class MockApp: + def __init__(self): + self.hub = Hub(routespec="/") + + +class MockSpawner: + name = "" + server = None + pending = None + + def __init__(self, name="", *, user, **kwargs): + self.name = name + self.user = user + for key, value in kwargs.items(): + setattr(self, key, value) + self.proxy_spec = url_path_join(self.user.proxy_spec, name, "/") + + def start(self): + self.server = Server.from_url("http://127.0.0.1:%i" % randint(1025, 65535)) + + def stop(self): + self.server = None + + @property + def ready(self): + """Is this server ready to use? + + A server is not ready if an event is pending. + """ + return self.server is not None + + @property + def active(self): + """Return True if the server is active. + + This includes fully running and ready or any pending start/stop event. + """ + return self.ready + + +class MockUser(User): + """Mock User for use in proxytest""" + + def __init__(self, name): + orm_user = Mock() + orm_user.name = name + orm_user.orm_spawners = "" + super().__init__(orm_user=orm_user, db=Mock()) + + def _new_spawner(self, spawner_name, **kwargs): + return MockSpawner(spawner_name, user=self, **kwargs) + + +def assert_equal(value, expected): + try: + assert value == expected + except AssertionError: + pp.pprint({'value': value}) + pp.pprint({"expected": expected}) + raise + + +@pytest.fixture +def launch_backend(): + dummy_server_path = abspath(join(dirname(__file__), "dummy_http_server.py")) + running_backends = [] + + def _launch_backend(port, proto="http"): + backend = subprocess.Popen( + [sys.executable, dummy_server_path, str(port), proto] + ) + running_backends.append(backend) + + yield _launch_backend + + for proc in running_backends: + proc.terminate() + for proc in running_backends: + proc.communicate() + for proc in running_backends: + proc.wait() @pytest.fixture( @@ -28,9 +129,17 @@ ] ) def proxy(request): + """Parameterized fixture to run all the tests with every proxy implementation""" return request.getfixturevalue(request.param) +async def wait_for_services(urls): + # Wait until traefik and the backend are ready + await exponential_backoff( + utils.check_services_ready, "Service not reacheable", urls=urls + ) + + def test_default_port(): p = TraefikProxy( public_url="http://127.0.0.1/", traefik_api_url="https://127.0.0.1/" @@ -40,3 +149,374 @@ def test_default_port(): with pytest.raises(ValueError): TraefikProxy(public_url="ftp://127.0.0.1:23/") + + +@pytest.mark.parametrize( + "routespec, existing_routes", + [ + # default route + ( + "/", + [ + "/abc", + "/has%20space/", + "/has%20space/foo/", + "/missing-trailing/", + "/missing-trailing/slash", + "/has/", + "/has/@/", + "host.name/", + "host.name/path/", + "other.host/", + "other.host/path/", + "other.host/path/no/", + "other.host/path/no/slash", + ], + ), + ("/has%20space/foo/", ["/", "/has%20space/", "/has%20space/foo/abc/"]), + ( + "/missing-trailing/slash", + ["/", "/missing-trailing/", "/missing-trailing/slash/abc"], + ), + ("/has/@/", ["/", "/has/", "/has/@/abc/"]), + ( + "/has/" + quote("üñîçø∂é"), + ["/", "/has/", "/has/" + quote("üñîçø∂é") + "/abc/"], + ), + ("host.name/path/", ["/", "host.name/", "host.name/path/abc/"]), + ( + "other.host/path/no/slash", + [ + "/", + "other.host/", + "other.host/path/", + "other.host/path/no/", + "other.host/path/no/slash/abc/", + ], + ), + ], +) +async def test_add_get_delete( + request, proxy, launch_backend, routespec, existing_routes, event_loop +): + default_target = "http://127.0.0.1:9000" + data = {"test": "test1", "user": "username"} + + default_backend = urlparse(default_target) + extra_backends = [] + + proxy_url = proxy.public_url.rstrip("/") + + def normalize_spec(spec): + return proxy.validate_routespec(spec) + + def expected_output(spec, url): + return { + "routespec": normalize_spec(spec), + "target": url, + "data": data, + } + + # just use existing Jupyterhub check instead of making own one + def expect_value_error(spec): + try: + normalize_spec(spec) + except ValueError: + return True + + return False + + @contextmanager + def context(spec): + if expect_value_error(spec): + with pytest.raises(ValueError): + yield + else: + yield + + async def test_route_exist(spec, backend): + with context(spec): + route = await proxy.get_route(spec) + + if not expect_value_error(spec): + try: + del route["data"]["last_activity"] # CHP + except TypeError as e: + raise TypeError(f"{e}\nRoute got:{route}") + except KeyError: + pass + + assert_equal(route, expected_output(spec, backend.geturl())) + + # Test the actual routing + responding_backend1 = await utils.get_responding_backend_port( + proxy_url, normalize_spec(spec) + ) + responding_backend2 = await utils.get_responding_backend_port( + proxy_url, normalize_spec(spec) + "something" + ) + assert responding_backend1 == backend.port + assert responding_backend2 == backend.port + + for i, spec in enumerate(existing_routes, start=1): + backend = default_backend._replace( + netloc=f"{default_backend.hostname}:{default_backend.port+i}" + ) + launch_backend(backend.port, backend.scheme) + extra_backends.append(backend) + + launch_backend(default_backend.port, default_backend.scheme) + await wait_for_services( + [proxy.public_url, default_backend.geturl()] + + [backend.geturl() for backend in extra_backends] + ) + + # Create existing routes + futures = [] + for i, spec in enumerate(existing_routes): + f = proxy.add_route(spec, extra_backends[i].geturl(), copy.copy(data)) + futures.append(f) + + if futures: + await asyncio.gather(*futures) + + def finalizer(): + async def cleanup(): + """Cleanup""" + futures = [] + for spec in existing_routes: + try: + f = proxy.delete_route(spec) + futures.append(f) + except Exception: + pass + if futures: + await asyncio.gather(*futures) + + event_loop.run_until_complete(cleanup()) + + request.addfinalizer(finalizer) + + # Test add + with context(routespec): + await proxy.add_route(routespec, default_backend.geturl(), copy.copy(data)) + + # Test get + await test_route_exist(routespec, default_backend) + for i, spec in enumerate(existing_routes): + await test_route_exist(spec, extra_backends[i]) + + # Test delete + with context(routespec): + await proxy.delete_route(routespec) + route = await proxy.get_route(routespec) + + # Test that deleted route does not exist anymore + if not expect_value_error(routespec): + assert_equal(route, None) + + async def _wait_for_deletion(): + deleted = 0 + for spec in [ + normalize_spec(routespec), + normalize_spec(routespec) + "something", + ]: + try: + result = await utils.get_responding_backend_port(proxy_url, spec) + if result != default_backend.port: + deleted += 1 + except HTTPClientError: + deleted += 1 + + return deleted == 2 + + # If this raises a TimeoutError, the route wasn't properly deleted, + # thus the proxy still has a route for the given routespec + await exponential_backoff(_wait_for_deletion, "Route still exists") + + # Test that other routes are still exist + for i, spec in enumerate(existing_routes): + await test_route_exist(spec, extra_backends[i]) + + +async def test_get_all_routes(proxy, launch_backend): + routespecs = ["/proxy/path1", "/proxy/path2/", "/proxy/path3/"] + targets = [ + "http://127.0.0.1:9900", + "http://127.0.0.1:9090", + "http://127.0.0.1:9999", + ] + datas = [{"test": "test1"}, {}, {"test": "test2"}] + + expected_output = { + routespecs[0] + + "/": { + "routespec": routespecs[0] + "/", + "target": targets[0], + "data": datas[0], + }, + routespecs[1]: { + "routespec": routespecs[1], + "target": targets[1], + "data": datas[1], + }, + routespecs[2]: { + "routespec": routespecs[2], + "target": targets[2], + "data": datas[2], + }, + } + + for target in targets: + launch_backend(urlparse(target).port) + + await wait_for_services([proxy.public_url] + targets) + + futures = [] + for routespec, target, data in zip(routespecs, targets, datas): + f = proxy.add_route(routespec, target, copy.copy(data)) + futures.append(f) + if futures: + await asyncio.gather(*futures) + + routes = await proxy.get_all_routes() + try: + for route_key in routes.keys(): + del routes[route_key]["data"]["last_activity"] # CHP + except KeyError: + pass + + assert_equal(routes, expected_output) + + +async def test_host_origin_headers(proxy, launch_backend): + routespec = "/user/username/" + target = "http://127.0.0.1:9000" + data = {} + + proxy_url = urlparse(proxy.public_url) + traefik_port = proxy_url.port + traefik_host = proxy_url.hostname + default_backend_port = urlparse(target).port + launch_backend(default_backend_port) + + # wait for traefik to be reachable + await exponential_backoff( + utils.check_host_up_http, + "Traefik not reacheable", + url=proxy_url.geturl(), + ) + + # wait for backend to be reachable + await exponential_backoff( + utils.check_host_up_http, + "Backends not reacheable", + url=target, + ) + + # Add route to default_backend + await proxy.add_route(routespec, target, data) + + req_url = proxy.public_url.rstrip("/") + routespec + + expected_host_header = traefik_host + ":" + str(traefik_port) + expected_origin_header = proxy.public_url + routespec + + req = HTTPRequest( + req_url, + method="GET", + headers={"Host": expected_host_header, "Origin": expected_origin_header}, + validate_cert=False, + ) + resp = await AsyncHTTPClient().fetch(req) + + assert resp.headers["Host"] == expected_host_header + assert resp.headers["Origin"] == expected_origin_header + + +@pytest.mark.parametrize("username", ["zoe", "50fia", "秀樹", "~TestJH", "has@"]) +async def test_check_routes(proxy, username): + # fill out necessary attributes for check_routes + proxy.app = MockApp() + proxy.hub = proxy.app.hub + + users = {} + services = {} + # run initial check first, to ensure that `/` is in the routes + await proxy.check_routes(users, services) + routes = await proxy.get_all_routes() + assert sorted(routes) == ["/"] + + users[username] = test_user = MockUser(username) + spawner = test_user.spawners[""] + f = spawner.start() + if inspect.isawaitable(f): + await f + assert spawner.ready + assert spawner.active + await proxy.add_user(test_user, "") + + # check a valid route exists for user + routes = await proxy.get_all_routes() + before = sorted(routes) + assert test_user.proxy_spec in before + + # check if a route is removed when user deleted + await proxy.check_routes(users, services) + await proxy.delete_user(test_user) + routes = await proxy.get_all_routes() + during = sorted(routes) + assert test_user.proxy_spec not in during + + # check if a route exists for user + await proxy.check_routes(users, services) + routes = await proxy.get_all_routes() + after = sorted(routes) + assert test_user.proxy_spec in after + + # check that before and after state are the same + assert_equal(before, after) + + +async def test_websockets(proxy, launch_backend): + routespec = "/user/username/" + target = "http://127.0.0.1:9000" + data = {} + + proxy_url = urlparse(proxy.public_url) + proxy_url.port + proxy_url.hostname + default_backend_port = urlparse(target).port + launch_backend(default_backend_port, "ws") + + # wait for traefik to be reachable + await exponential_backoff( + utils.check_host_up_http, + "Traefik cannot be reached", + url=proxy_url.geturl(), + ) + + # wait for backend to be reachable + await exponential_backoff( + utils.check_host_up_http, + "Backends cannot be reached", + url=target, + ) + + # Add route to default_backend + await proxy.add_route(routespec, target, data) + + if proxy.is_https: + kwargs = {'ssl': ssl._create_unverified_context()} + scheme = "wss://" + else: + kwargs = {} + scheme = "ws://" + req_url = scheme + proxy_url.netloc + routespec + + # Don't validate the ssl certificate, it's self-signed by traefik + print(f"Connecting with websockets to {req_url}") + async with websockets.connect(req_url, **kwargs) as websocket: + port = await websocket.recv() + + assert port == str(default_backend_port) From bb7c07ed09896bc44fe0fad55754ccc318fae5aa Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 6 Mar 2023 09:18:48 +0100 Subject: [PATCH 5/7] test: when creating external proxy, don't call proxy.start --- tests/conftest.py | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 76b38054..37270016 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -144,8 +144,7 @@ def auth_etcd_proxy(enable_auth_in_etcd, launch_etcd_proxy): yield launch_etcd_proxy -@pytest.fixture -async def launch_etcd_proxy(): +def _make_etcd_proxy(**kwargs): grpc_options = [ ("grpc.ssl_target_name_override", "localhost"), ("grpc.default_authority", "localhost"), @@ -160,10 +159,15 @@ async def launch_etcd_proxy(): etcd_client_ca_cert=f"{config_files}/fake-ca-cert.crt", etcd_insecure_skip_verify=True, check_route_timeout=45, - should_start=True, grpc_options=grpc_options, + **kwargs, ) + return proxy + +@pytest.fixture +async def launch_etcd_proxy(): + proxy = _make_etcd_proxy(should_start=True) await proxy.start() yield proxy await proxy.stop() @@ -257,7 +261,6 @@ async def external_consul_proxy(launch_consul, configure_consul, launch_traefik_ async def auth_external_consul_proxy( launch_consul_acl, configure_consul_auth, launch_traefik_consul_auth ): - print("creating proxy") proxy = TraefikConsulProxy( public_url=Config.public_url, consul_url=f"http://127.0.0.1:{Config.consul_auth_port}", @@ -287,9 +290,12 @@ async def external_etcd_proxy(launch_etcd, configure_etcd, launch_traefik_etcd): @pytest.fixture async def auth_external_etcd_proxy( - enable_auth_in_etcd, launch_traefik_etcd_auth, launch_etcd_proxy + enable_auth_in_etcd, + launch_traefik_etcd_auth, ): - yield launch_etcd_proxy + proxy = _make_etcd_proxy(should_start=False) + yield proxy + proxy.etcd.close() ######################################################################### @@ -385,7 +391,12 @@ def configure_etcd_auth(): def _config_etcd(*extra_args): - data_store_cmd = ("etcdctl", "txn") + extra_args + common_args = [ + "--insecure-skip-tls-verify=true", + "--insecure-transport=false", + "--debug", + ] + data_store_cmd = ("etcdctl", "txn") + extra_args + tuple(common_args) # Load a pre-baked dynamic configuration into the etcd store. # This essentially puts authentication on the traefik api handler. with open(os.path.join(config_files, "traefik_etcd_txns.txt")) as fd: @@ -410,10 +421,10 @@ def enable_auth_in_etcd(launch_etcd_auth): "--insecure-transport=false", "--debug", ] - subprocess.call( + subprocess.check_call( ["etcdctl", "user", "add", f"{user}:{pw}"] + common_args, env=Config.etcdctl_env ) - subprocess.call( + subprocess.check_call( ["etcdctl", "user", "grant-role", user, "root"] + common_args, env=Config.etcdctl_env, ) From a3b5f88af08211502d21f01dce6eda361c4eb52c Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 6 Mar 2023 09:53:38 +0100 Subject: [PATCH 6/7] more consistent etcd setup remove unused fixtures, consolidate etcd args, use ssl args everywhere auth is enabled, not just most places --- tests/conftest.py | 76 +++++++++++++++++++---------------------------- 1 file changed, 30 insertions(+), 46 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 37270016..a4107e9b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -123,61 +123,51 @@ async def no_auth_etcd_proxy(launch_etcd): Fixture returning a configured TraefikEtcdProxy. No etcd authentication. """ - proxy = TraefikEtcdProxy( - public_url=Config.public_url, - traefik_api_password=Config.traefik_api_pass, - traefik_api_username=Config.traefik_api_user, - check_route_timeout=45, - should_start=True, - ) + proxy = _make_etcd_proxy(auth=False) await proxy.start() yield proxy await proxy.stop() @pytest.fixture -def auth_etcd_proxy(enable_auth_in_etcd, launch_etcd_proxy): +async def auth_etcd_proxy(enable_auth_in_etcd): """ Fixture returning a configured TraefikEtcdProxy Etcd has credentials set up """ - yield launch_etcd_proxy + proxy = _make_etcd_proxy(auth=True) + await proxy.start() + yield proxy + await proxy.stop() -def _make_etcd_proxy(**kwargs): +def _make_etcd_proxy(auth=False, **extra_kwargs): grpc_options = [ ("grpc.ssl_target_name_override", "localhost"), ("grpc.default_authority", "localhost"), ] - proxy = TraefikEtcdProxy( + kwargs = dict( public_url=Config.public_url, traefik_api_password=Config.traefik_api_pass, traefik_api_username=Config.traefik_api_user, - etcd_url="https://localhost:2379", - etcd_username=Config.etcd_user, - etcd_password=Config.etcd_password, - etcd_client_ca_cert=f"{config_files}/fake-ca-cert.crt", - etcd_insecure_skip_verify=True, check_route_timeout=45, - grpc_options=grpc_options, - **kwargs, ) + if auth: + kwargs.update( + dict( + grpc_options=grpc_options, + etcd_url="https://localhost:2379", + etcd_client_ca_cert=f"{config_files}/fake-ca-cert.crt", + etcd_insecure_skip_verify=True, + etcd_username=Config.etcd_user, + etcd_password=Config.etcd_password, + ) + ) + kwargs.update(extra_kwargs) + proxy = TraefikEtcdProxy(**kwargs) return proxy -@pytest.fixture -async def launch_etcd_proxy(): - proxy = _make_etcd_proxy(should_start=True) - await proxy.start() - yield proxy - await proxy.stop() - - -@pytest.fixture(params=["no_auth_etcd_proxy", "auth_etcd_proxy"]) -def etcd_proxy(request): - return request.getfixturevalue(request.param) - - @pytest.fixture(autouse=True) def traitlets_log(): """Setup traitlets logger at debug-level @@ -276,13 +266,7 @@ async def auth_external_consul_proxy( @pytest.fixture async def external_etcd_proxy(launch_etcd, configure_etcd, launch_traefik_etcd): - proxy = TraefikEtcdProxy( - public_url=Config.public_url, - traefik_api_password=Config.traefik_api_pass, - traefik_api_username=Config.traefik_api_user, - check_route_timeout=45, - should_start=False, - ) + proxy = _make_etcd_proxy(auth=False, should_start=False) await proxy._wait_for_static_config() yield proxy proxy.etcd.close() @@ -293,7 +277,8 @@ async def auth_external_etcd_proxy( enable_auth_in_etcd, launch_traefik_etcd_auth, ): - proxy = _make_etcd_proxy(should_start=False) + proxy = _make_etcd_proxy(auth=True, should_start=False) + await proxy._wait_for_static_config() yield proxy proxy.etcd.close() @@ -387,16 +372,15 @@ def configure_etcd(): @pytest.fixture def configure_etcd_auth(): """Load traefik api rules into the etcd kv store, with authentication""" - yield _config_etcd("--user=" + Config.etcd_user + ":" + Config.etcd_password) + yield _config_etcd( + "--user=" + Config.etcd_user + ":" + Config.etcd_password, + "--insecure-skip-tls-verify=true", + "--insecure-transport=false", + ) def _config_etcd(*extra_args): - common_args = [ - "--insecure-skip-tls-verify=true", - "--insecure-transport=false", - "--debug", - ] - data_store_cmd = ("etcdctl", "txn") + extra_args + tuple(common_args) + data_store_cmd = ("etcdctl", "txn", "--debug") + extra_args # Load a pre-baked dynamic configuration into the etcd store. # This essentially puts authentication on the traefik api handler. with open(os.path.join(config_files, "traefik_etcd_txns.txt")) as fd: From de2ab54794908acfe30bb3501caca352edaf8428 Mon Sep 17 00:00:00 2001 From: Min RK Date: Mon, 6 Mar 2023 11:03:03 +0100 Subject: [PATCH 7/7] resolve dependencies in etcd fixtures ensures they are run in the right order --- tests/conftest.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index a4107e9b..fd1b9ee4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -265,7 +265,7 @@ async def auth_external_consul_proxy( @pytest.fixture -async def external_etcd_proxy(launch_etcd, configure_etcd, launch_traefik_etcd): +async def external_etcd_proxy(launch_traefik_etcd): proxy = _make_etcd_proxy(auth=False, should_start=False) await proxy._wait_for_static_config() yield proxy @@ -274,7 +274,6 @@ async def external_etcd_proxy(launch_etcd, configure_etcd, launch_traefik_etcd): @pytest.fixture async def auth_external_etcd_proxy( - enable_auth_in_etcd, launch_traefik_etcd_auth, ): proxy = _make_etcd_proxy(auth=True, should_start=False) @@ -299,7 +298,7 @@ def launch_traefik_file(): @pytest.fixture -def launch_traefik_etcd(): +def launch_traefik_etcd(launch_etcd, configure_etcd): env = Config.etcdctl_env proc = _launch_traefik_cli("--providers.etcd", env=env) yield proc @@ -307,8 +306,10 @@ def launch_traefik_etcd(): @pytest.fixture -def launch_traefik_etcd_auth(configure_etcd_auth): +def launch_traefik_etcd_auth(launch_etcd_auth, configure_etcd_auth): extra_args = ( + "--providers.etcd.tls.insecureSkipVerify=true", + "--providers.etcd.tls.ca=" + f"{config_files}/fake-ca-cert.crt", "--providers.etcd.username=" + Config.etcd_user, "--providers.etcd.password=" + Config.etcd_password, ) @@ -364,13 +365,13 @@ def _launch_traefik(*extra_args, env=None): @pytest.fixture -def configure_etcd(): +def configure_etcd(launch_etcd): """Load traefik api rules into the etcd kv store""" yield _config_etcd() @pytest.fixture -def configure_etcd_auth(): +def configure_etcd_auth(launch_etcd_auth, enable_auth_in_etcd): """Load traefik api rules into the etcd kv store, with authentication""" yield _config_etcd( "--user=" + Config.etcd_user + ":" + Config.etcd_password, @@ -459,7 +460,11 @@ async def launch_etcd_auth(): ) try: await _wait_for_etcd( - "--insecure-skip-tls-verify=true", "--insecure-transport=false", "--debug" + "--user", + f"{Config.etcd_user}:{Config.etcd_password}", + "--insecure-skip-tls-verify=true", + "--insecure-transport=false", + "--debug", ) yield etcd_proc finally: