Skip to content
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

wip: simplify and automate forwarding of env vars [server -> kernel] and [gateway -> kernel] #1000

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
18 changes: 9 additions & 9 deletions docs/source/config-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ To see the same configuration options at the command line, run the following:
jupyter enterprisegateway --help-all
```

A snapshot of this help appears below for ease of reference on the web.
A snapshot of this help appears below for ease of reference on the web.

```
Jupyter Enterprise Gateway
Expand Down Expand Up @@ -142,14 +142,14 @@ EnterpriseGatewayApp(EnterpriseGatewayConfigMixin, JupyterApp) options
A value of 0 or less disables dynamic config updates.
(EG_DYNAMIC_CONFIG_INTERVAL env var)
Default: 0
--EnterpriseGatewayApp.env_process_whitelist=<list-item-1>...
--EnterpriseGatewayApp.kernel_env_inherit=<list-item-1>...
Environment variables allowed to be inherited from the spawning process by
the kernel. (EG_ENV_PROCESS_WHITELIST env var)
the kernel. (EG_KERNEL_ENV_INHERIT env var)
Default: []
--EnterpriseGatewayApp.env_whitelist=<list-item-1>...
--EnterpriseGatewayApp.kernel_env_forward=<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)
(EG_KERNEL_ENV_FORWARD env var)
Default: []
--EnterpriseGatewayApp.expose_headers=<Unicode>
Sets the Access-Control-Expose-Headers header. (EG_EXPOSE_HEADERS env var)
Expand Down Expand Up @@ -433,10 +433,10 @@ The following environment variables can be used to influence functionality and a
in an exception indicating error 403 (Forbidden). See also EG_PROHIBITED_GIDS.

EG_RESPONSE_IP=None
Experimental. The IP address to use to formulate the response address (with
`EG_RESPONSE_PORT`). By default, the server's IP is used. However, we may find
Experimental. The IP address to use to formulate the response address (with
`EG_RESPONSE_PORT`). By default, the server's IP is used. However, we may find
it necessary to use a different IP in cases where the target kernels are external
to the Enterprise Gateway server (for example). It's value may also need to be
to the Enterprise Gateway server (for example). It's value may also need to be
set in cases where the computed (default) is not correct for the current topology.

EG_RESPONSE_PORT=8877
Expand Down Expand Up @@ -491,7 +491,7 @@ The following environment variables may be useful for troubleshooting:
should rarely be necessary.

EG_POLL_INTERVAL=0.5
The interval (in seconds) to wait before checking poll results again.
The interval (in seconds) to wait before checking poll results again.

EG_REMOVE_CONTAINER=True
Used by launch_docker.py, indicates whether the kernel's docker container should be
Expand Down
4 changes: 2 additions & 2 deletions enterprise_gateway/enterprisegatewayapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ def init_webapp(self):
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_kernel_env_forward=self.kernel_env_forward,
eg_kernel_env_inherit=self.kernel_env_inherit,
eg_kernel_headers=self.kernel_headers,
eg_list_kernels=self.list_kernels,
eg_authorized_users=self.authorized_users,
Expand Down
46 changes: 32 additions & 14 deletions enterprise_gateway/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,24 @@ def write_error(self, status_code, **kwargs):
self.finish(json.dumps(reply))


class KernelEnvMixin:
# @property
# def kernel_env_forward(self):
# return self.settings['eg_kernel_env_forward']

@property
def kernel_env_keys(self):
km = self.settings['kernel_manager']
if km.kernel_spec is not None and "env" in km.kernel_spec:
return tuple(km.kernel_spec['env'].keys())
else:
return None

@property
def kernel_env_inherit(self):
return self.settings['eg_kernel_env_inherit']


class EnterpriseGatewayConfigMixin(Configurable):
# Server IP / PORT binding
port_env = 'EG_PORT'
Expand Down Expand Up @@ -313,24 +331,24 @@ def default_kernel_name_default(self):
def list_kernels_default(self):
return os.getenv(self.list_kernels_env, os.getenv('KG_LIST_KERNELS', 'False')).lower() == 'true'

env_whitelist_env = 'EG_ENV_WHITELIST'
env_whitelist = List(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)""")
# kernel_env_forward_env = 'EG_KERNEL_ENV_FORWARD'
# kernel_env_forward = List(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_KERNEL_ENV_FORWARD env var)""")

@default('env_whitelist')
def env_whitelist_default(self):
return os.getenv(self.env_whitelist_env, os.getenv('KG_ENV_WHITELIST', '')).split(',')
# @default('kernel_env_forward')
# def kernel_env_forward_default(self):
# return os.getenv(self.kernel_env_forward_env, os.getenv('KG_KERNEL_ENV_FORWARD', '')).split(',')

env_process_whitelist_env = 'EG_ENV_PROCESS_WHITELIST'
env_process_whitelist = List(config=True,
kernel_env_inherit_env = 'EG_KERNEL_ENV_INHERIT'
kernel_env_inherit = List(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_KERNEL_ENV_INHERIT env var)""")

@default('env_process_whitelist')
def env_process_whitelist_default(self):
return os.getenv(self.env_process_whitelist_env, os.getenv('KG_ENV_PROCESS_WHITELIST', '')).split(',')
@default('kernel_env_inherit')
def kernel_env_inherit_default(self):
return os.getenv(self.kernel_env_inherit_env, os.getenv('KG_KERNEL_ENV_INHERIT', '')).split(',')

kernel_headers_env = 'EG_KERNEL_HEADERS'
kernel_headers = List(config=True,
Expand Down
6 changes: 4 additions & 2 deletions enterprise_gateway/services/api/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,15 @@
"tags": [
"kernelspecs"
],
"parameters": {
"parameters": [
{
"name": "user",
"required": false,
"in": "query",
"description": "When present, kernelspec results will be filtered based on the configured authorization of specified value.",
"type": "string"
},
}
],
"responses": {
"200": {
"description": "If no query parameter is specified, all kernel specs will be returned; otherwise the result set is filtered based on the query parameter.",
Expand Down
31 changes: 11 additions & 20 deletions enterprise_gateway/services/kernels/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,19 @@
from jupyter_client.jsonutil import date_default
from tornado import web
from functools import partial
from ...mixins import TokenAuthorizationMixin, CORSMixin, JSONErrorsMixin
from ...mixins import CORSMixin, JSONErrorsMixin, KernelEnvMixin, TokenAuthorizationMixin


class MainKernelHandler(TokenAuthorizationMixin,
CORSMixin,
JSONErrorsMixin,
KernelEnvMixin,
jupyter_server_handlers.MainKernelHandler):
"""Extends the jupyter_server main kernel handler with token auth, CORS, and
JSON errors.
"""

@property
def env_whitelist(self):
return self.settings['eg_env_whitelist']

@property
def env_process_whitelist(self):
return self.settings['eg_env_process_whitelist']

async def post(self):
"""Overrides the super class method to manage env in the request body.
Expand All @@ -50,19 +45,15 @@ async def post(self):
if model is not None and 'env' in model:
if not isinstance(model['env'], dict):
raise tornado.web.HTTPError(400)
# 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
env.update({key: value for key, value in os.environ.items()
if key in self.env_process_whitelist})
# 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()
env.update({key: value for key, value in model['env'].items()
if key.startswith('KERNEL_') or key in env_whitelist})

env = {
# always inherit PATH from the gateway env
**{'PATH': os.getenv('PATH', '')},
# inherit vars from the gateway env if var name in self.env_inherit
**{k:v for k, v in os.environ.items() if k in self.kernel_env_inherit},
# forward vars from request env if var name starts with KERNEL_ or in self.env_forward
**{k:v for k, v in model['env'].items() if k.startswith('KERNEL_') or k in self.kernel_env_keys},
}

# If kernel_headers are configured, fetch each of those and include in start request
kernel_headers = {}
Expand Down
22 changes: 11 additions & 11 deletions enterprise_gateway/services/kernels/remotemanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

from ..processproxies.processproxy import LocalProcessProxy, RemoteProcessProxy
from ..sessions.kernelsessionmanager import KernelSessionManager
from enterprise_gateway.mixins import EnterpriseGatewayConfigMixin
from ...mixins import EnterpriseGatewayConfigMixin, KernelEnvMixin


def get_process_proxy_config(kernelspec):
Expand Down Expand Up @@ -284,7 +284,7 @@ def new_kernel_id(self, **kwargs):
return new_kernel_id(kernel_id_fn=super(RemoteMappingKernelManager, self).new_kernel_id, log=self.log, **kwargs)


class RemoteKernelManager(EnterpriseGatewayConfigMixin, AsyncIOLoopKernelManager):
class RemoteKernelManager(EnterpriseGatewayConfigMixin, KernelEnvMixin, AsyncIOLoopKernelManager):
"""
Extends the AsyncIOLoopKernelManager used by the RemoteMappingKernelManager.

Expand Down Expand Up @@ -338,8 +338,8 @@ def _link_dependent_props(self):
"port_range",
"impersonation_enabled",
"max_kernels_per_user",
"env_whitelist",
"env_process_whitelist",
#"kernel_env_forward",
"kernel_env_inherit",
"yarn_endpoint",
"alt_yarn_endpoint",
"yarn_endpoint_security_enabled",
Expand All @@ -366,15 +366,15 @@ 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
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.
Make a copy of any whitelist 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.
"""
env = kwargs.get('env', {})
self.user_overrides.update({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})
self.user_overrides.update({k:v for k, v in env.items()
if k.startswith('KERNEL_') or
k in self.kernel_env_inherit or
k in self.kernel_env_keys})

def format_kernel_cmd(self, extra_arguments=None):
"""
Expand Down
21 changes: 7 additions & 14 deletions enterprise_gateway/tests/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,19 @@ def setup_app(self):
os.environ['JUPYTER_PATH'] = RESOURCES

# These are required for setup of test_kernel_defaults
os.environ['EG_ENV_PROCESS_WHITELIST'] = "PROCESS_VAR1,PROCESS_VAR2"
os.environ['EG_KERNEL_ENV_INHERIT'] = "PROCESS_VAR1,PROCESS_VAR2"
os.environ['PROCESS_VAR1'] = "process_var1_override"

self.app.env_whitelist = ['TEST_VAR', 'OTHER_VAR1', 'OTHER_VAR2']
# self.app.kernel_env_forward = ['TEST_VAR', 'OTHER_VAR1', 'OTHER_VAR2']

def tearDown(self):
"""Shuts down the app after test run."""

# Clean out items added to env
if 'JUPYTER_PATH' in os.environ:
os.environ.pop('JUPYTER_PATH')
if 'EG_ENV_PROCESS_WHITELIST' in os.environ:
os.environ.pop('EG_ENV_PROCESS_WHITELIST')
if 'EG_KERNEL_ENV_INHERIT' in os.environ:
os.environ.pop('EG_KERNEL_ENV_INHERIT')
if 'PROCESS_VAR1' in os.environ:
os.environ.pop('PROCESS_VAR1')

Expand Down Expand Up @@ -501,8 +501,7 @@ def test_json_errors(self):
@gen_test
def test_kernel_env(self):
"""Kernel should start with environment vars defined in the request."""
# Note: Only envs in request prefixed with KERNEL_ or in env_whitelist (TEST_VAR)
# with the exception of KERNEL_GATEWAY - which is "system owned".
# env vars are forwarded from request if key starts with KERNEL_ or key in kernel env stanza
kernel_body = json.dumps({
'name': 'python',
'env': {
Expand Down Expand Up @@ -530,11 +529,8 @@ def test_kernel_env(self):

@gen_test
def test_kernel_defaults(self):
"""Kernel should start with env vars defined in request overriding env vars defined in kernelspec."""

# Note: Only envs in request prefixed with KERNEL_ or in env_whitelist (OTHER_VAR1, OTHER_VAR2)
# with the exception of KERNEL_GATEWAY - which is "system owned" - will be set in kernel env.
# Since OTHER_VAR1 is not in the request, its existing value in kernel.json will be used.
"""Kernel should start with env ars defined in request overriding env vars defined in kernelspec."""
# env vars are forwarded from request if key starts with KERNEL_ or key in kernel env stanza

# NOTE: This test requires use of the kernels/kernel_defaults_test/kernel.json file.
kernel_body = json.dumps({
Expand Down Expand Up @@ -686,13 +682,10 @@ 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': {
Expand Down