Merge pull request #642 from dolfinus/expand_environment

Expand environment variables
jupyterhub · Nov 9, 2022 · b8472fa · b8472fa
2 parents 4960fcf + 6753073
commit b8472fa
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 14 deletions.
diff --git a/.gitignore b/.gitignore
@@ -38,6 +38,7 @@ lib64/
 parts/
 sdist/
 var/
+venv/
 *.egg-info/
 .installed.cfg
 *.egg

diff --git a/docs/source/changelog.md b/docs/source/changelog.md
@@ -4,6 +4,12 @@
 
 ## [Unreleased]
 
+#### Breaking changes
+
+- Now `c.KubeSpawner.environment` values supports substitution, just like other config options [#642](https://github.com/jupyterhub/kubespawner/pull/642) ([@dolfinus](https://github.com/dolfinus)).
+  For example, `{"MYVAR": "jupyterhub-{username}"}` will rendered as `{"MYVAR": "jupyterhub-sam"}` for a user named `sam`.
+  This could break backward compatibility if environment variable value contains strings like `{something}` there `something` is a substitution variable unknown for the KubeSpawner. You should escape braces by using `{{something}}` syntax.
+
 ## 4.3
 
 ### [4.3.0] - 2022-11-03

diff --git a/kubespawner/spawner.py b/kubespawner/spawner.py
@@ -2055,7 +2055,7 @@ async def get_pod_manifest(self):
             allow_privilege_escalation=self.allow_privilege_escalation,
             container_security_context=csc,
             pod_security_context=psc,
-            env=self.get_env(),
+            env=self._expand_all(self.get_env()),
             volumes=self._expand_all(self.volumes),
             volume_mounts=self._expand_all(self.volume_mounts),
             working_dir=self.working_dir,

diff --git a/tests/test_spawner.py b/tests/test_spawner.py
@@ -708,12 +708,12 @@ async def test_variable_expansion(ssl_app):
         },
         "extra_labels": {
             "configured_value": {"dummy": "common-extra-labels-{username}"},
-            "findable_value": "common-extra-labels-user1",
+            "findable_values": ["common-extra-labels-user1"],
             "findable_in": ["pod", "service", "secret"],
         },
         "extra_annotations": {
             "configured_value": {"dummy": "common-extra-annotations-{username}"},
-            "findable_value": "common-extra-annotations-user1",
+            "findable_values": ["common-extra-annotations-user1"],
             "findable_in": ["pod", "service", "secret"],
         },
         "working_dir": {
@@ -742,6 +742,31 @@ async def test_variable_expansion(ssl_app):
             ],
             "findable_in": ["pod"],
         },
+        "environment": {
+            "configured_value": {
+                'VAR1': 'VAR1-{username}',
+                'VAR2': {
+                    'value': 'VAR2-{username}',
+                },
+                'VAR3': {
+                    'valueFrom': {
+                        'secretKeyRef': {
+                            'name': 'VAR3-SECRET-{username}',
+                            'key': 'VAR3-KEY-{username}',
+                        },
+                    },
+                },
+                'VAR4': 'VAR4-{{username}}-{{SHELL}}',
+            },
+            "findable_values": [
+                "VAR1-user1",
+                "VAR2-user1",
+                "VAR3-SECRET-user1",
+                "VAR3-KEY-user1",
+                "VAR4-{username}-{SHELL}",
+            ],
+            "findable_in": ["pod"],
+        },
         "init_containers": {
             "configured_value": [
                 {
@@ -770,8 +795,8 @@ async def test_variable_expansion(ssl_app):
     for key, value in config_to_test.items():
         c.KubeSpawner[key] = value["configured_value"]
 
-        if "findable_value" not in value:
-            value["findable_value"] = key.replace("_", "-") + "-user1"
+        if "findable_values" not in value:
+            value["findable_values"] = [key.replace("_", "-") + "-user1"]
 
     user = MockUser(name="user1")
     spawner = KubeSpawner(
@@ -795,15 +820,16 @@ async def test_variable_expansion(ssl_app):
         manifest_string = str(manifest)
         for config in config_to_test.values():
             if resource_kind in config["findable_in"]:
-                assert config["findable_value"] in manifest_string, (
-                    manifest_string
-                    + "\n\n"
-                    + "finable_value: "
-                    + config["findable_value"]
-                    + "\n"
-                    + "resource_kind: "
-                    + resource_kind
-                )
+                for value in config["findable_values"]:
+                    assert value in manifest_string, (
+                        manifest_string
+                        + "\n\n"
+                        + "findable_value: "
+                        + value
+                        + "\n"
+                        + "resource_kind: "
+                        + resource_kind
+                    )
 
 
 async def test_url_changed(kube_ns, kube_client, config, hub_pod, hub):