Merge pull request #162 from minrk/address

Respect ip address config in urls, don't serve dashboard by default

jupyterhub_traefik_proxy/proxy.py

@@ -22,12 +22,12 @@
 import os
 from os.path import abspath
 from subprocess import Popen, TimeoutExpired
-from urllib.parse import urlparse
+from urllib.parse import urlparse, urlunparse
 
 from jupyterhub.proxy import Proxy
 from jupyterhub.utils import exponential_backoff, new_token, url_path_join
 from tornado.httpclient import AsyncHTTPClient
-from traitlets import Any, Bool, Dict, Integer, Unicode, default
+from traitlets import Any, Bool, Dict, Integer, Unicode, default, validate
 
 from . import traefik_utils
 
@@ -125,6 +125,23 @@ def get_is_https(self):
         # Check if we set https
         return urlparse(self.public_url).scheme == "https"
 
+    @validate("public_url", "traefik_api_url")
+    def _add_port(self, proposal):
+        url = proposal.value
+        parsed = urlparse(url)
+        if parsed.scheme not in ("http", "https"):
+            raise ValueError(
+                f"{self.__class__.__name__}.{proposal.trait.name} must be of the form http[s]://host:port/, got {url}"
+            )
+        if not parsed.port:
+            # ensure port is defined
+            if parsed.scheme == 'http':
+                parsed = parsed._replace(netloc=f'{parsed.hostname}:80')
+            elif parsed.scheme == 'https':
+                parsed = parsed._replace(netloc=f'{parsed.hostname}:443')
+            url = urlunparse(parsed)
+        return url
+
     traefik_cert_resolver = Unicode(
         config=True,
         help="""The traefik certificate Resolver to use for requesting certificates""",
@@ -146,31 +163,16 @@ async def _get_traefik_entrypoint(self):
                 return "web"
         import re
 
-        # FIXME: Adding '_wait_for_static_config' to get through 'external'
-        # tests. Would this be required in the 'real world'?
-        # Adding _wait_for_static_config to the 'external' conftests instead...
-        # await self._wait_for_static_config()
         resp = await self._traefik_api_request("/api/entrypoints")
         json_data = json.loads(resp.body)
         public_url = urlparse(self.public_url)
-        hub_port = public_url.port
-        if not hub_port:
-            # If the port is not specified, then use the default port
-            # according to the scheme (http, or https)
-            if public_url.scheme == 'http':
-                hub_port = 80
-            elif public_url.scheme == 'https':
-                hub_port = 443
-            else:
-                raise ValueError(
-                    f"Cannot discern public_url port from {self.public_url}!"
-                )
+
         # Traefik entrypoint format described at:-
         # https://doc.traefik.io/traefik/routing/entrypoints/#address
         entrypoint_re = re.compile('([^:]+)?:([0-9]+)/?(tcp|udp)?')
         for entrypoint in json_data:
             host, port, prot = entrypoint_re.match(entrypoint["address"]).groups()
-            if int(port) == hub_port:
+            if int(port) == public_url.port:
                 return entrypoint["name"]
         entrypoints = [entrypoint["address"] for entrypoint in json_data]
         raise ValueError(
@@ -357,15 +359,15 @@ async def _setup_traefik_static_config(self):
 
         entrypoints = {
             self.traefik_entrypoint: {
-                "address": f":{urlparse(self.public_url).port}",
+                "address": urlparse(self.public_url).netloc,
             },
             "enter_api": {
-                "address": f":{urlparse(self.traefik_api_url).port}",
+                "address": urlparse(self.traefik_api_url).netloc,
             },
         }
 
         self.static_config["entryPoints"] = entrypoints
-        self.static_config["api"] = {"dashboard": True}
+        self.static_config["api"] = {}
 
         self.log.info(f"Writing traefik static config: {self.static_config}")
 
@@ -389,7 +391,7 @@ async def _setup_traefik_dynamic_config(self):
                 "http": {
                     "routers": {
                         "route_api": {
-                            "rule": f"Host(`{api_url.hostname}`) && (PathPrefix(`{api_path}`) || PathPrefix(`/dashboard`))",
+                            "rule": f"Host(`{api_url.hostname}`) && PathPrefix(`{api_path}`)",
                             "entryPoints": ["enter_api"],
                             "service": "api@internal",
                             "middlewares": ["auth_api"],

tests/test_proxy.py

@@ -2,6 +2,8 @@
 
 import pytest
 
+from jupyterhub_traefik_proxy.proxy import TraefikProxy
+
 # Mark all tests in this file as asyncio and slow
 pytestmark = [pytest.mark.asyncio, pytest.mark.slow]
 
@@ -24,3 +26,14 @@
 )
 def proxy(request):
     return request.getfixturevalue(request.param)
+
+
+def test_default_port():
+    p = TraefikProxy(
+        public_url="http://127.0.0.1/", traefik_api_url="https://127.0.0.1/"
+    )
+    assert p.public_url == "http://127.0.0.1:80/"
+    assert p.traefik_api_url == "https://127.0.0.1:443/"
+
+    with pytest.raises(ValueError):
+        TraefikProxy(public_url="ftp://127.0.0.1:23/")

tests/test_traefik_api_auth.py

@@ -26,18 +26,15 @@ def proxy(request):
     [("api_admin", "admin", 200), ("api_admin", "1234", 401), ("", "", 401)],
 )
 async def test_traefik_api_auth(proxy, username, password, expected_rc):
-    traefik_api_url = proxy.traefik_api_url + "/api"
-
-    # Must have a trailing slash!
-    dashboard_url = proxy.traefik_api_url + "/dashboard/"
+    traefik_api_url = proxy.traefik_api_url + "/api/overview"
 
     async def api_login():
         try:
             if not username and not password:
                 resp = await AsyncHTTPClient().fetch(traefik_api_url)
             else:
                 resp = await AsyncHTTPClient().fetch(
-                    dashboard_url,
+                    traefik_api_url,
                     auth_username=username,
                     auth_password=password,
                 )
@@ -54,6 +51,7 @@ async def cmp_api_login():
         if rc == expected_rc:
             return True
         else:
+            print(f"{rc} != {expected_rc}")
             return False
 
     await exponential_backoff(cmp_api_login, "Traefik API not reacheable")