diff --git a/jupyterhub/app.py b/jupyterhub/app.py index 930b1977..52451a2c 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1182,6 +1182,7 @@ class JupyterHub(Application): app=self, log=self.log, hub=self.hub, + host_routing=bool(self.subdomain_host), ssl_cert=self.ssl_cert, ssl_key=self.ssl_key, ) diff --git a/jupyterhub/proxy.py b/jupyterhub/proxy.py index 48d4f1b3..9fa5e3a9 100644 --- a/jupyterhub/proxy.py +++ b/jupyterhub/proxy.py @@ -3,10 +3,12 @@ # Copyright (c) IPython Development Team. # Distributed under the terms of the Modified BSD License. +from collections import namedtuple import json import os from subprocess import Popen import time +from urllib.parse import quote from tornado import gen from tornado.httpclient import AsyncHTTPClient, HTTPRequest @@ -26,6 +28,30 @@ from . import utils from .utils import url_path_join +class RouteSpec(namedtuple('RouteSpec', ['path', 'host'])): + def __new__(cls, path, *, host=''): + # give host a default value + if isinstance(path, cls) and host == '': + # RouteSpec(routespec) makes a copy + path, host = path + + @classmethod + def as_routespec(cls, routespec): + """Ensure a RouteSpec or str is a RouteSpec + + Allows string arguments to be accepted anywhere RouteSpecs are accepted. + """ + if isinstance(routespec, str): + return cls(routespec) + else: + return routespec + + def __repr__(self): + if not self.host: + return '%s(path=%r)' % (self.__class__.__name__, self.path) + else: + return super().__repr__() + class Proxy(LoggingConfigurable): """Base class for configurable proxies that JupyterHub can use.""" @@ -35,6 +61,7 @@ class Proxy(LoggingConfigurable): public_url = Unicode() ssl_key = Unicode() ssl_cert = Unicode() + host_routing = Bool() should_start = Bool(True, config=True, help="""Should the Hub start the proxy. @@ -61,8 +88,8 @@ class Proxy(LoggingConfigurable): """Add a route to the proxy. Args: - routespec (str): A specification for which this route will be matched. - Could be either a url_prefix or a fqdn. + routespec (RouteSpec or str): A specification for which this route will be matched. + If a string, should be treated as RouteSpec(routespec). target (str): A URL that will be the target of this route. data (dict): A JSONable dict that will be associated with this route, and will be returned when retrieving information about this route. @@ -85,7 +112,8 @@ class Proxy(LoggingConfigurable): """Return the route info for a given routespec. Args: - routespec (str): The route specification that was used to add this routespec + routespec (RouteSpec or str): The route specification that was used to add this route. + If a string, should be treated as RouteSpec(routespec). Returns: result (dict): with the following keys: @@ -95,7 +123,10 @@ class Proxy(LoggingConfigurable): route. None: if there are no routes matching the given routespec """ - pass + # default implementation relies on get_all_routes + routespec = RouteSpec.as_routespec(routespec) + routes = yield self.get_all_routes() + return routes.get(routespec) @gen.coroutine def get_all_routes(self): @@ -118,11 +149,11 @@ class Proxy(LoggingConfigurable): "Service %s does not have an http endpoint to add to the proxy.", service.name) self.log.info("Adding service %s to proxy %s => %s", - service.name, service.proxy_path, service.server.host, + service.name, service.proxy_spec, service.server.host, ) yield self.add_route( - service.proxy_path, + service.proxy_spec, service.server.host, {'service': service.name} ) @@ -131,13 +162,13 @@ class Proxy(LoggingConfigurable): def delete_service(self, service, client=None): """Remove a service's server from the proxy table.""" self.log.info("Removing service %s from proxy", service.name) - yield self.delete_route(service.proxy_path) + yield self.delete_route(service.proxy_spec) @gen.coroutine def add_user(self, user, client=None): """Add a user's server to the proxy table.""" self.log.info("Adding user %s to proxy %s => %s", - user.name, user.proxy_path, user.server.host, + user.name, user.proxy_spec, user.server.host, ) if user.spawn_pending: @@ -145,7 +176,7 @@ class Proxy(LoggingConfigurable): "User %s's spawn is pending, shouldn't be added to the proxy yet!", user.name) yield self.add_route( - user.proxy_path, + user.proxy_spec, user.server.host, {'user': user.name} ) @@ -154,7 +185,7 @@ class Proxy(LoggingConfigurable): def delete_user(self, user): """Remove a user's server from the proxy table.""" self.log.info("Removing user %s from proxy", user.name) - yield self.delete_route(user.proxy_path) + yield self.delete_route(user.proxy_spec) @gen.coroutine def add_all_services(self, service_dict): @@ -236,7 +267,7 @@ class Proxy(LoggingConfigurable): self.log.info("Setting up routes on new proxy") yield self.add_all_users(self.app.users) yield self.add_all_services(self.app.services) - self.log.info("New proxy back up, and good to go") + self.log.info("New proxy back up and good to go") class ConfigurableHTTPProxy(Proxy): @@ -361,10 +392,37 @@ class ConfigurableHTTPProxy(Proxy): ) yield self.start() yield self.restore_routes() + + def _routespec_to_chp_path(self, routespec): + """Turn a RouteSpec into a CHP API path""" + path = routespec.path + if routespec.host: + if not self.host_routing: + raise RuntimeError("Adding route with a host") + path = '/' + url_path_join(routespec.host, path) + return path + + def _routespec_from_chp_path(self, chp_path): + """Turn a CHP route into a RouteSpec + + In the JSON API, CHP route keys are unescaped, + so re-escape them to raw URLs. + """ + # chp stores routes in unescaped form. + # restore escaped-form we created it with. + path = quote(chp_path, safe='@/') + host = '' + if self.host_routing: + host, *rest = path.lstrip('/').split('/', 1) + path = '/' + ''.join(rest) + return RouteSpec(path, host=host) + def api_request(self, path, method='GET', body=None, client=None): """Make an authenticated API request of the proxy.""" client = client or AsyncHTTPClient() + if isinstance(path, RouteSpec): + path = self._routespec_to_chp_path(path) url = url_path_join(self.api_url, 'api/routes', path) if isinstance(body, dict): @@ -380,6 +438,8 @@ class ConfigurableHTTPProxy(Proxy): return client.fetch(req) def add_route(self, routespec, target, data=None): + # ensure RouteSpec object + routespec = RouteSpec.as_routespec(routespec) body = data or {} body['target'] = target return self.api_request(routespec, @@ -388,29 +448,27 @@ class ConfigurableHTTPProxy(Proxy): ) def delete_route(self, routespec): + routespec = RouteSpec.as_routespec(routespec) return self.api_request(routespec, method='DELETE') def _reformat_routespec(self, routespec, chp_data): """Reformat CHP data format to JupyterHub's proxy API.""" + # ensure RouteSpec object target = chp_data.pop('target') return { 'routespec': routespec, 'target': target, 'data': chp_data, } - - @gen.coroutine - def get_route(self, routespec): - chp_data = yield self.api_request(routespec, method='DELETE') - return self._reformat_routespec(routespec, chp_data) - + @gen.coroutine def get_all_routes(self, client=None): """Fetch the proxy's routes.""" resp = yield self.api_request('', client=client) chp_routes = json.loads(resp.body.decode('utf8', 'replace')) all_routes = {} - for routespec, chp_data in chp_routes.items(): + for chp_path, chp_data in chp_routes.items(): + routespec = self._routespec_from_chp_path(chp_path) all_routes[routespec] = self._reformat_routespec( routespec, chp_data) return all_routes diff --git a/jupyterhub/services/service.py b/jupyterhub/services/service.py index b6134a45..355b2a02 100644 --- a/jupyterhub/services/service.py +++ b/jupyterhub/services/service.py @@ -52,6 +52,7 @@ from traitlets.config import LoggingConfigurable from .. import orm from ..objects import Server +from ..proxy import RouteSpec from ..traitlets import Command from ..spawner import LocalProcessSpawner, set_user_setuid from ..utils import url_path_join @@ -243,13 +244,13 @@ class Service(LoggingConfigurable): return url_path_join(self.base_url, 'services', self.name + '/') @property - def proxy_path(self): + def proxy_spec(self): if not self.server: return '' if self.domain: - return url_path_join('/' + self.domain, self.server.base_url) + return RouteSpec(path=self.server.base_url, host=self.domain) else: - return self.server.base_url + return RouteSpec(self.server.base_url) def __repr__(self): return "<{cls}(name={name}{managed})>".format( diff --git a/jupyterhub/tests/test_proxy.py b/jupyterhub/tests/test_proxy.py index befb0291..013d901e 100644 --- a/jupyterhub/tests/test_proxy.py +++ b/jupyterhub/tests/test_proxy.py @@ -4,7 +4,7 @@ import json import os from queue import Queue from subprocess import Popen -from urllib.parse import urlparse, unquote +from urllib.parse import urlparse from traitlets.config import Config @@ -15,6 +15,40 @@ from .mocking import MockHub from .test_api import api_request from ..utils import wait_for_http_server, url_path_join as ujoin +from jupyterhub.proxy import RouteSpec + +def test_routespec(): + with pytest.raises(TypeError): + RouteSpec() + + spec = RouteSpec('/test') + assert spec.host == '' + assert spec.path == '/test' + + assert 'path=%r' % spec.path in repr(spec) + assert 'host' not in repr(spec) + + spec = RouteSpec('/test2', host='myhost') + assert spec.path == '/test2' + assert spec.host == 'myhost' + + assert 'path=%r' % spec.path in repr(spec) + assert 'host=%r' % spec.host in repr(spec) + + copyspec = RouteSpec(spec) + assert copyspec.path == '/test2' + assert copyspec.host == 'myhost' + assert copyspec == spec + +def test_as_routespec(): + spec = RouteSpec('/test', host='myhost') + as_spec = RouteSpec.as_routespec(spec) + assert as_spec is spec + + spec2 = RouteSpec.as_routespec('/path') + assert isinstance(spec2, RouteSpec) + assert spec2.path == '/path' + def test_external_proxy(request, io_loop): @@ -64,7 +98,7 @@ def test_external_proxy(request, io_loop): # test if api service has a root route '/' routes = io_loop.run_sync(app.proxy.get_all_routes) - assert list(routes.keys()) == ['/'] + assert list(routes.keys()) == [RouteSpec('/')] # add user to the db and start a single user server name = 'river' @@ -75,11 +109,12 @@ def test_external_proxy(request, io_loop): routes = io_loop.run_sync(app.proxy.get_all_routes) # sets the desired path result - user_path = unquote(ujoin(app.base_url, 'user/river')) + user_path = ujoin(app.base_url, 'user/river') + host = '' if app.subdomain_host: - domain = urlparse(app.subdomain_host).hostname - user_path = '/%s.%s' % (name, domain) + user_path - assert sorted(routes.keys()) == ['/', user_path] + host = '%s.%s' % (name, urlparse(app.subdomain_host).hostname) + user_spec = RouteSpec(user_path, host=host) + assert sorted(routes.keys()) == [RouteSpec('/'), user_spec] # teardown the proxy and start a new one in the same place proxy.terminate() @@ -88,7 +123,7 @@ def test_external_proxy(request, io_loop): routes = io_loop.run_sync(app.proxy.get_all_routes) - assert list(routes.keys()) == ['/'] + assert list(routes.keys()) == [RouteSpec('/')] # poke the server to update the proxy r = api_request(app, 'proxy', method='post') @@ -96,7 +131,7 @@ def test_external_proxy(request, io_loop): # check that the routes are correct routes = io_loop.run_sync(app.proxy.get_all_routes) - assert sorted(routes.keys()) == ['/', user_path] + assert sorted(routes.keys()) == [RouteSpec('/'), user_spec] # teardown the proxy, and start a new one with different auth and port proxy.terminate() @@ -135,7 +170,7 @@ def test_external_proxy(request, io_loop): # check that the routes are correct routes = io_loop.run_sync(app.proxy.get_all_routes) - assert sorted(routes.keys()) == ['/', user_path] + assert sorted(routes.keys()) == [RouteSpec('/'), user_spec] @pytest.mark.parametrize("username, endpoints", [ @@ -156,18 +191,18 @@ def test_check_routes(app, io_loop, username, endpoints): # check a valid route exists for user test_user = app.users[username] before = sorted(io_loop.run_sync(app.proxy.get_all_routes)) - assert unquote(test_user.proxy_path) in before + assert test_user.proxy_spec in before # check if a route is removed when user deleted io_loop.run_sync(lambda: app.proxy.check_routes(app.users, app._service_map)) io_loop.run_sync(lambda: proxy.delete_user(test_user)) during = sorted(io_loop.run_sync(app.proxy.get_all_routes)) - assert unquote(test_user.proxy_path) not in during + assert test_user.proxy_spec not in during # check if a route exists for user io_loop.run_sync(lambda: app.proxy.check_routes(app.users, app._service_map)) after = sorted(io_loop.run_sync(app.proxy.get_all_routes)) - assert unquote(test_user.proxy_path) in after + assert test_user.proxy_spec in after # check that before and after state are the same assert before == after diff --git a/jupyterhub/user.py b/jupyterhub/user.py index a504ed13..4394ffb7 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -15,7 +15,7 @@ from . import orm from .objects import Server from traitlets import HasTraits, Any, Dict, observe, default from .spawner import LocalProcessSpawner - +from .proxy import RouteSpec class UserDict(dict): """Like defaultdict, but for users @@ -169,11 +169,11 @@ class User(HasTraits): return quote(self.name, safe='@') @property - def proxy_path(self): + def proxy_spec(self): if self.settings.get('subdomain_host'): - return url_path_join('/' + self.domain, self.base_url) + return RouteSpec(path=self.base_url, host=self.domain) else: - return self.base_url + return RouteSpec(path=self.base_url) @property def domain(self):