From eb08583f9984260ab50bef583922c52ad1443c9b Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Thu, 14 Sep 2023 11:30:30 -0700 Subject: [PATCH 1/5] Add failing test when kubespawner_override is a dict --- tests/test_profile.py | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/test_profile.py b/tests/test_profile.py index 09d675e3..f40c9cc5 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -238,6 +238,45 @@ async def test_find_slug_exception(): spawner._get_profile('does-not-exist', profile_list) +async def test_unlisted_choice_non_string_override(): + profiles = [ + { + 'display_name': 'CPU only', + 'slug': 'cpu', + 'profile_options': { + 'image': { + 'display_name': 'Image', + 'unlisted_choice': { + 'enabled': True, + 'display_name': 'Image Location', + 'validation_regex': '^pangeo/.*$', + 'validation_message': 'Must be a pangeo image, matching ^pangeo/.*$', + 'kubespawner_override': { + 'image': '{value}', + 'environment': { + 'CUSTOM_IMAGE_USED': 'yes', + 'CUSTOM_IMAGE': '{value}', + }, + }, + }, + } + }, + }, + ] + spawner = KubeSpawner(_mock=True) + spawner.profile_list = profiles + + image = "pangeo/pangeo-notebook:latest" + # Set user option for image directly + spawner.user_options = {"profile": "cpu", "image--unlisted-choice": image} + + # this shouldn't error + await spawner.load_user_options() + + assert spawner.image == image + assert spawner.environment == {'CUSTOM_IMAGE_USED': 'yes', 'CUSTOM_IMAGE': image} + + async def test_empty_user_options_and_profile_options_api(): profiles = [ { From 83dea299dd3614d1911e2f4d5bde9148d907b8f0 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Thu, 14 Sep 2023 12:20:06 -0700 Subject: [PATCH 2/5] Support lists & dicts in unlisted_choice kubespawner_override --- kubespawner/spawner.py | 7 ++----- kubespawner/utils.py | 43 ++++++++++++++++++++++++++++++++++++++++++ tests/test_profile.py | 16 +++++++++++++++- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py index fa511e33..67e8fc24 100644 --- a/kubespawner/spawner.py +++ b/kubespawner/spawner.py @@ -46,7 +46,7 @@ make_service, ) from .reflector import ResourceReflector -from .utils import recursive_update +from .utils import recursive_format, recursive_update class PodReflector(ResourceReflector): @@ -3191,10 +3191,7 @@ def _load_profile(self, slug, profile_list): "kubespawner_override", {} ) for k, v in option_overrides.items(): - # FIXME: This logic restricts unlisted_choice to define - # kubespawner_override dictionaries where all keys - # have string values. - option_overrides[k] = v.format(value=unlisted_choice) + option_overrides[k] = recursive_format(v, value=unlisted_choice) elif choice: # A pre-defined choice was selected option_overrides = option["choices"][choice].get( diff --git a/kubespawner/utils.py b/kubespawner/utils.py index 47db20eb..a9335861 100644 --- a/kubespawner/utils.py +++ b/kubespawner/utils.py @@ -3,6 +3,7 @@ """ import copy import hashlib +from typing import Union def generate_hashed_slug(slug, limit=63, hash_length=6): @@ -222,3 +223,45 @@ def recursive_update(target, new): else: target[k] = v + + +class IgnoreMissing(dict): + """ + Dictionary subclass for use with format_map + + Renders missing values as "{key}", so format strings with + missing values just get rendered as is. + + Stolen from https://docs.python.org/3/library/stdtypes.html#str.format_map + """ + + def __missing__(self, key): + return f"{{{key}}}" + + +def recursive_format( + format_object: Union[str, dict, list], **kwargs +) -> Union[str, dict, list]: + """ + Recursively format given object with values provided as keyword arguments. + + If the given object (string, list or dict) has items that do not have + placeholders for passed in kwargs, no formatting is performed. + + recursive_format("{v}", v=5) -> Returns "5" + recrusive_format("{a}") -> Returns "{a}" rather than erroring, as is + the behavior of "format" + """ + if isinstance(format_object, str): + return format_object.format_map(IgnoreMissing(kwargs)) + elif isinstance(format_object, list): + return [recursive_format(i, **kwargs) for i in format_object] + elif isinstance(format_object, dict): + return { + recursive_format(k, **kwargs): recursive_format(v, **kwargs) + for k, v in format_object.items() + } + else: + raise ValueError( + f"Object of type {type(format_object)} can not be recursively formatted" + ) diff --git a/tests/test_profile.py b/tests/test_profile.py index f40c9cc5..87a3d40d 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -256,7 +256,15 @@ async def test_unlisted_choice_non_string_override(): 'environment': { 'CUSTOM_IMAGE_USED': 'yes', 'CUSTOM_IMAGE': '{value}', + # This should just be passed through, as JUPYTER_USER is not replaced + 'USER': '${JUPYTER_USER}', + # This should render as ${JUPYTER_USER}, as the {{ and }} escape them. + # this matches existing behavior for other replacements elsewhere + 'USER_TEST': '${{JUPYTER_USER}}', }, + "init_containers": [ + {"name": "testing", "image": "{value}"} + ], }, }, } @@ -274,7 +282,13 @@ async def test_unlisted_choice_non_string_override(): await spawner.load_user_options() assert spawner.image == image - assert spawner.environment == {'CUSTOM_IMAGE_USED': 'yes', 'CUSTOM_IMAGE': image} + assert spawner.environment == { + 'CUSTOM_IMAGE_USED': 'yes', + 'CUSTOM_IMAGE': image, + 'USER': '${JUPYTER_USER}', + 'USER_TEST': '${JUPYTER_USER}', + } + assert spawner.init_containers == [{"name": "testing", "image": image}] async def test_empty_user_options_and_profile_options_api(): From 7620094b983e30002ed65c06f6d897a15c075017 Mon Sep 17 00:00:00 2001 From: YuviPanda Date: Fri, 15 Sep 2023 00:15:24 -0700 Subject: [PATCH 3/5] Allow passing ints and floats to recursive_format --- kubespawner/utils.py | 10 +++------- tests/test_profile.py | 10 ++++++++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/kubespawner/utils.py b/kubespawner/utils.py index a9335861..0ed03f57 100644 --- a/kubespawner/utils.py +++ b/kubespawner/utils.py @@ -3,7 +3,6 @@ """ import copy import hashlib -from typing import Union def generate_hashed_slug(slug, limit=63, hash_length=6): @@ -239,9 +238,7 @@ def __missing__(self, key): return f"{{{key}}}" -def recursive_format( - format_object: Union[str, dict, list], **kwargs -) -> Union[str, dict, list]: +def recursive_format(format_object, **kwargs): """ Recursively format given object with values provided as keyword arguments. @@ -262,6 +259,5 @@ def recursive_format( for k, v in format_object.items() } else: - raise ValueError( - f"Object of type {type(format_object)} can not be recursively formatted" - ) + # Everything else just gets returned as is, unformatted + return format_object diff --git a/tests/test_profile.py b/tests/test_profile.py index 87a3d40d..74599473 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -263,7 +263,11 @@ async def test_unlisted_choice_non_string_override(): 'USER_TEST': '${{JUPYTER_USER}}', }, "init_containers": [ - {"name": "testing", "image": "{value}"} + { + "name": "testing", + "image": "{value}", + "securityContext": {"runAsUser": 1000}, + } ], }, }, @@ -288,7 +292,9 @@ async def test_unlisted_choice_non_string_override(): 'USER': '${JUPYTER_USER}', 'USER_TEST': '${JUPYTER_USER}', } - assert spawner.init_containers == [{"name": "testing", "image": image}] + assert spawner.init_containers == [ + {"name": "testing", "image": image, 'securityContext': {'runAsUser': 1000}} + ] async def test_empty_user_options_and_profile_options_api(): From 6a86c7e44bbd93f1f3df722fbeefb348c7e762b2 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Tue, 22 Aug 2023 11:15:41 +0200 Subject: [PATCH 4/5] Fix formatting and tweak reference docs for profile_list --- kubespawner/spawner.py | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py index 67e8fc24..ad5a2cc4 100644 --- a/kubespawner/spawner.py +++ b/kubespawner/spawner.py @@ -1523,9 +1523,10 @@ def _validate_image_pull_secrets(self, proposal): Signature is: `List(Dict())`, where each item is a dictionary that has two keys: - `display_name`: the human readable display name (should be HTML safe) - - `slug`: the machine readable slug to identify the profile - (missing slugs are generated from display_name) + - `default`: (Optional Bool) True if this is the default selected option - `description`: Optional description of this profile displayed to the user. + - `slug`: (Optional) the machine readable string to identify the + profile (missing slugs are generated from display_name) - `kubespawner_override`: a dictionary with overrides to apply to the KubeSpawner settings. Each value can be either the final value to change or a callable that take the `KubeSpawner` instance as parameter and return the final value. This can @@ -1541,18 +1542,32 @@ def _validate_image_pull_secrets(self, proposal): - `display_name`: Name used to identify this particular option - `unlisted_choice`: Object to specify if there should be a free-form field if the user selected "Other" as a choice: + - `enabled`: Boolean, whether the free form input should be enabled - `display_name`: String, label for input field - `display_name_in_choices`: Optional, display name for the choice to specify an unlisted choice in the dropdown list of pre-defined choices. Defaults to "Other...". - - `validation_regex`: Optional, regex that the free form input should match - eg. ^pangeo/.*$ - - `validation_message`: Optional, validation message for the regex. Should describe the required - input format in a human-readable way. - - `kubespawner_override`: Object specifying what key:values should be over-ridden - with the value of the free form input, using `{value}` for the value to be substituted with - the user POSTed value in the `unlisted_choice` input field. eg: - - some_config_key: some_value-with-{value}-substituted-with-what-user-wrote + - `validation_regex`: Optional, regex that the free form input + should match, eg. `^pangeo/.*$`. + - `validation_message`: Optional, validation message for the regex. + Should describe the required input format in a human-readable way. + - `kubespawner_override`: a dictionary with overrides to apply to + the KubeSpawner settings, where the string `{value}` will be + substituted with what was filled in by the user if its found in + string values anywhere in the dictionary. As an example, if the + choice made is about an image tag for an image only to be used + with JupyterLab, it could look like this: + + .. code-block:: python + + { + "image_spec": "jupyter/datascience-notebook:{value}", + "default_url": "/lab", + "extra_labels: { + "user-specified-image-tag": "{value}", + }, + } - `choices`: A dictionary containing list of choices for the user to choose from to set the value for this particular option. The key is an identifier for this choice, and the value is a dictionary with the following possible keys: @@ -1568,7 +1583,6 @@ def _validate_image_pull_secrets(self, proposal): If the traitlet being overriden is a *dictionary*, the dictionary will be *recursively updated*, rather than overriden. If you want to remove a key, set its value to `None` - - `default`: (optional Bool) True if this is the default selected option kubespawner setting overrides work in the following manner, with items further in the list *replacing* (not merging with) items earlier in the list: From 4797bec53a35a2c3ec1ea08a649b5703ae3e6355 Mon Sep 17 00:00:00 2001 From: Yuvi Panda Date: Mon, 18 Sep 2023 18:02:09 -0700 Subject: [PATCH 5/5] Support format_objects being sets Co-authored-by: Erik Sundell --- kubespawner/utils.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kubespawner/utils.py b/kubespawner/utils.py index 0ed03f57..ffe2b65a 100644 --- a/kubespawner/utils.py +++ b/kubespawner/utils.py @@ -228,7 +228,7 @@ class IgnoreMissing(dict): """ Dictionary subclass for use with format_map - Renders missing values as "{key}", so format strings with + Returns missing dictionary keys' values as "{key}", so format strings with missing values just get rendered as is. Stolen from https://docs.python.org/3/library/stdtypes.html#str.format_map @@ -242,7 +242,7 @@ def recursive_format(format_object, **kwargs): """ Recursively format given object with values provided as keyword arguments. - If the given object (string, list or dict) has items that do not have + If the given object (string, list, set, or dict) has items that do not have placeholders for passed in kwargs, no formatting is performed. recursive_format("{v}", v=5) -> Returns "5" @@ -253,6 +253,8 @@ def recursive_format(format_object, **kwargs): return format_object.format_map(IgnoreMissing(kwargs)) elif isinstance(format_object, list): return [recursive_format(i, **kwargs) for i in format_object] + elif isinstance(format_object, set): + return {recursive_format(i, **kwargs) for i in format_object} elif isinstance(format_object, dict): return { recursive_format(k, **kwargs): recursive_format(v, **kwargs)