jupyterhub / kubespawner

Kubernetes spawner for JupyterHub
https://jupyterhub-kubespawner.readthedocs.io
BSD 3-Clause "New" or "Revised" License
541 stars 303 forks source link

Bug using `profile_options` when `profile_list` has entries named like `short` and `short-plus` #702

Closed consideRatio closed 1 year ago

consideRatio commented 1 year ago

Bug description

If a profile_list is configured, where one is named short and one is named short-plus, and both provide profile_options, one can end up erroring when selecting the short option.

ValueError: Expected option plus-irrelevant for profile short, not found in posted form

Explanation

This is because we have a HTML form with names constructed like profile-option-<profile slug>-<profile option slug> and because there can be a mixup if one profile slug is longer than another like short and short-plus is.

https://github.com/jupyterhub/kubespawner/blob/cd4c08d5e175e3b6d58e279c27265e6e95e6197b/kubespawner/spawner.py#L2981-L2991

Test function to reproduce

@pytest.mark.parametrize(
    "profile_list, formdata",
    [
        (
            [
                {
                    "display_name": "short",
                    "slug": "short",
                    "kubespawner_override": {},
                    "profile_options": {
                        "relevant": {
                            "choices": {
                                "choice-a": {
                                    "kubespawner_override": {},
                                },
                            },
                        },
                    },
                },
                {
                    "display_name": "short-plus",
                    "slug": "short-plus",
                    "profile_options": {
                        "irrelevant": {
                            "choices": {
                                "choice-b": {
                                    "kubespawner_override": {},
                                },
                            },
                        },
                    },
                },
            ],
            # What is below is hardcoded based on what is above and based on
            # how the HTML form looks currently. If that changes, whats below needs
            # to change as well.
            {
                'profile': ['short'],
                'profile-option-short-relevant': ['choice-a'],
                'profile-option-short-plus-irrelevant': ['choice-b'],
            }
        ),
    ],
)
async def test_profile_options_mixup(profile_list, formdata):
    """
    If we have a profile list with two entries, their respective profile_options
    should not be mixed up with each other. This has happened when one profile
    list entry was named like another but shorter.
    """
    spawner = KubeSpawner(_mock=True)
    spawner.profile_list = profile_list
    spawner.user_options = spawner.options_from_form(formdata)
    await spawner.load_user_options()

Action points

consideRatio commented 1 year ago

I'll start working on this now

consideRatio commented 1 year ago

I think I've resolved this bug in a reasonable way, but i've made a bit too many changes that needs to be isolated in separate PRs first. I look to get it finished tomorrow.