From 223115834d2edbaeb8ebb0e545b1f3e23c5803a9 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Fri, 18 Oct 2024 10:29:58 +0200 Subject: [PATCH] Add escape_slug as a utility function alongside safe_slug By providing this, it becomes easier to test and compare slug changes. --- kubespawner/proxy.py | 21 ++++++++------------- kubespawner/slugs.py | 15 +++++++++++++++ kubespawner/spawner.py | 15 +++------------ 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/kubespawner/proxy.py b/kubespawner/proxy.py index e765a103..330d998b 100644 --- a/kubespawner/proxy.py +++ b/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) diff --git a/kubespawner/slugs.py b/kubespawner/slugs.py index 406eea40..2cf49b04 100644 --- a/kubespawner/slugs.py +++ b/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 diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py index 00bb2f28..e7f5564e 100644 --- a/kubespawner/spawner.py +++ b/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