Rename allowed env configuration traits (#1137)

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

* Address changes in docs and deployment scripts
* Update tests
* Revert changes to drop wildcard support and send client_envs in kspec

Author: kevin-bates

docs/source/developers/custom-images.md

@@ -164,6 +164,6 @@ cp -r python_kubernetes python_myCustomKernel
 }
 ```
 
-- If using a whitelist (`EG_KERNEL_WHITELIST`), be sure to update it with the new kernel specification directory name (e.g., `python_myCustomKernel`) and restart/redeploy Enterprise Gateway.
+- If using kernel filtering (`EG_ALLOWED_KERNELS`), be sure to update it with the new kernel specification directory name (e.g., `python_myCustomKernel`) and restart/redeploy Enterprise Gateway.
 - Launch or refresh your Notebook session and confirm `My Custom Kernel` appears in the _new kernel_ drop-down.
 - Create a new notebook using `My Custom Kernel`.

docs/source/operators/config-cli.md

@@ -121,6 +121,10 @@ EnterpriseGatewayApp(EnterpriseGatewayConfigMixin, JupyterApp) options
     The full path to a certificate authority certificate for SSL/TLS client
     authentication. (EG_CLIENT_CA env var)
     Default: None
+--EnterpriseGatewayApp.client_envs=<list-item-1>...
+    Environment variables allowed to be set when a client requests a
+    new kernel. (EG_CLIENT_ENVS env var)
+    Default: []
 --EnterpriseGatewayApp.conductor_endpoint=<Unicode>
     The http url for accessing the Conductor REST API. (EG_CONDUCTOR_ENDPOINT
     env var)
@@ -140,13 +144,10 @@ EnterpriseGatewayApp(EnterpriseGatewayConfigMixin, JupyterApp) options
     (EG_DYNAMIC_CONFIG_INTERVAL env var)
     Default: 0
 --EnterpriseGatewayApp.env_process_whitelist=<list-item-1>...
-    Environment variables allowed to be inherited from the spawning process by
-    the kernel. (EG_ENV_PROCESS_WHITELIST env var)
+    DEPRECATED, use inherited_envs
     Default: []
 --EnterpriseGatewayApp.env_whitelist=<list-item-1>...
-    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)
+    DEPRECATED, use client_envs.
     Default: []
 --EnterpriseGatewayApp.expose_headers=<Unicode>
     Sets the Access-Control-Expose-Headers header. (EG_EXPOSE_HEADERS env var)
@@ -158,6 +159,10 @@ EnterpriseGatewayApp(EnterpriseGatewayConfigMixin, JupyterApp) options
     Indicates whether impersonation will be performed during kernel launch.
     (EG_IMPERSONATION_ENABLED env var)
     Default: False
+--EnterpriseGatewayApp.inherited_envs=<list-item-1>...
+    Environment variables allowed to be inherited
+    from the spawning process by the kernel. (EG_INHERITED_ENVS env var)
+    Default: []
 --EnterpriseGatewayApp.ip=<Unicode>
     IP address on which to listen (EG_IP env var)
     Default: '127.0.0.1'

docs/source/operators/config-kernel-override.md

@@ -38,8 +38,10 @@ those same-named variables in the kernel.json `env` stanza.
 
 Environment variables for which this can occur are any variables prefixed with `KERNEL_`
 as well as any variables
-listed in the `EnterpriseGatewayApp.env_whitelist` configurable trait (or via
-the `EG_ENV_WHITELIST` variable). Locally defined variables listed in `EG_PROCESS_ENV_WHITELIST`
+listed in the `EnterpriseGatewayApp.client_envs` configurable trait (or via
+the `EG_CLIENT_ENVS` variable). Likewise, environment variables of the Enterprise Gateway
+server process listed in the `EnterpriseGatewayApp.inherited_envs` configurable trait
+(or via the `EG_INHERITED_ENVS` variable)
 are also available for replacement in the kernel process' environment.
 
 See [Kernel Environment Variables](../users/kernel-envs.md) in the Users documentation section for a complete set of recognized `KERNEL_` variables.

enterprise_gateway/enterprisegatewayapp.py

@@ -236,8 +236,8 @@ def init_webapp(self) -> None:
             eg_expose_headers=self.expose_headers,
             eg_max_age=self.max_age,
             eg_max_kernels=self.max_kernels,
-            eg_env_process_whitelist=self.env_process_whitelist,
-            eg_env_whitelist=self.env_whitelist,
+            eg_inherited_envs=self.inherited_envs,
+            eg_client_envs=self.client_envs,
             eg_kernel_headers=self.kernel_headers,
             eg_list_kernels=self.list_kernels,
             eg_authorized_users=self.authorized_users,

enterprise_gateway/mixins.py

@@ -372,30 +372,49 @@ def list_kernels_default(self) -> bool:
             == "true"
         )
 
-    env_whitelist_env = "EG_ENV_WHITELIST"
     env_whitelist = ListTrait(
+        config=True,
+        help="""DEPRECATED, use client_envs.""",
+    )
+
+    @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. Use '*' to allow all environment variables sent in the request.
-                         (EG_ENV_WHITELIST env var)""",
+                               new kernel. (EG_CLIENT_ENVS env var)""",
     )
 
-    @default("env_whitelist")
-    def env_whitelist_default(self) -> List[str]:
-        return os.getenv(self.env_whitelist_env, os.getenv("KG_ENV_WHITELIST", "")).split(",")
+    @default("client_envs")
+    def client_envs_default(self):
+        return os.getenv(self.client_envs_env, os.getenv("EG_ENV_WHITELIST", "")).split(",")
 
-    env_process_whitelist_env = "EG_ENV_PROCESS_WHITELIST"
     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/api/swagger.json

@@ -160,7 +160,7 @@
                 },
                 "env": {
                   "type": "object",
-                  "description": "A dictionary of environment variables and values to include in the kernel process - subject to whitelisting.",
+                  "description": "A dictionary of environment variables and values to include in the kernel process - subject to filtering.",
                   "additionalProperties": {
                     "type": "string"
                   }

enterprise_gateway/services/api/swagger.yaml

@@ -141,7 +141,7 @@ paths:
                 type: object
                 description: |
                   A dictionary of environment variables and values to include in the
-                  kernel process - subject to whitelisting.
+                  kernel process - subject to filtering.
                 additionalProperties:
                   type: string
       responses:

enterprise_gateway/services/kernels/handlers.py

@@ -22,12 +22,12 @@ class MainKernelHandler(
     """
 
     @property
-    def env_whitelist(self):
-        return self.settings["eg_env_whitelist"]
+    def client_envs(self):
+        return self.settings["eg_client_envs"]
 
     @property
-    def env_process_whitelist(self):
-        return self.settings["eg_env_process_whitelist"]
+    def inherited_envs(self):
+        return self.settings["eg_inherited_envs"]
 
     async def post(self):
         """Overrides the super class method to manage env in the request body.
@@ -54,24 +54,24 @@ async def post(self):
             # Start with the PATH from the current env. Do not provide the entire environment
             # which might contain server secrets that should not be passed to kernels.
             env = {"PATH": os.getenv("PATH", "")}
-            # Whitelist environment variables from current process environment
+            # Transfer inherited environment variables from current process
             env.update(
-                {
-                    key: value
-                    for key, value in os.environ.items()
-                    if key in self.env_process_whitelist
-                }
+                {key: value for key, value in os.environ.items() if key in self.inherited_envs}
             )
-            # Whitelist KERNEL_* args and those allowed by configuration from client.  If all
-            # envs are requested, just use the keys from the payload.
-            env_whitelist = self.env_whitelist
-            if env_whitelist == ["*"]:
-                env_whitelist = model["env"].keys()
+            # 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 env_whitelist
+                    if key.startswith("KERNEL_") or key in allowed_envs
                 }
             )
 

enterprise_gateway/services/kernels/remotemanager.py

@@ -427,8 +427,8 @@ def _link_dependent_props(self):
             "port_range",
             "impersonation_enabled",
             "max_kernels_per_user",
-            "env_whitelist",
-            "env_process_whitelist",
+            "client_envs",
+            "inherited_envs",
             "yarn_endpoint",
             "alt_yarn_endpoint",
             "yarn_endpoint_security_enabled",
@@ -456,7 +456,7 @@ async def start_kernel(self, **kwargs):
 
     def _capture_user_overrides(self, **kwargs):
         """
-        Make a copy of any whitelist or KERNEL_ env values provided by user.  These will be injected
+        Make a copy of any allowed or KERNEL_ env values provided by user.  These will be injected
         back into the env after the kernelspec env has been applied.  This enables defaulting behavior
         of the kernelspec env stanza that would have otherwise overridden the user-provided values.
         """
@@ -470,8 +470,8 @@ def _capture_user_overrides(self, **kwargs):
                 key: value
                 for key, value in env.items()
                 if key.startswith("KERNEL_")
-                or key in self.env_process_whitelist
-                or key in self.env_whitelist
+                or key in self.inherited_envs
+                or key in self.client_envs
             }
         )
 

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

@@ -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;
@@ -135,12 +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(
-            "KernelSpecCache: adding/updating kernelspec: {kernel_name}".format(
-                kernel_name=kernel_name
-            )
-        )
         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)
 
@@ -159,9 +155,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 +207,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 +216,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):

enterprise_gateway/services/processproxies/k8s.py

@@ -51,9 +51,7 @@ async def launch_process(
 
         # Kubernetes relies on many internal env variables.  Since EG is running in a k8s pod, we will
         # transfer its env to each launched kernel.
-        kwargs["env"] = dict(
-            os.environ, **kwargs["env"]
-        )  # FIXME: Should probably use process-whitelist in JKG #280
+        kwargs["env"] = dict(os.environ, **kwargs["env"])
         self.kernel_pod_name = self._determine_kernel_pod_name(**kwargs)
         self.kernel_namespace = self._determine_kernel_namespace(
             **kwargs

enterprise_gateway/tests/test_handlers.py

@@ -24,11 +24,12 @@ def setup_app(self):
         os.environ["JUPYTER_PATH"] = RESOURCES
 
         # These are required for setup of test_kernel_defaults
+        # Note: We still reference the DEPRECATED config parameter and environment variable so that
+        # we can test client_envs and inherited_envs, respectively.
+        self.app.env_whitelist = ["TEST_VAR", "OTHER_VAR1", "OTHER_VAR2"]
         os.environ["EG_ENV_PROCESS_WHITELIST"] = "PROCESS_VAR1,PROCESS_VAR2"
         os.environ["PROCESS_VAR1"] = "process_var1_override"
 
-        self.app.env_whitelist = ["TEST_VAR", "OTHER_VAR1", "OTHER_VAR2"]
-
     def tearDown(self):
         """Shuts down the app after test run."""
 

enterprise_gateway/tests/test_kernelspec_cache.py

@@ -137,6 +137,7 @@ async def tests_get_modified_spec(kernel_spec_cache):
 
     # 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!"
 
@@ -180,6 +181,7 @@ async def tests_remove_spec(kernel_spec_cache):
 
     assert kernel_spec_cache.cache_misses == 0
     shutil.rmtree(kspec.resource_dir)
+    await asyncio.sleep(0.5)  # sleep for a half-second to allow cache to remove item
     with pytest.raises(NoSuchKernel):
         await kernel_spec_cache.get_kernel_spec("test2")
 

etc/docker/docker-compose.yml

@@ -22,7 +22,8 @@ services:
       - "EG_DOCKER_NETWORK=${EG_DOCKER_NETWORK:-enterprise-gateway_enterprise-gateway}"
       - "EG_KERNEL_LAUNCH_TIMEOUT=${EG_KERNEL_LAUNCH_TIMEOUT:-60}"
       - "EG_CULL_IDLE_TIMEOUT=${EG_CULL_IDLE_TIMEOUT:-3600}"
-      - "EG_KERNEL_WHITELIST=${EG_KERNEL_WHITELIST:-'r_docker','python_docker','python_tf_docker','python_tf_gpu_docker','scala_docker'}"
+      # Use double-defaulting for B/C.  Support for EG_KERNEL_WHITELIST will be removed in a future release
+      - "EG_ALLOWED_KERNELS=${EG_ALLOWED_KERNELS:-${EG_KERNEL_WHITELIST:-'r_docker','python_docker','python_tf_docker','python_tf_gpu_docker','scala_docker'}}"
       - "EG_MIRROR_WORKING_DIRS=${EG_MIRROR_WORKING_DIRS:-False}"
       - "EG_RESPONSE_PORT=${EG_RESPONSE_PORT:-8877}"
       - "KG_PORT=${KG_PORT:-8888}"