-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
K8 dot unames #854
base: master
Are you sure you want to change the base?
K8 dot unames #854
Conversation
I think we're overthinking this. Why not just
Another reason to simplify this. Can this script compute the same hash/various other calculations? No, but it could |
I guess I am just trying to make it work in all cases. Let's say we replace "." by "-", and then firstname.lastname is going to become firstname-lastname. But what if there is already a user with that username? Same with upper case to lower case conversion etc. It is a bit contrived and you could say that if someone has a directory like that it is their problem.
The way I was testing this is to call ruby inside a script. This needs rb file in the same directory, but there might be a way to call a library too? Not very familiar with ruby.
|
I'd rather have something simple that solves most cases we actually see in the wild. I think a simple For example, while I commend you on your diligence to cover all cases, but as a maintainer I need to balance simplicity with the solutions the code provides. Which is to say, I'd rather cover most cases we'll actually see with simple solutions than to have a complex one that covers a lot of cases we'll never actually encounter. (not to mention the distribution and upgrade problems this would bring) |
Resolves the following issue #852 by sanitizing k8 namespace name. Deals with ".", "@", unicode, and converts upper case to lower case. Slug generator is based on JupyterHub code https://github.com/jupyterhub/kubespawner/blob/main/kubespawner/slugs.py and can also be used to sanitize other types of k8 objects.
The bootstrap script would also need to be modified to take advantage of that. https://github.com/OSC/ondemand/blob/master/hooks/k8s-bootstrap/k8s-bootstrap-ondemand.sh
Otherwise the change is not compatible.
Most of the work done by my colleague @DininduSenanayake.