Server names starting with upper letter fail to start

[enolfc]

Bug description

Named server that start with an upper letter produces escaped labels with invalid characters in label annotations. E.g. for "RStudioServerOption" I get this error from kubernetes:

HTTP response body: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Pod \"jupyter-enol-2efernandez---52-53tudio-53erver-4fption\" is invalid: metadata.labels: Invalid value: \"-52-53tudio-53erver-4fption\": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')","reason":"Invalid","details":{"name":"jupyter-enol-2efernandez---52-53tudio-53erver-4fption","kind":"Pod","causes":[{"reason":"FieldValueInvalid","message":"Invalid value: \"-52-53tudio-53erver-4fption\": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')","field":"metadata.labels"}]},"code":422}

Expected behaviour

Named servers work independently of the name used

Actual behaviour

How to reproduce

1. Enable Named Servers
2. Spawn a Named server starting with capital letter
3. See error

Your personal set up

zero-to-jupyterhub with several customisations, jupyterhub-kubespawner v5.0.0

Issue is similar to #730 and #498

[yuvipanda]

@enolfc this should have been fixed by #694 - perhaps you need to upgrade your version of kubespawner?

[enolfc]

I have checked my installed version and I have v6.0.0.

Going to the code I see the issue here:
https://github.com/jupyterhub/kubespawner/blob/main/kubespawner/spawner.py#L1849

I have added this to my code:

        self.log.debug(escapism.escape(
                    self.name, safe=self.safe_chars, escape_char='-'
                ).lower())

And using as server name ALLCAPS I see in the log:

[D 2023-06-06 06:27:56.006 JupyterHub spawner:1849] -41-4c-4c-43-41-50-53

These are the pod labels:

                  'labels': {'app': 'jupyterhub',
                             'chart': 'jupyterhub-2.0.0',
                             'component': 'singleuser-server',
                             'heritage': 'jupyterhub',
                             'hub.jupyter.org/network-access-hub': 'true',
                             'hub.jupyter.org/servername': '-41-4c-4c-43-41-50-53',
                             'hub.jupyter.org/username': 'enol-2efernandez',
                             'release': 'd4science-dev'},

[yuvipanda]

Ah, I see - the .lower is done after the escaping, so the uppercase letters are still being escaped. Hmm.

@minrk are servernames case insensitive? Can we lowercase the servername first before escaping?

[minrk]

Server names aren't restricted by JupyterHub (I know, I should have specified a strict, limited schema), but it's a namespace entirely controlled by each user, so I don't think you need to worry about case sensitivity. A user definitely can create case collisions if they really want to shoot themselves in the foot by naming their servers COLLISION and collision. They can't shoot at any other feet, though.

Changing escaping can lead to lost volumes, though, so need to be careful about that.

But just like other things we use for labels, we can't assume that things won't start with escape-needing characters, so we need to handle that in KubeSpawner. Case insensitivity wouldn't solve this if another escaped character were at the beginning. We need to solve it in our slug-generation code to produce valid labels when the first character is - (escaped or not).

[yuvipanda]

@jbusecke just ran into this :D

Given #744 is backwards incompatible, particularly with respect to volume names (ugh), is there a smaller more specific fix we can put in here for server names?

[minrk]

An approach I took at one point in jupyterhub/jupyterhub#4471 to avoid backward-compatibility breakage is to specifically check if names need escaping, and opt-in to the new scheme when they do.

That provides a path to backward-compatibility by having two schemes - one that's unchanged from before, and another that's only taken in the situations that we know don't work already.

i.e. something like:

name = current_scheme()
if not is_valid_name(name):
    name = ensure_valid_name(name)

That way, it's fully backward compatible in that it only changes the scheme for names that don't work today. ensure_valid_name and is_valid_name should be written such that they can't produce collisions with valid names.That means you have to exclude some valid (ideally unlikely) names. I used -- in jupyterhub/jupyterhub#4471, for example. That's almost backward-compatible (excluding the very unlikely class of names that include the escape signifier - can possibly be guaranteed in some cases, since escapes are already applied).

We can do a smaller version of this to specifically catch the starts-with-escape case, but we do know there are other ways to produce invalid names (max length, etc.). The approach is the same, it's just in the definitions of is_valid_name and ensure_valid_name.

[yuvipanda]

@minrk think you'll have any time to work on such an approach?

[minrk]

I'm not sure. Possibly, but I wouldn't count on it in the next few weeks.

If I were to do it, I would probably go with finishing #744 and adding a backward-compatible scheme that preserves the old scheme, but opts into the new 'safe' one in cases of known failure.

We could also do a totally separate backward-compatibility PR that ensures all of these templated fields go in Spawner state (volumes, etc.), so they don't get rerendered when config changes, no matter what, i.e.

pod_name = state.get("pod_name", templated_name)
volume_name = state.get("volume_name", templated_volume_name)

But I suspect that's going to be really hard to do in general, given how completely freely our templated strings can be used. It will also be hard to say which keys should expire on stop vs persisting (e.g. pod name should expire, volume name should not...usually!)

[jabbera]

My cluster is only 60 or so instances so this is probably easier for me, but an opt in switch to modernize the naming scheme would be better for me. IE, let me pick which scheme to use so I can control when the template changes. The proposed solution is the worst for me because we make assumptions about the naming scheme in our environment and new pods that can show up that don't match the scheme would be terrible.

[minrk]

an opt in switch to modernize the naming scheme would be better for me

That's what #744 does now.

[aiweaver]

I have the same problem.
In named server, if the server name starts with an uppercase letter (in this case Snow1), an invalid value error occurs.

• kubespawner: v6.2.0
• jupyterhub: v4.1.5