Systematically expand strings like `{username}` in the KubeSpawner configuration

consideRatio commented 2 years ago

Proposed change

As suggested by @yuvipanda in https://github.com/jupyterhub/kubespawner/pull/518#issuecomment-881583288: to expand strings containing {username} etc in all relevant configuration, wherever it may be.

We should systematically expand it in everything [...]

Maybe we can do this in two steps:

[ ] Evaluate places that aren't going to contain shell scripts (like serviceaccount) and expand them automatically. I don't think this needs a major version bump.

[ ] During next major version bump, let's make it expand everything. Folks will then have to explicitly escape any {} in shell scripts.

Alternative options

To add expansion of configuration as we see observe it to be useful for someone, like #518 was for #512.

Who would use this feature?

Users of the expansion feature are many, and while it can be done with pre_spawn_hooks, it is hard to define more than a single one, so a Helm chart like binderhub shouldn't make use of these because then they would block the end users of configuring their own without overriding such logic.

Suggested action points

I suggest the next action points with regards to this feature proposal are to evaluate it further. The goal for me with such evaluation is to overview the value of implementing something like this and to overview the risk of causing disruptive experience through potential introducing of bugs or directly via a breaking change.

[x] To list config options that is getting expanded ( see this list )
[ ] To list config options that currently isn't getting expanded.
[ ] To evaluate if some options are possibly going to cause breaking changes to expand.
[ ] To evaluate to do given the overview above.

Without an overview like above concluding it is a valuable enough change to implement given the drawbacks, I favor the alternative option listed above - to add individual expansions one at the time.

manics commented 1 year ago

As part of these changes do you think we could make part of https://github.com/jupyterhub/kubespawner/blob/6d6fc1d4b56715bf74764462b862bded63b14cb7/kubespawner/spawner.py#L1856-L1887 into a configurable Callable? E.g.

def get_expansions(self):
    ...
    return dict(...)

template_expansions = Callable(default=get_expansions, config=True)

def _expand_user_properties(self, template):
    rendered = template.format(**self.template_expansions())
    return rendered.rstrip("-")

It would provide a way to revert any new expansions, and also means admins can add more (I saw a post/issue asking how to add addition expansions but I can't find it now)

yuvipanda commented 1 year ago

https://github.com/jupyterhub/kubespawner/pull/653 expands this in labels, not sure if that was expanded (hah) to other properties.

jupyterhub / kubespawner