From 56602aa3275cb4571369583f7f436b3cec702184 Mon Sep 17 00:00:00 2001 From: Charles Surett Date: Thu, 11 Feb 2021 16:59:52 -0500 Subject: [PATCH 1/4] Added support for named-servers --- systemdspawner/systemdspawner.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/systemdspawner/systemdspawner.py b/systemdspawner/systemdspawner.py index d1d9160..9b9e5d1 100644 --- a/systemdspawner/systemdspawner.py +++ b/systemdspawner/systemdspawner.py @@ -137,6 +137,13 @@ class SystemdSpawner(Spawner): """ ).tag(config=True) + append_name = Bool( + True, + help=""" + Append -{self.name} to the unit if using c.JupyterHub.allow_named_servers + """ + ).tag(config=True) + slice = Unicode( None, allow_none=True, @@ -151,6 +158,8 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) # All traitlets configurables are configured by now self.unit_name = self._expand_user_vars(self.unit_name_template) + if self.append_name and self.name: + self.unit_name = self.unit_name + '-' + self.name self.log.debug('user:%s Initialized spawner with unit %s', self.user.name, self.unit_name) From aaaa64a184918e4533f6d9b3286ed5cdb83d441a Mon Sep 17 00:00:00 2001 From: Charles Surett Date: Thu, 11 Feb 2021 17:41:16 -0500 Subject: [PATCH 2/4] Handle the server name in _expand_user_vars --- systemdspawner/systemdspawner.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/systemdspawner/systemdspawner.py b/systemdspawner/systemdspawner.py index 9b9e5d1..7f9106b 100644 --- a/systemdspawner/systemdspawner.py +++ b/systemdspawner/systemdspawner.py @@ -53,11 +53,11 @@ class SystemdSpawner(Spawner): ).tag(config=True) unit_name_template = Unicode( - 'jupyter-{USERNAME}-singleuser', + 'jupyter-{USERNAME}-singleuser-{SERVERNAME}', help=""" Template to use to make the systemd service names. - {USERNAME} and {USERID} are expanded} + {USERNAME}, {SERVERNAME}, and {USERID} are expanded} """ ).tag(config=True) @@ -137,13 +137,6 @@ class SystemdSpawner(Spawner): """ ).tag(config=True) - append_name = Bool( - True, - help=""" - Append -{self.name} to the unit if using c.JupyterHub.allow_named_servers - """ - ).tag(config=True) - slice = Unicode( None, allow_none=True, @@ -158,8 +151,6 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) # All traitlets configurables are configured by now self.unit_name = self._expand_user_vars(self.unit_name_template) - if self.append_name and self.name: - self.unit_name = self.unit_name + '-' + self.name self.log.debug('user:%s Initialized spawner with unit %s', self.user.name, self.unit_name) @@ -170,11 +161,14 @@ def _expand_user_vars(self, string): Currently expands: {USERNAME} -> Name of the user {USERID} -> UserID + {SERVERNAME} -> Name of the server (self.name) """ + # Strip the trailing - if we don't have a name. return string.format( USERNAME=self.user.name, - USERID=self.user.id - ) + USERID=self.user.id, + SERVERNAME=self.name + ).rstrip('-') def get_state(self): """ From b24bb6df86334ec5d894b39f7689ed03e1f2704c Mon Sep 17 00:00:00 2001 From: scj643 Date: Thu, 18 Feb 2021 13:41:00 -0500 Subject: [PATCH 3/4] Implemented escapism for _expand_user_vars. --- setup.py | 1 + systemdspawner/systemdspawner.py | 22 +++++++++++++++++----- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/setup.py b/setup.py index e363f06..89f1a81 100644 --- a/setup.py +++ b/setup.py @@ -23,5 +23,6 @@ install_requires=[ "jupyterhub>=0.9", "tornado>=5.0", + "escapism" ], ) diff --git a/systemdspawner/systemdspawner.py b/systemdspawner/systemdspawner.py index 7f9106b..bfa146d 100644 --- a/systemdspawner/systemdspawner.py +++ b/systemdspawner/systemdspawner.py @@ -3,7 +3,9 @@ import subprocess from traitlets import Bool, Unicode, List, Dict import asyncio +import string +import escapism from systemdspawner import systemd from jupyterhub.spawner import Spawner @@ -147,6 +149,9 @@ class SystemdSpawner(Spawner): """ ).tag(config=True) + # Characters that are safe for systemd units. + safe_chars = set(string.ascii_lowercase + string.digits + string.ascii_uppercase + '_-') + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) # All traitlets configurables are configured by now @@ -154,7 +159,13 @@ def __init__(self, *args, **kwargs): self.log.debug('user:%s Initialized spawner with unit %s', self.user.name, self.unit_name) - def _expand_user_vars(self, string): + def _escape_variable(self, in_string): + """ + Escape variables for systemd unit naming + """ + return escapism.escape(in_string, safe=self.safe_chars, escape_char='-') + + def _expand_user_vars(self, in_string): """ Expand user related variables in a given string @@ -164,10 +175,10 @@ def _expand_user_vars(self, string): {SERVERNAME} -> Name of the server (self.name) """ # Strip the trailing - if we don't have a name. - return string.format( - USERNAME=self.user.name, + return in_string.format( + USERNAME=self._escape_variable(self.user.name), USERID=self.user.id, - SERVERNAME=self.name + SERVERNAME=self._escape_variable(self.name) ).rstrip('-') def get_state(self): @@ -208,7 +219,8 @@ async def start(self): # from earlier. Regardless, we kill it and start ours in its place. # FIXME: Carefully look at this when doing a security sweep. if await systemd.service_running(self.unit_name): - self.log.info('user:%s Unit %s already exists but not known to JupyterHub. Killing', self.user.name, self.unit_name) + self.log.info('user:%s Unit %s already exists but not known to JupyterHub. Killing', self.user.name, + self.unit_name) await systemd.stop_service(self.unit_name) if await systemd.service_running(self.unit_name): self.log.error('user:%s Could not stop already existing unit %s', self.user.name, self.unit_name) From 422de94a30de479d57260ab8129b1f587cfed0cb Mon Sep 17 00:00:00 2001 From: scj643 Date: Mon, 22 Feb 2021 12:02:29 -0500 Subject: [PATCH 4/4] Moved escaping to all of _expand_user_vars Added checking for string length when escaping. Added option for striping trailing character for _expand_user_vars. Made escape_char configurable. --- systemdspawner/systemdspawner.py | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/systemdspawner/systemdspawner.py b/systemdspawner/systemdspawner.py index bfa146d..88562bd 100644 --- a/systemdspawner/systemdspawner.py +++ b/systemdspawner/systemdspawner.py @@ -149,8 +149,23 @@ class SystemdSpawner(Spawner): """ ).tag(config=True) + unit_trailing_character = Unicode( + '-', + help=""" + When using a unit that ends in a SERVERNAME strip this character from the end of the parsed string. + """ + ).tag(config=True) + + escape_char = Unicode( + ':', + help=""" + The character to change invalid safe characters to. + """ + ).tag(config=True) + # Characters that are safe for systemd units. - safe_chars = set(string.ascii_lowercase + string.digits + string.ascii_uppercase + '_-') + safe_chars = set(string.ascii_lowercase + string.digits + string.ascii_uppercase + ':-_.\\') + max_unit_length = 248 def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -163,7 +178,10 @@ def _escape_variable(self, in_string): """ Escape variables for systemd unit naming """ - return escapism.escape(in_string, safe=self.safe_chars, escape_char='-') + if len(in_string) > self.max_unit_length: + self.log.warning(f'String is longer than {self.max_unit_length}') + # Slice the string to the max unit length + return escapism.escape(in_string[:self.max_unit_length], safe=self.safe_chars, escape_char=self.escape_char) def _expand_user_vars(self, in_string): """ @@ -175,11 +193,11 @@ def _expand_user_vars(self, in_string): {SERVERNAME} -> Name of the server (self.name) """ # Strip the trailing - if we don't have a name. - return in_string.format( - USERNAME=self._escape_variable(self.user.name), + return self._escape_variable(in_string.format( + USERNAME=self.user.name, USERID=self.user.id, - SERVERNAME=self._escape_variable(self.name) - ).rstrip('-') + SERVERNAME=self.name + )).rstrip(self.unit_trailing_character) def get_state(self): """