Skip to content

Commit

Permalink
Merge pull request #510 from consideRatio/pr/sort-env-smart
Browse files Browse the repository at this point in the history 
Sort env to reliably expand nested env references
  • Loading branch information
@minrk
minrk authored Jul 21, 2021
2 parents a8c01d3 + 9b98ac3 commit 40c0723
Show file tree
Hide file tree
Showing 2 changed files with 186 additions and 117 deletions.
80 changes: 68 additions & 12 deletions kubespawner/objects.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import base64
import ipaddress
import json
import operator
import os
import re
from urllib.parse import urlparse
Expand Down Expand Up @@ -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,
Expand Down
223 changes: 118 additions & 105 deletions tests/test_objects.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -675,17 +675,24 @@ 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',
'key': 'password',
},
},
},
'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',
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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',
},
}

0 comments on commit 40c0723

Please sign in to comment.