diff --git a/kubespawner/objects.py b/kubespawner/objects.py index 814d826b..801d04fc 100644 --- a/kubespawner/objects.py +++ b/kubespawner/objects.py @@ -4,6 +4,7 @@ import base64 import ipaddress import json +import operator import os import re from urllib.parse import urlparse @@ -435,24 +436,79 @@ def make_pod( if not csc: csc = None - # Transform a dict into valid Kubernetes EnvVar Python representations. This - # representation shall always have a "name" field as well as either a - # "value" field or "value_from" field. For examples see the - # test_make_pod_with_env function. - prepared_env = [] - for k, v in (env or {}).items(): - if type(v) == dict: - if not "name" in v: - v["name"] = k - prepared_env.append(get_k8s_model(V1EnvVar, v)) + def _get_env_var_deps(env): + # only consider env var objects with an explicit string value + if not env.value: + return set() + # $(MY_ENV) pattern: $( followed by non-)-characters to be captured, followed by ) + re_k8s_env_reference_pattern = r"\$\(([^\)]+)\)" + deps = set(re.findall(re_k8s_env_reference_pattern, env.value)) + return deps - {env.name} + + unsorted_env = {} + for key, env in (env or {}).items(): + # Normalize KubeSpawners env input to valid Kubernetes EnvVar Python + # representations. They should have a "name" field as well as either a + # "value" field or "value_from" field. For examples see the + # test_make_pod_with_env function. + if type(env) == dict: + if not "name" in env: + env["name"] = key + env = get_k8s_model(V1EnvVar, env) else: - prepared_env.append(V1EnvVar(name=k, value=v)) + env = V1EnvVar(name=key, value=env) + + # Extract information about references to other envs as we want to use + # those to make an intelligent sorting before we render this into a list + # with an order that matters. + unsorted_env[env.name] = { + "deps": _get_env_var_deps(env), + "key": key, + "env": env, + } + + # We sort environment variables in a way that allows dependencies to other + # env to resolve as much as possible. There could be circular dependencies + # so we will just do our best and settle with that. + # + # Algorithm description: + # + # - loop step: + # - pop all unsorted_env entries with dependencies in sorted_env + # - sort popped env based on key and extend the sorted_env list + # - loop exit: + # - exit if loop step didn't pop anything from unsorted_env + # - before exit, sort what remains and extending the sorted_env list + # + sorted_env = [] + while True: + already_resolved_env_names = [e.name for e in sorted_env] + + extracted_env = {} + for k, v in unsorted_env.copy().items(): + if v["deps"].issubset(already_resolved_env_names): + extracted_env[k] = unsorted_env.pop(k) + + if extracted_env: + extracted_env = [ + d["env"] + for d in sorted(extracted_env.values(), key=operator.itemgetter("key")) + ] + sorted_env.extend(extracted_env) + else: + remaining_env = [ + d["env"] + for d in sorted(unsorted_env.values(), key=operator.itemgetter("key")) + ] + sorted_env.extend(remaining_env) + break + notebook_container = V1Container( name='notebook', image=image, working_dir=working_dir, ports=[V1ContainerPort(name='notebook-port', container_port=port)], - env=prepared_env, + env=sorted_env, args=cmd, image_pull_policy=image_pull_policy, lifecycle=lifecycle_hooks, diff --git a/tests/test_objects.py b/tests/test_objects.py index 3e14c501..e707add0 100644 --- a/tests/test_objects.py +++ b/tests/test_objects.py @@ -675,8 +675,11 @@ def test_make_pod_with_env(): name='test', image='jupyter/singleuser:latest', env={ - 'TEST_KEY_1': 'TEST_VALUE', - 'TEST_KEY_2': { + 'NAME_1': 'TEST_VALUE', + 'NAME_2': { + 'value': 'TEST_VALUE', + }, + 'NAME_3': { 'valueFrom': { 'secretKeyRef': { 'name': 'my-k8s-secret', @@ -684,8 +687,12 @@ def test_make_pod_with_env(): }, }, }, - 'TEST_KEY_NAME_IGNORED': { - 'name': 'TEST_KEY_3', + 'IGNORED_NAME_1': { + 'name': 'NAME_4', + 'value': 'TEST_VALUE', + }, + 'IGNORED_NAME_2': { + 'name': 'NAME_5', 'valueFrom': { 'secretKeyRef': { 'name': 'my-k8s-secret', @@ -710,11 +717,11 @@ def test_make_pod_with_env(): { "env": [ { - 'name': 'TEST_KEY_1', + 'name': 'NAME_4', 'value': 'TEST_VALUE', }, { - 'name': 'TEST_KEY_2', + 'name': 'NAME_5', 'valueFrom': { 'secretKeyRef': { 'name': 'my-k8s-secret', @@ -723,7 +730,15 @@ def test_make_pod_with_env(): }, }, { - 'name': 'TEST_KEY_3', + 'name': 'NAME_1', + 'value': 'TEST_VALUE', + }, + { + 'name': 'NAME_2', + 'value': 'TEST_VALUE', + }, + { + 'name': 'NAME_3', 'valueFrom': { 'secretKeyRef': { 'name': 'my-k8s-secret', @@ -749,6 +764,97 @@ def test_make_pod_with_env(): } +def test_make_pod_with_env_dependency_sorted(): + """ + Test specification of a pod with custom environment variables that involve + references to other environment variables. Note that KubeSpawner also tries + to sort the environment variables in the rendered list in a way that helps + maximize the amount of variables that are resolved. + """ + assert api_client.sanitize_for_serialization( + make_pod( + name='test', + image='jupyter/singleuser:latest', + env={ + 'NAME_SELF_REF': '$(NAME_SELF_REF)', + 'NAME_1B': '$(NAME_2)', + 'NAME_1A': '$(NAME_2)', + 'NAME_1C': {"name": "NAME_0", "value": "$(NAME_2)"}, + 'NAME_2': '$(NAME_3A)', + 'NAME_3B': 'NAME_3B_VALUE', + 'NAME_3A': 'NAME_3A_VALUE', + 'NAME_CIRCLE_B': '$(NAME_CIRCLE_A)', + 'NAME_CIRCLE_A': '$(NAME_CIRCLE_B)', + }, + cmd=['jupyterhub-singleuser'], + port=8888, + image_pull_policy='IfNotPresent', + ) + ) == { + "metadata": { + "name": "test", + "annotations": {}, + "labels": {}, + }, + "spec": { + 'automountServiceAccountToken': False, + "containers": [ + { + "env": [ + { + 'name': 'NAME_3A', + 'value': 'NAME_3A_VALUE', + }, + { + 'name': 'NAME_3B', + 'value': 'NAME_3B_VALUE', + }, + { + 'name': 'NAME_SELF_REF', + 'value': '$(NAME_SELF_REF)', + }, + { + 'name': 'NAME_2', + 'value': '$(NAME_3A)', + }, + { + 'name': 'NAME_1A', + 'value': '$(NAME_2)', + }, + { + 'name': 'NAME_1B', + 'value': '$(NAME_2)', + }, + { + 'name': 'NAME_0', + 'value': '$(NAME_2)', + }, + { + 'name': 'NAME_CIRCLE_A', + 'value': '$(NAME_CIRCLE_B)', + }, + { + 'name': 'NAME_CIRCLE_B', + 'value': '$(NAME_CIRCLE_A)', + }, + ], + "name": "notebook", + "image": "jupyter/singleuser:latest", + "imagePullPolicy": "IfNotPresent", + "args": ["jupyterhub-singleuser"], + "ports": [{"name": "notebook-port", "containerPort": 8888}], + 'volumeMounts': [], + "resources": {"limits": {}, "requests": {}}, + } + ], + 'restartPolicy': 'OnFailure', + 'volumes': [], + }, + "kind": "Pod", + "apiVersion": "v1", + } + + def test_make_pod_with_lifecycle(): """ Test specification of a pod with lifecycle @@ -2012,79 +2118,6 @@ def test_make_ingress_external_name(): } -def test_make_pod_with_ssl(): - """ - Test specification of a pod with ssl enabled - """ - assert api_client.sanitize_for_serialization( - make_pod( - name='ssl', - image='jupyter/singleuser:latest', - env={ - 'JUPYTERHUB_SSL_KEYFILE': 'TEST_VALUE', - 'JUPYTERHUB_SSL_CERTFILE': 'TEST', - 'JUPYTERHUB_USER': 'TEST', - }, - working_dir='/', - cmd=['jupyterhub-singleuser'], - port=8888, - image_pull_policy='IfNotPresent', - ssl_secret_name='ssl', - ssl_secret_mount_path="/etc/jupyterhub/ssl/", - ) - ) == { - "metadata": { - "name": "ssl", - "annotations": {}, - "labels": {}, - }, - "spec": { - 'automountServiceAccountToken': False, - "containers": [ - { - "env": [ - { - 'name': 'JUPYTERHUB_SSL_KEYFILE', - 'value': '/etc/jupyterhub/ssl/ssl.key', - }, - { - 'name': 'JUPYTERHUB_SSL_CERTFILE', - 'value': '/etc/jupyterhub/ssl/ssl.crt', - }, - {'name': 'JUPYTERHUB_USER', 'value': 'TEST'}, - { - 'name': 'JUPYTERHUB_SSL_CLIENT_CA', - 'value': '/etc/jupyterhub/ssl/notebooks-ca_trust.crt', - }, - ], - "name": "notebook", - "image": "jupyter/singleuser:latest", - "imagePullPolicy": "IfNotPresent", - "args": ["jupyterhub-singleuser"], - "ports": [{"name": "notebook-port", "containerPort": 8888}], - 'volumeMounts': [ - { - 'mountPath': '/etc/jupyterhub/ssl/', - 'name': 'jupyterhub-internal-certs', - } - ], - 'workingDir': '/', - "resources": {"limits": {}, "requests": {}}, - } - ], - 'restartPolicy': 'OnFailure', - 'volumes': [ - { - 'name': 'jupyterhub-internal-certs', - 'secret': {'defaultMode': 511, 'secretName': 'ssl'}, - } - ], - }, - "kind": "Pod", - "apiVersion": "v1", - } - - def test_make_namespace(): labels = { 'heritage': 'jupyterhub', @@ -2136,19 +2169,19 @@ def test_make_pod_with_ssl(): "containers": [ { "env": [ - { - 'name': 'JUPYTERHUB_SSL_KEYFILE', - 'value': '/etc/jupyterhub/ssl/ssl.key', - }, { 'name': 'JUPYTERHUB_SSL_CERTFILE', 'value': '/etc/jupyterhub/ssl/ssl.crt', }, - {'name': 'JUPYTERHUB_USER', 'value': 'TEST'}, { 'name': 'JUPYTERHUB_SSL_CLIENT_CA', 'value': '/etc/jupyterhub/ssl/notebooks-ca_trust.crt', }, + { + 'name': 'JUPYTERHUB_SSL_KEYFILE', + 'value': '/etc/jupyterhub/ssl/ssl.key', + }, + {'name': 'JUPYTERHUB_USER', 'value': 'TEST'}, ], "name": "notebook", "image": "jupyter/singleuser:latest", @@ -2176,23 +2209,3 @@ def test_make_pod_with_ssl(): "kind": "Pod", "apiVersion": "v1", } - - -def test_make_namespace(): - labels = { - 'heritage': 'jupyterhub', - 'component': 'singleuser-server', - } - namespace = api_client.sanitize_for_serialization( - make_namespace(name='test-namespace', labels=labels) - ) - assert namespace == { - 'metadata': { - 'annotations': {}, - 'labels': { - 'component': 'singleuser-server', - 'heritage': 'jupyterhub', - }, - 'name': 'test-namespace', - }, - }