diff --git a/docs/source/config-options.md b/docs/source/config-options.md index 7c0717944..31968ef49 100644 --- a/docs/source/config-options.md +++ b/docs/source/config-options.md @@ -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 @@ -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=... +--EnterpriseGatewayApp.kernel_env_inherit=... 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=... +--EnterpriseGatewayApp.kernel_env_forward=... 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= Sets the Access-Control-Expose-Headers header. (EG_EXPOSE_HEADERS env var) @@ -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 @@ -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 diff --git a/enterprise_gateway/enterprisegatewayapp.py b/enterprise_gateway/enterprisegatewayapp.py index a1b9e04e9..842673f45 100644 --- a/enterprise_gateway/enterprisegatewayapp.py +++ b/enterprise_gateway/enterprisegatewayapp.py @@ -147,23 +147,25 @@ def _create_request_handlers(self): """Create default Jupyter handlers and redefine them off of the base_url path. Assumes init_configurables() has already been called. """ - handlers = [] + default_auth_handlers = [ + (r"/login", self.auth_handler_class), + ] + + handlers_collection = ( + default_api_handlers, + default_auth_handlers, + default_kernel_handlers, + default_kernelspec_handlers, + default_session_handlers, + default_base_handlers, + ) # append tuples for the standard kernel gateway endpoints - for handler in ( - default_api_handlers + - default_kernel_handlers + - default_kernelspec_handlers + - default_session_handlers + - default_base_handlers - ): - # Create a new handler pattern rooted at the base_url - pattern = url_path_join('/', self.base_url, handler[0]) - # Some handlers take args, so retain those in addition to the - # handler class ref - new_handler = tuple([pattern] + list(handler[1:])) - handlers.append(new_handler) - return handlers + return [( + url_path_join('/', self.base_url, handler[0]), + *handler[1:], + ) for handlers in handlers_collection + for handler in handlers] def init_webapp(self): """Initializes Tornado web application with uri handlers. @@ -194,8 +196,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, @@ -205,6 +207,7 @@ def init_webapp(self): allow_origin=self.allow_origin, # Always allow remote access (has been limited to localhost >= notebook 5.6) allow_remote_access=True, + login_url=url_path_join('/', self.base_url, '/login'), # setting ws_ping_interval value that can allow it to be modified for the purpose of toggling ping mechanism # for zmq web-sockets or increasing/decreasing web socket ping interval/timeouts. ws_ping_interval=self.ws_ping_interval * 1000 diff --git a/enterprise_gateway/mixins.py b/enterprise_gateway/mixins.py index 8530ac0e7..dcbf361f6 100644 --- a/enterprise_gateway/mixins.py +++ b/enterprise_gateway/mixins.py @@ -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' @@ -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, diff --git a/enterprise_gateway/services/api/swagger.json b/enterprise_gateway/services/api/swagger.json index 03cb7e738..3372d1b58 100644 --- a/enterprise_gateway/services/api/swagger.json +++ b/enterprise_gateway/services/api/swagger.json @@ -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.", diff --git a/enterprise_gateway/services/kernels/handlers.py b/enterprise_gateway/services/kernels/handlers.py index a4f5705a2..8f37ae087 100644 --- a/enterprise_gateway/services/kernels/handlers.py +++ b/enterprise_gateway/services/kernels/handlers.py @@ -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. @@ -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 = {} diff --git a/enterprise_gateway/services/kernels/remotemanager.py b/enterprise_gateway/services/kernels/remotemanager.py index 36df28466..b54f18081 100644 --- a/enterprise_gateway/services/kernels/remotemanager.py +++ b/enterprise_gateway/services/kernels/remotemanager.py @@ -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): @@ -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. @@ -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", @@ -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): """ diff --git a/enterprise_gateway/tests/test_handlers.py b/enterprise_gateway/tests/test_handlers.py index 398c91c9e..5a098dd95 100644 --- a/enterprise_gateway/tests/test_handlers.py +++ b/enterprise_gateway/tests/test_handlers.py @@ -24,10 +24,10 @@ 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.""" @@ -35,8 +35,8 @@ def tearDown(self): # 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') @@ -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': { @@ -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({ @@ -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': {