Merge pull request #856 from minrk/slug-value

fix some safe slug patterns

kubespawner/slugs.py

@@ -11,13 +11,14 @@
 import string
 
 _alphanum = tuple(string.ascii_letters + string.digits)
+_alpha_lower = tuple(string.ascii_lowercase)
 _alphanum_lower = tuple(string.ascii_lowercase + string.digits)
 _lower_plus_hyphen = _alphanum_lower + ('-',)
 
 # patterns _do not_ need to cover length or start/end conditions,
 # which are handled separately
-_object_pattern = re.compile(r'^[a-z0-9\.-]+$')
-_label_pattern = re.compile(r'^[a-z0-9\.-_]+$', flags=re.IGNORECASE)
+_object_pattern = re.compile(r'^[a-z0-9\-]+$')
+_label_pattern = re.compile(r'^[a-z0-9\.\-_]+$', flags=re.IGNORECASE)
 
 # match anything that's not lowercase alphanumeric (will be stripped, replaced with '-')
 _non_alphanum_pattern = re.compile(r'[^a-z0-9]+')
@@ -47,14 +48,22 @@ def _is_valid_general(
 
 
 def is_valid_object_name(s):
-    """is_valid check for object names"""
+    """is_valid check for object names
+
+    Ensures all strictest object rules apply,
+    satisfying both RFC 1035 and 1123 dns label name rules
+
+    - 63 characters
+    - starts with letter, ends with letter or number
+    - only lowercalse letters, numbers, '-'
+    """
     # object rules: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
     return _is_valid_general(
         s,
-        starts_with=_alphanum_lower,
+        starts_with=_alpha_lower,
         ends_with=_alphanum_lower,
         pattern=_object_pattern,
-        max_length=255,
+        max_length=63,
         min_length=1,
     )
 
@@ -79,27 +88,20 @@ def is_valid_default(s):
 
     Returns True if it's valid for _all_ our known uses
 
-    So we can more easily have a single is_valid check.
-
-    - object names have stricter character rules, but have longer max length
-    - labels have short max length, but allow uppercase
+    Currently, this is the same as is_valid_object_name,
+    which produces a valid DNS label under RFC1035 AND RFC 1123,
+    which is always also a valid label value.
     """
-    return _is_valid_general(
-        s,
-        starts_with=_alphanum_lower,
-        ends_with=_alphanum_lower,
-        pattern=_object_pattern,
-        min_length=1,
-        max_length=63,
-    )
+    return is_valid_object_name(s)
 
 
 def _extract_safe_name(name, max_length):
     """Generate safe substring of a name
 
     Guarantees:
 
-    - always starts and ends with a lowercase letter or number
+    - always starts with a lowercase letter
+    - always ends with a lowercase letter or number
     - never more than one hyphen in a row (no '--')
     - only contains lowercase letters, numbers, and hyphens
     - length at least 1 ('x' if other rules strips down to empty string)
@@ -111,6 +113,9 @@ def _extract_safe_name(name, max_length):
     safe_name = _non_alphanum_pattern.sub("-", name.lower())
     # truncate to max_length chars, strip '-' off ends
     safe_name = safe_name.lstrip("-")[:max_length].rstrip("-")
+    # ensure starts with lowercase letter
+    if safe_name and not safe_name.startswith(_alpha_lower):
+        safe_name = "x-" + safe_name[: max_length - 2]
     if not safe_name:
         # make sure it's non-empty
         safe_name = 'x'

kubespawner/spawner.py

@@ -165,7 +165,7 @@ def __init__(self, *args, **kwargs):
             # runs during test execution only
             if 'user' not in kwargs:
                 user = MockObject()
-                user.name = 'mock_name'
+                user.name = 'mock@name'
                 user.id = 'mock_id'
                 user.url = 'mock_url'
                 self.user = user

tests/test_slugs.py

@@ -12,7 +12,11 @@
         ("endswith-", "endswith---165f1166"),
         ("[email protected]", "user-email-com---0925f997"),
         ("user-_@_emailß.com", "user-email-com---7e3a7efd"),
+        ("has.dot", "has-dot---03e27fdf"),
+        ("z9", "z9"),
+        ("9z9", "x-9z9---224de202"),
         ("-start", "start---f587e2dc"),
+        ("üser", "ser---73506260"),
         ("username--servername", "username-servername---d957f1de"),
         ("start---f587e2dc", "start-f587e2dc---cc5bb9c9"),
         pytest.param("x" * 63, "x" * 63, id="x63"),
@@ -51,6 +55,13 @@ def test_safe_slug_max_length(max_length, length, expected):
     [
         ("", ""),
         ("x", "x"),
+        ("a-b", "a-b"),
+        ("9a", "9a"),
+        ("9.", "x-9---99a1b84b"),
+        ("AbC", "AbC"),
+        ("AbC.", "abc---dbe8c5d1"),
+        ("ab.c", "ab.c"),
+        ("[email protected]", "a-b-c---d648b243"),
         ("-x", "x---a4209624"),
         ("x-", "x---c8b60efc"),
         pytest.param("x" * 63, "x" * 63, id="x63"),

tests/test_spawner.py

@@ -27,7 +27,7 @@
 
 
 class MockUser(Mock):
-    name = 'fake'
+    name = '[email protected]'
     server = Server()
 
     def __init__(self, **kwargs):
@@ -272,7 +272,7 @@ async def test_spawn_start_in_different_namespace(
 async def test_spawn_enable_user_namespaces():
     user = MockUser()
     spawner = KubeSpawner(user=user, _mock=True, enable_user_namespaces=True)
-    assert spawner.namespace.endswith(f"-{user.escaped_name}")
+    assert spawner.namespace.endswith(f"-{safe_slug(user.name)}")
 
 
 async def test_spawn_start_enable_user_namespaces(
@@ -289,7 +289,7 @@ async def test_spawn_start_enable_user_namespaces(
 
     spawner = KubeSpawner(
         hub=hub,
-        user=MockUser(name="start"),
+        user=MockUser(name="start@test"),
         config=config,
         api_token="abc123",
         oauth_client_id="unused",
@@ -1488,7 +1488,7 @@ async def test_spawner_env():
     assert env["STATIC"] == "static"
     assert env["EXPANDED"] == f"{slug} (expanded)"
     assert env["ESCAPED"] == "{username}"
-    assert env["CALLABLE"] == "mock_name (callable)"
+    assert env["CALLABLE"] == "mock@name (callable)"
 
 
 async def test_jupyterhub_supplied_env():
@@ -1503,7 +1503,7 @@ async def test_jupyterhub_supplied_env():
     env = pod_manifest.spec.containers[0].env
 
     # Set via .environment, must be expanded
-    assert V1EnvVar("HELLO", "It's mock_name") in env
+    assert V1EnvVar("HELLO", "It's mock@name") in env
     # Set by JupyterHub itself, must not be expanded
     assert V1EnvVar("JUPYTERHUB_COOKIE_OPTIONS", json.dumps(cookie_options)) in env
 
@@ -1625,7 +1625,7 @@ async def test_pod_connect_ip(kube_ns, kube_client, config, hub_pod, hub):
 async def test_get_pvc_manifest():
     c = Config()
 
-    username = "mock_name"
+    username = "mock@name"
     slug = safe_slug(username)
     c.KubeSpawner.pvc_name_template = "user-{username}"
     c.KubeSpawner.storage_extra_labels = {"user": "{username}"}
@@ -1640,7 +1640,7 @@ async def test_get_pvc_manifest():
     assert manifest.metadata.name == f"user-{slug}"
     assert manifest.metadata.labels == {
         "user": slug,
-        "hub.jupyter.org/username": username,
+        "hub.jupyter.org/username": slug,
         "app.kubernetes.io/name": "jupyterhub",
         "app.kubernetes.io/managed-by": "kubespawner",
         "app.kubernetes.io/component": "singleuser-server",