Revert changes to drop wildcard support and send client_envs in kspec

Author: kevin-bates

enterprise_gateway/services/kernels/handlers.py

@@ -58,12 +58,20 @@ async def post(self):
             env.update(
                 {key: value for key, value in os.environ.items() if key in self.inherited_envs}
             )
+            # Allow all KERNEL_* envs and those specified in client_envs and set from client.  If this EG
+            # instance is configured to accept all envs in the payload (i.e., client_envs == '*'), go ahead
+            # and add those keys to the "working" allowed_envs list, otherwise, just transfer the configured envs.
+            allowed_envs: List[str]
+            if self.client_envs == ["*"]:
+                allowed_envs = model["env"].keys()
+            else:
+                allowed_envs = self.client_envs
             # Allow KERNEL_* args and those allowed by configuration.
             env.update(
                 {
                     key: value
                     for key, value in model["env"].items()
-                    if key.startswith("KERNEL_") or key in self.client_envs
+                    if key.startswith("KERNEL_") or key in allowed_envs
                 }
             )
 

enterprise_gateway/services/kernelspecs/kernelspec_cache.py

@@ -3,7 +3,7 @@
 """Cache handling for kernel specs."""
 
 import os
-from typing import Dict, Optional, Set, Union
+from typing import Dict, Optional, Union
 
 from jupyter_client.kernelspec import KernelSpec
 from jupyter_server.utils import ensure_async
@@ -135,36 +135,8 @@ def put_item(self, kernel_name: str, cache_item: Union[KernelSpec, CacheItemType
         If it determines the cache entry corresponds to a currently unwatched directory,
         that directory will be added to list of observed directories and scheduled accordingly.
         """
-        self.log.info(f"KernelSpecCache: adding/updating kernelspec: {kernel_name}")
-        # Irrespective of cache enablement, add/update the 'metadata.client_envs' entry
-        # with the set of configured values.  If the stanza already exists in the kernelspec
-        # update with the union of it and those values configured via `EnterpriseGatewayApp'.
-        # We apply this logic here so that its only performed once for cached values or on
-        # every retrieval when caching is not enabled.
-        # Note: We only need to do this if we have a KernelSpec instance, since CacheItemType
-        # instances will have already been modified.
-
-        # Create a set from the configured value, update it with the (potential) value
-        # in the kernelspec, and apply the changes back to the kernelspec.
-
-        client_envs: Set[str] = set(self.parent.client_envs)
-        kspec_client_envs: Set[str]
-        if type(cache_item) is KernelSpec:
-            kspec: KernelSpec = cache_item
-            kspec_client_envs = set(kspec.metadata.get("client_envs", []))
-        else:
-            kspec_client_envs = set(cache_item["spec"].get("metadata", {}).get("client_envs", []))
-
-        client_envs.update(kspec_client_envs)
-        if type(cache_item) is KernelSpec:
-            kspec: KernelSpec = cache_item
-            kspec.metadata["client_envs"] = list(client_envs)
-        else:
-            if "metadata" not in cache_item["spec"]:
-                cache_item["spec"]["metadata"] = {}
-            cache_item["spec"]["metadata"]["client_envs"] = list(client_envs)
-
         if self.cache_enabled:
+            self.log.info(f"KernelSpecCache: adding/updating kernelspec: {kernel_name}")
             if type(cache_item) is KernelSpec:
                 cache_item = KernelSpecCache.kernel_spec_to_cache_item(cache_item)
 

enterprise_gateway/tests/test_handlers.py

@@ -644,44 +644,44 @@ def test_base_url(self):
         self.assertEqual(response.code, 200)
 
 
-# class TestWildcardEnvs(TestHandlers):
-#     """Base class for jupyter-websocket mode tests that spawn kernels."""
-#
-#     def setup_app(self):
-#         """Configure JUPYTER_PATH so that we can use local kernelspec files for testing."""
-#         super().setup_app()
-#         # overwrite env_whitelist
-#         self.app.env_whitelist = ["*"]
-#
-#     @gen_test
-#     def test_kernel_wildcard_env(self):
-#         """Kernel should start with environment vars defined in the request."""
-#         # Note: Since env_whitelist == '*', all values should be present.
-#         kernel_body = json.dumps(
-#             {
-#                 "name": "python",
-#                 "env": {
-#                     "KERNEL_FOO": "kernel-foo-value",
-#                     "OTHER_VAR1": "other-var1-value",
-#                     "OTHER_VAR2": "other-var2-value",
-#                     "TEST_VAR": "test-var-value",
-#                 },
-#             }
-#         )
-#         ws = yield self.spawn_kernel(kernel_body)
-#         req = self.execute_request(
-#             "import os; "
-#             'print(os.getenv("KERNEL_FOO"), '
-#             'os.getenv("OTHER_VAR1"), '
-#             'os.getenv("OTHER_VAR2"), '
-#             'os.getenv("TEST_VAR"))'
-#         )
-#         ws.write_message(json_encode(req))
-#         content = yield self.await_stream(ws)
-#         self.assertEqual(content["name"], "stdout")
-#         self.assertIn("kernel-foo-value", content["text"])
-#         self.assertIn("other-var1-value", content["text"])
-#         self.assertIn("other-var2-value", content["text"])
-#         self.assertIn("test-var-value", content["text"])
-#
-#         ws.close()
+class TestWildcardEnvs(TestHandlers):
+    """Base class for jupyter-websocket mode tests that spawn kernels."""
+
+    def setup_app(self):
+        """Configure JUPYTER_PATH so that we can use local kernelspec files for testing."""
+        super().setup_app()
+        # overwrite env_whitelist
+        self.app.env_whitelist = ["*"]
+
+    @gen_test
+    def test_kernel_wildcard_env(self):
+        """Kernel should start with environment vars defined in the request."""
+        # Note: Since env_whitelist == '*', all values should be present.
+        kernel_body = json.dumps(
+            {
+                "name": "python",
+                "env": {
+                    "KERNEL_FOO": "kernel-foo-value",
+                    "OTHER_VAR1": "other-var1-value",
+                    "OTHER_VAR2": "other-var2-value",
+                    "TEST_VAR": "test-var-value",
+                },
+            }
+        )
+        ws = yield self.spawn_kernel(kernel_body)
+        req = self.execute_request(
+            "import os; "
+            'print(os.getenv("KERNEL_FOO"), '
+            'os.getenv("OTHER_VAR1"), '
+            'os.getenv("OTHER_VAR2"), '
+            'os.getenv("TEST_VAR"))'
+        )
+        ws.write_message(json_encode(req))
+        content = yield self.await_stream(ws)
+        self.assertEqual(content["name"], "stdout")
+        self.assertIn("kernel-foo-value", content["text"])
+        self.assertIn("other-var1-value", content["text"])
+        self.assertIn("other-var2-value", content["text"])
+        self.assertIn("test-var-value", content["text"])
+
+        ws.close()

enterprise_gateway/tests/test_kernelspec_cache.py

@@ -12,7 +12,6 @@
 import pytest
 from jupyter_client.kernelspec import KernelSpecManager, NoSuchKernel
 
-from enterprise_gateway.enterprisegatewayapp import EnterpriseGatewayApp
 from enterprise_gateway.services.kernelspecs import KernelSpecCache
 
 
@@ -61,16 +60,10 @@ def environ(
 
 # END - Remove once transition to jupyter_server occurs
 
-GLOBAL_CLIENT_ENVS = ["TEST_VAR", "OTHER_VAR1", "OTHER_VAR2"]
-KSPEC_CLIENT_ENVS = ["TEST_VAR", "OTHER_VAR1", "OTHER_VAR3", "OTHER_VAR4"]
-UNION_CLIENT_ENVS = ["TEST_VAR", "OTHER_VAR1", "OTHER_VAR2", "OTHER_VAR3", "OTHER_VAR4"]
 
 kernelspec_json = {
     "argv": ["cat", "{connection_file}"],
     "display_name": "Test kernel: {kernel_name}",
-    "metadata": {
-        "client_envs": KSPEC_CLIENT_ENVS,
-    }
 }
 
 
@@ -110,22 +103,17 @@ def setup_kernelspecs(environ, kernelspec_location):
 
 @pytest.fixture
 def kernel_spec_manager(environ, setup_kernelspecs):
-    app = EnterpriseGatewayApp()
-    app.client_envs = ["TEST_VAR", "OTHER_VAR1", "OTHER_VAR2"]
-
-    yield KernelSpecManager(ensure_native_kernel=False, parent=app)
+    yield KernelSpecManager(ensure_native_kernel=False)
 
 
 @pytest.fixture
 def kernel_spec_cache(is_enabled, kernel_spec_manager):
-
     kspec_cache = KernelSpecCache.instance(
-        kernel_spec_manager=kernel_spec_manager,
-        cache_enabled=is_enabled,
-        parent=kernel_spec_manager.parent,
+        kernel_spec_manager=kernel_spec_manager, cache_enabled=is_enabled
     )
     yield kspec_cache
-    kspec_cache.clear_instance()
+    kspec_cache = None
+    KernelSpecCache.clear_instance()
 
 
 @pytest.fixture(params=[False, True])  # Add types as needed
@@ -146,14 +134,12 @@ async def tests_get_named_spec(kernel_spec_cache):
 async def tests_get_modified_spec(kernel_spec_cache):
     kspec = await kernel_spec_cache.get_kernel_spec("test2")
     assert kspec.display_name == "Test kernel: test2"
-    assert set(kspec.metadata['client_envs']) == set(UNION_CLIENT_ENVS)
 
     # Modify entry
     _modify_kernelspec(kspec.resource_dir, "test2")
     await asyncio.sleep(0.5)  # sleep for a half-second to allow cache to update item
     kspec = await kernel_spec_cache.get_kernel_spec("test2")
     assert kspec.display_name == "test2 modified!"
-    assert set(kspec.metadata['client_envs']) == set(UNION_CLIENT_ENVS)
 
 
 async def tests_add_spec(kernel_spec_cache, kernelspec_location, other_kernelspec_location):