Merge pull request #1871 from minrk/fix-hub-bind-url

fix and test constructing objects from bind_url

docs/source/changelog.md

@@ -47,12 +47,13 @@ and tornado < 5.0.
 
       jupyterhub &>> /var/log/jupyterhub.log
 
-- deprecate `JupyterHub.ip`, `JupyterHub.port`, `JupyterHub.base_url` config in favor of single `JupyterHub.bind_url` config.
+- Add `JupyterHub.bind_url` config for setting the full bind URL of the proxy.
-- deprecate `JupyterHub.hub_ip`, `JupyterHub.hub_port` config
+  Sets ip, port, base_url all at once.
-  in favor of single `JupyterHub.hub_bind_url` config.
+- Add `JupyterHub.hub_bind_url` for setting the full host+port of the Hub.
   `hub_bind_url` supports unix domain sockets, e.g.
   `unix+http://%2Fsrv%2Fjupytrehub.sock`
-- deprecate `JupyterHub.hub_connect_port` config in favor of `JupyterHub.hub_connect_url`.
+- Deprecate `JupyterHub.hub_connect_port` config in favor of `JupyterHub.hub_connect_url`. `hub_connect_ip` is not deprecated
+  and can still be used in the common case where only the ip address of the hub differs from the bind ip.
 
 #### Added
 

jupyterhub/app.py

@@ -359,7 +359,6 @@ class JupyterHub(Application):
     @default('base_url')
     def _default_base_url(self):
         # call validate to ensure leading/trailing slashes
-        print(self.bind_url)
         return JupyterHub.base_url.validate(self, urlparse(self.bind_url).path)
 
     subdomain_host = Unicode('',
@@ -451,8 +450,11 @@ class JupyterHub(Application):
         This is the internal port of the hub itself. It should never be accessed directly.
         See JupyterHub.port for the public port to use when accessing jupyterhub.
         It is rare that this port should be set except in cases of port conflict.
+
+        See also `hub_ip` for the ip and `hub_bind_url` for setting the full bind URL.
         """
     ).tag(config=True)
+
     hub_ip = Unicode('127.0.0.1',
         help="""The ip address for the Hub process to *bind* to.
 
@@ -460,7 +462,8 @@ class JupyterHub(Application):
         the proxy and user servers. You may need to set this to a public ip or '' for all
         interfaces if the proxy or user servers are in containers or on a different host.
 
-        See `hub_connect_ip` for cases where the bind and connect address should differ.
+        See `hub_connect_ip` for cases where the bind and connect address should differ,
+        or `hub_bind_url` for setting the full bind URL.
         """
     ).tag(config=True)
 
@@ -492,10 +495,12 @@ class JupyterHub(Application):
         .. seealso::
             JupyterHub.hub_connect_ip
             JupyterHub.hub_bind_url
+
         .. versionadded:: 0.9
         """,
         config=True
     )
+
     hub_bind_url = Unicode(
         help="""
         The URL on which the Hub will listen.
@@ -1064,6 +1069,11 @@ class JupyterHub(Application):
             public_host=self.subdomain_host,
         )
         if self.hub_bind_url:
+            # ensure hub_prefix is set on bind_url
+            self.hub_bind_url = urlunparse(
+                urlparse(self.hub_bind_url)
+                ._replace(path=self.hub_prefix)
+            )
             hub_args['bind_url'] = self.hub_bind_url
         else:
             hub_args['ip'] = self.hub_ip
@@ -1081,6 +1091,11 @@ class JupyterHub(Application):
             )
 
         if self.hub_connect_url:
+            # ensure hub_prefix is on connect_url
+            self.hub_connect_url = urlunparse(
+                urlparse(self.hub_connect_url)
+                ._replace(path=self.hub_prefix)
+            )
             self.hub.connect_url = self.hub_connect_url
 
     async def init_users(self):

jupyterhub/objects.py

@@ -4,12 +4,12 @@
 # Distributed under the terms of the Modified BSD License.
 
 import socket
-from urllib.parse import urlparse
+from urllib.parse import urlparse, urlunparse
 import warnings
 
 from traitlets import (
     HasTraits, Instance, Integer, Unicode,
-    default, observe,
+    default, observe, validate,
 )
 from .traitlets import URLPrefix
 from . import orm
@@ -47,6 +47,28 @@ class Server(HasTraits):
             return self.url.replace(self._connect_ip, self.ip or '*', 1)
         return self.url
 
+    @observe('bind_url')
+    def _bind_url_changed(self, change):
+        urlinfo = urlparse(change.new)
+        self.proto = urlinfo.scheme
+        self.ip = urlinfo.hostname or ''
+        port = urlinfo.port
+        if port is None:
+            if self.proto == 'https':
+                port = 443
+            else:
+                port = 80
+        self.port = port
+
+    @validate('connect_url')
+    def _connect_url_add_prefix(self, proposal):
+        """Ensure connect_url includes base_url"""
+        urlinfo = urlparse(proposal.value)
+        if not urlinfo.path.startswith(self.base_url):
+            urlinfo = urlinfo._replace(path=self.base_url)
+            return urlunparse(urlinfo)
+        return proposal.value
+
     @property
     def _connect_ip(self):
         """The address to use when connecting to this server
@@ -83,16 +105,7 @@ class Server(HasTraits):
     @classmethod
     def from_url(cls, url):
         """Create a Server from a given URL"""
-        urlinfo = urlparse(url)
+        return cls(bind_url=url, base_url=urlparse(url).path)
-        proto = urlinfo.scheme
-        ip = urlinfo.hostname or ''
-        port = urlinfo.port
-        if not port:
-            if proto == 'https':
-                port = 443
-            else:
-                port = 80
-        return cls(proto=proto, ip=ip, port=port, base_url=urlinfo.path)
 
     @default('port')
     def _default_port(self):

jupyterhub/tests/test_objects.py

@@ -0,0 +1,70 @@
+"""Tests for basic object-wrappers"""
+
+import socket
+import pytest
+
+from jupyterhub.objects import Server
+
+
+@pytest.mark.parametrize(
+    'bind_url, attrs',
+    [
+        (
+            'http://abc:123',
+            {
+                'ip': 'abc',
+                'port': 123,
+                'host': 'http://abc:123',
+                'url': 'http://abc:123/x/',
+            }
+        ),
+        (
+            'https://abc',
+            {
+                'ip': 'abc',
+                'port': 443,
+                'proto': 'https',
+                'host': 'https://abc:443',
+                'url': 'https://abc:443/x/',
+            }
+        ),
+    ]
+)
+def test_bind_url(bind_url, attrs):
+    s = Server(bind_url=bind_url, base_url='/x/')
+    for attr, value in attrs.items():
+        assert getattr(s, attr) == value
+
+
+_hostname = socket.gethostname()
+
+
+@pytest.mark.parametrize(
+    'ip, port, attrs',
+    [
+        (
+            '', 123,
+            {
+                'ip': '',
+                'port': 123,
+                'host': 'http://{}:123'.format(_hostname),
+                'url': 'http://{}:123/x/'.format(_hostname),
+                'bind_url': 'http://*:123/x/',
+            }
+        ),
+        (
+            '127.0.0.1', 999,
+            {
+                'ip': '127.0.0.1',
+                'port': 999,
+                'host': 'http://127.0.0.1:999',
+                'url': 'http://127.0.0.1:999/x/',
+                'bind_url': 'http://127.0.0.1:999/x/',
+            }
+        ),
+    ]
+)
+def test_ip_port(ip, port, attrs):
+    s = Server(ip=ip, port=port, base_url='/x/')
+    for attr, value in attrs.items():
+        assert getattr(s, attr) == value