diff --git a/kubespawner/slugs.py b/kubespawner/slugs.py index ee3a283d..406eea40 100644 --- a/kubespawner/slugs.py +++ b/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,19 +88,11 @@ 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): @@ -99,7 +100,8 @@ def _extract_safe_name(name, max_length): 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' diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py index 023d6869..306f9be5 100644 --- a/kubespawner/spawner.py +++ b/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 diff --git a/tests/test_slugs.py b/tests/test_slugs.py index bad39885..157cce09 100644 --- a/tests/test_slugs.py +++ b/tests/test_slugs.py @@ -12,7 +12,11 @@ ("endswith-", "endswith---165f1166"), ("user@email.com", "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"), + ("a@b.c", "a-b-c---d648b243"), ("-x", "x---a4209624"), ("x-", "x---c8b60efc"), pytest.param("x" * 63, "x" * 63, id="x63"), diff --git a/tests/test_spawner.py b/tests/test_spawner.py index 453ae1ca..61f7a179 100644 --- a/tests/test_spawner.py +++ b/tests/test_spawner.py @@ -27,7 +27,7 @@ class MockUser(Mock): - name = 'fake' + name = '9user@email.com' 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",