Merge pull request #865 from consideRatio/pr/escape-slug

Add escape_slug as a utility function alongside safe_slug

kubespawner/proxy.py

@@ -12,6 +12,7 @@
 from .clients import load_config, shared_client
 from .objects import make_ingress
 from .reflector import ResourceReflector
+from .slugs import escape_slug
 from .utils import generate_hashed_slug
 
 
@@ -360,6 +361,10 @@ def __init__(self, *args, **kwargs):
         asyncio.ensure_future(self.endpoint_reflector.start())
 
     def _safe_name_for_routespec(self, routespec):
+        # FIXME: escape_slug isn't exactly as whats done here, because we aren't
+        #        calling .lower(), it may have been fine to just transition to
+        #        escape_slug though, but its wasn't obvious a safe change so it
+        #        wasn't done.
         safe_chars = set(string.ascii_lowercase + string.digits)
         safe_name = generate_hashed_slug(
             'jupyter-'
@@ -369,28 +374,18 @@ def _safe_name_for_routespec(self, routespec):
         return safe_name
 
     def _expand_user_properties(self, template, routespec, data):
-        # Make sure username and servername match the restrictions for DNS labels
-        # Note: '-' is not in safe_chars, as it is being used as escape character
-        safe_chars = set(string.ascii_lowercase + string.digits)
-
         raw_servername = data.get('server_name') or ''
-        safe_servername = escapism.escape(
-            raw_servername, safe=safe_chars, escape_char='-'
-        ).lower()
+        safe_servername = escape_slug(raw_servername)
 
         hub_namespace = self._namespace_default()
         if hub_namespace == "default":
             hub_namespace = "user"
 
         raw_username = data.get('user') or ''
-        safe_username = escapism.escape(
-            raw_username, safe=safe_chars, escape_char='-'
-        ).lower()
+        safe_username = escape_slug(raw_username)
 
         raw_servicename = data.get('services') or ''
-        safe_servicename = escapism.escape(
-            raw_servicename, safe=safe_chars, escape_char='-'
-        ).lower()
+        safe_servicename = escape_slug(raw_servicename)
 
         raw_routespec = routespec
         safe_routespec = self._safe_name_for_routespec(routespec)

kubespawner/slugs.py

@@ -10,6 +10,8 @@
 import re
 import string
 
+import escapism
+
 _alphanum = tuple(string.ascii_letters + string.digits)
 _alpha_lower = tuple(string.ascii_lowercase)
 _alphanum_lower = tuple(string.ascii_lowercase + string.digits)
@@ -26,6 +28,19 @@
 # length of hash suffix
 _hash_length = 8
 
+# Make sure username and servername match the restrictions for DNS labels
+# Note: '-' is not in safe_chars, as it is being used as escape character
+_escape_slug_safe_chars = set(string.ascii_lowercase + string.digits)
+
+
+def escape_slug(name):
+    """Generate a slug with the legacy system, safe_slug is preferred."""
+    return escapism.escape(
+        name,
+        safe=_escape_slug_safe_chars,
+        escape_char='-',
+    ).lower()
+
 
 def _is_valid_general(
     s, starts_with=None, ends_with=None, pattern=None, min_length=None, max_length=None

kubespawner/spawner.py

@@ -17,7 +17,6 @@
 from typing import Optional, Tuple, Type
 from urllib.parse import urlparse
 
-import escapism
 import jupyterhub
 from jinja2 import ChoiceLoader, Environment, FileSystemLoader, PackageLoader
 from jupyterhub.spawner import Spawner
@@ -50,7 +49,7 @@
     make_service,
 )
 from .reflector import ResourceReflector
-from .slugs import is_valid_label, multi_slug, safe_slug
+from .slugs import escape_slug, is_valid_label, multi_slug, safe_slug
 from .utils import recursive_format, recursive_update
 
 
@@ -2015,17 +2014,11 @@ def _set_deprecated(name, new_name, version, self, value):
     del _deprecated_name
 
     def _expand_user_properties(self, template, slug_scheme=None):
-        # Make sure username and servername match the restrictions for DNS labels
-        # Note: '-' is not in safe_chars, as it is being used as escape character
         if slug_scheme is None:
             slug_scheme = self.slug_scheme
 
-        safe_chars = set(string.ascii_lowercase + string.digits)
-
         raw_servername = self.name or ''
-        escaped_servername = escapism.escape(
-            raw_servername, safe=safe_chars, escape_char='-'
-        ).lower()
+        escaped_servername = escape_slug(raw_servername)
 
         # TODO: measure string template?
         # for object names, max is 255, so very unlikely to exceed
@@ -2066,9 +2059,7 @@ def _expand_user_properties(self, template, slug_scheme=None):
         if hub_namespace == "default":
             hub_namespace = "user"
 
-        escaped_username = escapism.escape(
-            self.user.name, safe=safe_chars, escape_char='-'
-        ).lower()
+        escaped_username = escape_slug(self.user.name)
 
         if slug_scheme == "safe":
             username = safe_username