Deprecate env-whitelist traits, send client-envs in kernelspec

Author: kevin-bates

enterprise_gateway/mixins.py

@@ -372,30 +372,53 @@ def list_kernels_default(self) -> bool:
             == "true"
         )
 
-    env_whitelist_env = "EG_ENV_WHITELIST"
     env_whitelist = ListTrait(
         config=True,
-        help="""Environment variables allowed to be set when a client requests a
-                         new kernel. Use '*' to allow all environment variables sent in the request.
-                         (EG_ENV_WHITELIST env var)""",
+        help="""DEPRECATED, use allowed_envs.""",
     )
 
     @default("env_whitelist")
     def env_whitelist_default(self) -> List[str]:
         return os.getenv(self.env_whitelist_env, os.getenv("KG_ENV_WHITELIST", "")).split(",")
 
-    env_process_whitelist_env = "EG_ENV_PROCESS_WHITELIST"
+    @observe("env_whitelist")
+    def _update_env_whitelist(self, change):
+        self.log.warning("env_whitelist is deprecated, use client_envs")
+        self.client_envs = change["new"]
+
+    client_envs_env = "EG_CLIENT_ENVS"
+    client_envs = ListTrait(
+        config=True,
+        help="""Environment variables allowed to be set when a client requests a
+                               new kernel. (EG_CLIENT_ENVS env var)""",
+    )
+
+    @default("client_envs")
+    def client_envs_default(self):
+        return os.getenv(self.client_envs_env, os.getenv("EG_ENV_WHITELIST", "")).split(",")
+
     env_process_whitelist = ListTrait(
+        config=True,
+        help="""DEPRECATED, use inherited_envs""",
+    )
+
+    @observe("env_process_whitelist")
+    def _update_env_process_whitelist(self, change):
+        self.log.warning("env_process_whitelist is deprecated, use inherited_envs")
+        self.inherited_envs = change["new"]
+
+    inherited_envs_env = "EG_INHERITED_ENVS"
+    inherited_envs = ListTrait(
         config=True,
         help="""Environment variables allowed to be inherited
-                                 from the spawning process by the kernel. (EG_ENV_PROCESS_WHITELIST env var)""",
+                                from the spawning process by the kernel. (EG_INHERITED_ENVS env var)""",
     )
 
-    @default("env_process_whitelist")
-    def env_process_whitelist_default(self) -> List[str]:
-        return os.getenv(
-            self.env_process_whitelist_env, os.getenv("KG_ENV_PROCESS_WHITELIST", "")
-        ).split(",")
+    @default("inherited_envs")
+    def inherited_envs_default(self) -> List[str]:
+        return os.getenv(self.inherited_envs_env, os.getenv("EG_ENV_PROCESS_WHITELIST", "")).split(
+            ","
+        )
 
     kernel_headers_env = "EG_KERNEL_HEADERS"
     kernel_headers = ListTrait(

enterprise_gateway/services/kernelspecs/handlers.py

@@ -69,7 +69,7 @@ def kernel_spec_cache(self) -> KernelSpecCache:
 
     @web.authenticated
     async def get(self) -> None:
-        ksm = self.kernel_spec_cache
+        ksc = self.kernel_spec_cache
         km = self.kernel_manager
         model = {}
         model["default"] = km.default_kernel_name
@@ -82,7 +82,7 @@ async def get(self) -> None:
             if kernel_user:
                 self.log.debug("Searching kernels for user '%s' " % kernel_user)
 
-        kspecs = await ensure_async(ksm.get_all_specs())
+        kspecs = await ensure_async(ksc.get_all_specs())
 
         list_kernels_found = []
         for kernel_name, kernel_info in kspecs.items():
@@ -122,14 +122,14 @@ def kernel_spec_cache(self) -> KernelSpecCache:
 
     @web.authenticated
     async def get(self, kernel_name: str) -> None:
-        ksm = self.kernel_spec_cache
+        ksc = self.kernel_spec_cache
         kernel_name = url_unescape(kernel_name)
         kernel_user_filter = self.request.query_arguments.get("user")
         kernel_user = None
         if kernel_user_filter:
             kernel_user = kernel_user_filter[0].decode("utf-8")
         try:
-            spec = await ensure_async(ksm.get_kernel_spec(kernel_name))
+            spec = await ensure_async(ksc.get_kernel_spec(kernel_name))
         except KeyError:
             raise web.HTTPError(404, "Kernel spec %s not found" % kernel_name)
         if is_kernelspec_model(spec):
@@ -166,9 +166,9 @@ def initialize(self) -> None:
 
     @web.authenticated
     async def get(self, kernel_name: str, path: str, include_body: bool = True) -> None:
-        ksm = self.kernel_spec_cache
+        ksc = self.kernel_spec_cache
         try:
-            kernelspec = await ensure_async(ksm.get_kernel_spec(kernel_name))
+            kernelspec = await ensure_async(ksc.get_kernel_spec(kernel_name))
             self.root = kernelspec.resource_dir
         except KeyError as e:
             raise web.HTTPError(404, "Kernel spec %s not found" % kernel_name) from e

enterprise_gateway/services/kernelspecs/kernelspec_cache.py

@@ -3,7 +3,7 @@
 """Cache handling for kernel specs."""
 
 import os
-from typing import Dict, Optional, Union
+from typing import Dict, Optional, Set, Union
 
 from jupyter_client.kernelspec import KernelSpec
 from jupyter_server.utils import ensure_async
@@ -33,7 +33,7 @@ class KernelSpecCache(SingletonConfigurable):
 
     cache_enabled_env = "EG_KERNELSPEC_CACHE_ENABLED"
     cache_enabled = CBool(
-        False,
+        True,
         config=True,
         help="""Enable Kernel Specification caching. (EG_KERNELSPEC_CACHE_ENABLED env var)""",
     )
@@ -110,7 +110,7 @@ def get_item(self, kernel_name: str) -> Optional[KernelSpec]:
                 )
         return kernelspec
 
-    def get_all_items(self) -> Optional[Dict[str, CacheItemType]]:
+    def get_all_items(self) -> Dict[str, CacheItemType]:
         """Retrieves all kernel specification from the cache.
 
         If cache is disabled or no items are in the cache, an empty dictionary is returned;
@@ -140,6 +140,34 @@ def put_item(self, kernel_name: str, cache_item: Union[KernelSpec, CacheItemType
                 kernel_name=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:
             if type(cache_item) is KernelSpec:
                 cache_item = KernelSpecCache.kernel_spec_to_cache_item(cache_item)
@@ -159,9 +187,8 @@ def put_item(self, kernel_name: str, cache_item: Union[KernelSpec, CacheItemType
 
     def put_all_items(self, kernelspecs: Dict[str, CacheItemType]) -> None:
         """Adds or updates a dictionary of kernel specification in the cache."""
-        if self.cache_enabled and kernelspecs:
-            for kernel_name, cache_item in kernelspecs.items():
-                self.put_item(kernel_name, cache_item)
+        for kernel_name, cache_item in kernelspecs.items():
+            self.put_item(kernel_name, cache_item)
 
     def remove_item(self, kernel_name: str) -> Optional[CacheItemType]:
         """Removes the cache item corresponding to kernel_name from the cache."""
@@ -212,7 +239,7 @@ def _initialize(self):
 
     @staticmethod
     def kernel_spec_to_cache_item(kernelspec: KernelSpec) -> CacheItemType:
-        """Convets a KernelSpec instance to a CacheItemType for storage into the cache."""
+        """Converts a KernelSpec instance to a CacheItemType for storage into the cache."""
         cache_item = dict()
         cache_item["spec"] = kernelspec.to_dict()
         cache_item["resource_dir"] = kernelspec.resource_dir
@@ -221,7 +248,8 @@ def kernel_spec_to_cache_item(kernelspec: KernelSpec) -> CacheItemType:
     @staticmethod
     def cache_item_to_kernel_spec(cache_item: CacheItemType) -> KernelSpec:
         """Converts a CacheItemType to a KernelSpec instance for user consumption."""
-        return KernelSpec.from_resource_dir(cache_item["resource_dir"])
+        kernel_spec = KernelSpec(resource_dir=cache_item["resource_dir"], **cache_item["spec"])
+        return kernel_spec
 
 
 class KernelSpecChangeHandler(FileSystemEventHandler):