Skip to content

Rename allowed env configuration traits #1137

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

Merged
merged 4 commits into from
Aug 2, 2022
Merged
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
2 changes: 1 addition & 1 deletion docs/source/developers/custom-images.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
15 changes: 10 additions & 5 deletions docs/source/operators/config-cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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'
Expand Down
6 changes: 4 additions & 2 deletions docs/source/operators/config-kernel-override.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
4 changes: 2 additions & 2 deletions enterprise_gateway/enterprisegatewayapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
45 changes: 32 additions & 13 deletions enterprise_gateway/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion enterprise_gateway/services/api/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
2 changes: 1 addition & 1 deletion enterprise_gateway/services/api/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
32 changes: 16 additions & 16 deletions enterprise_gateway/services/kernels/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
)

Expand Down
10 changes: 5 additions & 5 deletions enterprise_gateway/services/kernels/remotemanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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.
"""
Expand All @@ -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
}
)

Expand Down
12 changes: 6 additions & 6 deletions enterprise_gateway/services/kernelspecs/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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():
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down
20 changes: 8 additions & 12 deletions enterprise_gateway/services/kernelspecs/kernelspec_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)""",
)
Expand Down Expand Up @@ -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;
Expand All @@ -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)

Expand All @@ -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."""
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand Down
4 changes: 1 addition & 3 deletions enterprise_gateway/services/processproxies/k8s.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions enterprise_gateway/tests/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand Down
2 changes: 2 additions & 0 deletions enterprise_gateway/tests/test_kernelspec_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -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!"

Expand Down Expand Up @@ -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")

Expand Down
3 changes: 2 additions & 1 deletion etc/docker/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
Loading