Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store provided environment of key/value kind into a k8s Secret owned by the spawned pod #471

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 8 additions & 47 deletions kubespawner/objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import os
import re
from urllib.parse import urlparse

from kubernetes.client.models import (
V1Affinity,
V1Container,
Expand Down Expand Up @@ -36,6 +35,8 @@
V1PreferredSchedulingTerm,
V1ResourceRequirements,
V1Secret,
V1EnvFromSource,
V1SecretEnvSource,
V1SecurityContext,
V1Service,
V1ServicePort,
Expand Down Expand Up @@ -63,6 +64,7 @@ def make_pod(
run_privileged=False,
allow_privilege_escalation=True,
env=None,
env_from=None,
working_dir=None,
volumes=None,
volume_mounts=None,
Expand Down Expand Up @@ -266,6 +268,7 @@ def make_pod(

pod.spec = V1PodSpec(containers=[])
pod.spec.restart_policy = 'OnFailure'


if image_pull_secrets is not None:
# image_pull_secrets as received by the make_pod function should always
Expand All @@ -288,12 +291,6 @@ def make_pod(
}
)

env['JUPYTERHUB_SSL_KEYFILE'] = ssl_secret_mount_path + "ssl.key"
env['JUPYTERHUB_SSL_CERTFILE'] = ssl_secret_mount_path + "ssl.crt"
env['JUPYTERHUB_SSL_CLIENT_CA'] = (
ssl_secret_mount_path + "notebooks-ca_trust.crt"
)

if not volume_mounts:
volume_mounts = []
volume_mounts.append(
Expand Down Expand Up @@ -340,24 +337,13 @@ def make_pod(
if all([e is None for e in container_security_context.to_dict().values()]):
container_security_context = 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))
else:
prepared_env.append(V1EnvVar(name=k, value=v))
notebook_container = V1Container(
name='notebook',
image=image,
working_dir=working_dir,
ports=[V1ContainerPort(name='notebook-port', container_port=port)],
env=prepared_env,
env=env,
env_from=[V1EnvFromSource(secret_ref=V1SecretEnvSource(env_from))],
args=cmd,
image_pull_policy=image_pull_policy,
lifecycle=lifecycle_hooks,
Expand Down Expand Up @@ -671,9 +657,7 @@ def make_owner_reference(name, uid):

def make_secret(
name,
username,
cert_paths,
hub_ca,
string_data,
owner_references,
labels=None,
annotations=None,
Expand All @@ -688,10 +672,6 @@ def make_secret(
going to be created in.
username:
The name of the user notebook.
cert_paths:
JupyterHub spawners cert_paths dictionary container certificate path references
hub_ca:
Path to the hub certificate authority
labels:
Labels to add to the secret.
annotations:
Expand All @@ -706,26 +686,7 @@ def make_secret(
secret.metadata.annotations = (annotations or {}).copy()
secret.metadata.labels = (labels or {}).copy()
secret.metadata.owner_references = owner_references

secret.data = {}

with open(cert_paths['keyfile'], 'r') as file:
encoded = base64.b64encode(file.read().encode("utf-8"))
secret.data['ssl.key'] = encoded.decode("utf-8")

with open(cert_paths['certfile'], 'r') as file:
encoded = base64.b64encode(file.read().encode("utf-8"))
secret.data['ssl.crt'] = encoded.decode("utf-8")

with open(cert_paths['cafile'], 'r') as file:
encoded = base64.b64encode(file.read().encode("utf-8"))
secret.data["notebooks-ca_trust.crt"] = encoded.decode("utf-8")

with open(hub_ca, 'r') as file:
encoded = base64.b64encode(file.read().encode("utf-8"))
secret.data["notebooks-ca_trust.crt"] = secret.data[
"notebooks-ca_trust.crt"
] + encoded.decode("utf-8")
secret.string_data = string_data

return secret

Expand Down
123 changes: 88 additions & 35 deletions kubespawner/spawner.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import asyncio
import json
import base64
import multiprocessing
import os
import string
Expand All @@ -27,6 +28,8 @@
from jupyterhub.spawner import Spawner
from jupyterhub.traitlets import Command
from jupyterhub.utils import exponential_backoff
from kubespawner.utils import get_k8s_model
from kubernetes.client.models import V1EnvVar
from kubernetes import client
from kubernetes.client.rest import ApiException
from slugify import slugify
Expand Down Expand Up @@ -1624,7 +1627,8 @@ async def get_pod_manifest(self):
supplemental_gids=supplemental_gids,
run_privileged=self.privileged,
allow_privilege_escalation=self.allow_privilege_escalation,
env=self.get_env(),
env=self.get_env_value_from() if self.get_env_value_from() else None,
env_from=self.secret_name,
volumes=self._expand_all(self.volumes),
volume_mounts=self._expand_all(self.volume_mounts),
working_dir=self.working_dir,
Expand Down Expand Up @@ -1665,12 +1669,10 @@ def get_secret_manifest(self, owner_reference):
annotations = self._build_common_annotations(
self._expand_all(self.extra_annotations)
)

return make_secret(
name=self.secret_name,
username=self.user.name,
cert_paths=self.cert_paths,
hub_ca=self.internal_trust_bundles['hub-ca'],
string_data=self.get_env(),
owner_references=[owner_reference],
labels=labels,
annotations=annotations,
Expand Down Expand Up @@ -1768,11 +1770,57 @@ def get_env(self):
"""

env = super(KubeSpawner, self).get_env()

# Filter env. variables with only values and pass it to secret
env = {k: v for k, v in (env or {}).items() if type(v) != dict}
# deprecate image
env['JUPYTER_IMAGE_SPEC'] = self.image
env['JUPYTER_IMAGE'] = self.image

if self.internal_ssl:
"""
cert_paths:
certificate path references
hub_ca:
Path to the hub certificate authority
"""
with open(self.cert_paths['keyfile'], 'r') as file:
env['ssl.key'] = file.read()

with open(self.cert_paths['certfile'], 'r') as file:
env['ssl.crt'] = file.read()

with open(self.cert_paths['cafile'], 'r') as file:
env["notebooks-ca_trust.crt"] = file.read()

with open(self.internal_trust_bundles['hub-ca'], 'r') as file:
env["notebooks-ca_trust.crt"] = env[
"notebooks-ca_trust.crt"
] + file.read()
env['JUPYTERHUB_SSL_KEYFILE'] = self.secret_mount_path + "ssl.key"
env['JUPYTERHUB_SSL_CERTFILE'] = self.secret_mount_path + "ssl.crt"
env['JUPYTERHUB_SSL_CLIENT_CA'] = (self.secret_mount_path + "notebooks-ca_trust.crt")

return env

def get_env_value_from(self):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@consideRatio I've created a separate get_env_value_from to filter only the env. variables which has value_from, hence the name :). Now get_env will filter only the env. variables with values but not value_from. Also i'm using V1EnvVar in this since we already have the right format for it to process.

For example.
This "TEST_KEY_2": {'valueFrom': {'secretKeyRef': {'name': 'my-test-secret', 'key': 'password'}}} being formatted as {"name": "TEST_KEY_2", "value_from": {'secretKeyRef': {'name': 'my-test-secret', 'key': 'password'}}}. I'm creating list of these env. variables containing value_from which are then passed to V1Container object in env variable.


"""Return the environment dict to use for the Spawner.

See also: jupyterhub.Spawner.get_env
"""

env = super(KubeSpawner, self).get_env()
env_value_from = []
# Filter env. variables with only "valueFrom" environment variables.
extra_env = {k: v for k, v in (env or {}).items() if type(v) == dict}
for k, v in (extra_env or {}).items():
if not "name" in v:
v["name"] = k
env_value_from.append(get_k8s_model(V1EnvVar, v))

return env_value_from


def load_state(self, state):
"""
Expand Down Expand Up @@ -2182,34 +2230,38 @@ async def _start(self):
timeout=self.k8s_api_request_retry_timeout
)

if self.internal_ssl:
try:
# wait for pod to have uid,
# required for creating owner reference
await exponential_backoff(
lambda: self.pod_has_uid(
self.pod_reflector.pods.get(self.pod_name, None)
),
"pod/%s does not have a uid!" % (self.pod_name),
)
try:
# wait for pod to have uid,
# required for creating owner reference
await exponential_backoff(
lambda: self.pod_has_uid(
self.pod_reflector.pods.get(self.pod_name, None)
),
"pod/%s does not have a uid!" % (self.pod_name),
)

pod = self.pod_reflector.pods[self.pod_name]
owner_reference = make_owner_reference(
self.pod_name, pod["metadata"]["uid"]
)
pod = self.pod_reflector.pods[self.pod_name]
owner_reference = make_owner_reference(
self.pod_name, pod["metadata"]["uid"]
)

"""Create secret object

# internal ssl, create secret object
secret_manifest = self.get_secret_manifest(owner_reference)
await exponential_backoff(
partial(
self._ensure_not_exists, "secret", secret_manifest.metadata.name
),
f"Failed to delete secret {secret_manifest.metadata.name}",
)
await exponential_backoff(
partial(self._make_create_resource_request, "secret", secret_manifest),
f"Failed to create secret {secret_manifest.metadata.name}",
)
Assuming there will be atleast one env. variable that will be passed
"""
secret_manifest = self.get_secret_manifest(owner_reference)
await exponential_backoff(
partial(
self._ensure_not_exists, "secret", secret_manifest.metadata.name
),
f"Failed to delete secret {secret_manifest.metadata.name}",
)
await exponential_backoff(
partial(self._make_create_resource_request, "secret", secret_manifest),
f"Failed to create secret {secret_manifest.metadata.name}",
)

if self.internal_ssl:

service_manifest = self.get_service_manifest(owner_reference)
await exponential_backoff(
Expand All @@ -2224,10 +2276,11 @@ async def _start(self):
),
f"Failed to create service {service_manifest.metadata.name}",
)
except Exception:
# cleanup on failure and re-raise
await self.stop(True)
raise

except Exception:
# cleanup on failure and re-raise
await self.stop(True)
raise

# we need a timeout here even though start itself has a timeout
# in order for this coroutine to finish at some point.
Expand Down
Loading