Skip to content
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

avoid name collisions when using named servers #386

Merged
merged 3 commits into from
Aug 5, 2020

Conversation

minrk
Copy link
Member

@minrk minrk commented Jul 17, 2020

we haven't made a release yet with #315 (merged last month), but the default template added there allows collisions between certain usernames and named servers because - is considered a safe character and not escaped.

This changes the default template to use __, the doubled escape character (same as on KubeSpawner, which uses - as the escape character), which cannot occur in an escaped username or servername.

This also ensures the escaped form of servername is used in the template, matching handling of usernames.

Additionally, this persists object_name in state, to avoid changes in the template (as is done here) from causing a disconnect between objects across configuration changes.
That is, changes in the template will only affect newly created objects, not ones that already exist.

/cc @danielballan

@danielballan
Copy link
Contributor

Good catch! Thanks for cleaning up after me. :- D

@minrk
Copy link
Member Author

minrk commented Jul 17, 2020

Tests are failing because conftests imports some fixtures from jupyterhub that are new in 1.0. Need to work around that...

@manics
Copy link
Member

manics commented Jul 17, 2020

@GeorgianaElena already started work on updating the tests for JupyterHub 1.0 #379

@GeorgianaElena
Copy link
Member

I didn't know what to do about the jupyterhub/singleuser:1.1 image 😕
I'll rebase that PR and maybe test with JupyterHub 1.0 only? What do you think?

@minrk minrk changed the title avoid name collisions when using server avoid name collisions when using named servers Jul 18, 2020
prioritize already running names over new template config
separator must not be a safe character or the escape character to avoid collisions

'.' is not in the safe char list, but is a valid docker identifier

- servername is template variable is escaped, matching username
- add raw_servername for the raw value
- fix safe_username to be the safe version
. is not safe in swarmspawner, since it's a DNS component

let's be consistent, at least
@GeorgianaElena GeorgianaElena merged commit b6fccbf into jupyterhub:master Aug 5, 2020
@GeorgianaElena
Copy link
Member

Thanks, Min ❤️ !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants