Add whitelist for environment variables to be inherited from gateway process by kernel.

[GrahamDumpleton]

As raised in #279, this PR adds ability to define a whitelist of environment variables which are allowed to be inherited from the kernel gateway process, when spawning a kernel process. This is needed to allow environment variables such as LD_LIBRARY_PATH, LD_PRELOAD or other special environment variables to be passed through when executing the kernel process. In the case of LD_LIBRARY_PATH this avoids error such as:

[KernelGatewayApp] KernelRestarter: restarting kernel (3/5), new random ports
[KernelGatewayApp] Starting kernel: ['/opt/app-root/bin/python3', '-m', 'ipykernel_launcher', '-f', '/opt/app-root/src/.local/share/jupyter/runtime/kernel-5f5bb348-3c3a-4ec5-aea2-fafba2c0cf3a.json']
/opt/app-root/bin/python3: error while loading shared libraries: libpython3.5m.so.rh-python35-1.0: cannot open shared object file: No such file or directory

The list of environment variables allowed to be inherited can be set by KG_ENV_PROCESS_WHITELIST or KernelGatewayApp.env_process_whitelist. Used env_process_whitelist instead of process_whitelist on KernelGatewayApp in the end so clearly distinguished from JupyterWebsocketPersonality.env_whitelist which is whitelist of environment variables that can be passed from client making request to start kernel. Need to be different to also avoid conflict with KG_ENV_WHITELIST environment variable.

[rolweber]

Travis CI detects problems with all tested Python versions except 3.3.
I'm not sure if this is something in the PR, or an infrastructure problem.

[rolweber]

The code changes look good to me.

Unfortunately, I currently don't have the time to dig further into the Travis failures. @parente, would you have some cycles to spare?

[rolweber]

This is the Travis CI problem for all failing versions:

ERROR: Failure: ImportError (cannot import name LogTrapTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.14/lib/python2.7/site-packages/nose/loader.py", line 418, in loadTestsFromName
    addr.filename, addr.module)
  File "/home/travis/virtualenv/python2.7.14/lib/python2.7/site-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/home/travis/virtualenv/python2.7.14/lib/python2.7/site-packages/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/home/travis/build/jupyter/kernel_gateway/kernel_gateway/tests/test_gatewayapp.py", line 10, in <module>
    from tornado.testing import AsyncHTTPTestCase, LogTrapTestCase
ImportError: cannot import name LogTrapTestCase

The PR doesn't mention LogTrapTestCase anywhere, so the problem is not in the code changes. Since this is Tornado related, it might have to do with the release of Tornado 5.0. I've seen that cause problems elsewhere, when the latest version is installed automatically.

[rolweber]

For example jupyter/notebook#3023

[rolweber]

Indeed, Tornado 5.0 dropped support for Python 3.3. So our Travis CI tests are failing on all Python versions for which Tornado 5.0 is available.

http://www.tornadoweb.org/en/stable/releases/v5.0.0.html

[kevin-bates]

@rolweber - We ran into this same problem in Enterprise Gateway (since all unittests are derived from JKG) and I had to block use of tornado >= 5.0. LogTrapTestCase was deprecated long ago and now removed in 5.0. However, its removal or replacement with the suggested class ExpectLog isn't straightforward and I couldn't take the time to figure it out. JEG and presumably JKG work fine otherwise with tornado 5.0, but I wanted a clean solution, then was going to apply the same here.

[kevin-bates]

@GrahamDumpleton @rolweber - Regarding the code changes for this PR, isn't the env: stanza in the kernelspec (i.e., kernel.json) file how things like server-side envs are addressed? For example, here's such an env stanza from one of the kernelspecs (samples) we provide in Enterprise Gateway.

I'm 99% certain that the env: stanza is what seeds the model structure that you referenced in #279. So if the LD_ envs you need where configured into the kernel.json file, you'd be good to go. In addition, some envs may not apply to all kernels (especially language-specific ones) and since JKG (and JEG) are intended to serve up multitudes of kernels, I think its best to let the existing configuration strategies take their course.

As a result, I'd have to down-vote this PR unless I'm missing something.

[rolweber]

kernelspecs are one way to define env variables. But if the setting is not specific to the kernel, but rather to the Jupyter installation, it makes sense to pass environment variables from the Jupyter process to the kernels as well. Then you can use kernelspecs you find somewhere on the internet unmodified.

[kevin-bates]

Thanks Roland - that makes sense (and is what Graham was trying to convey previously - sorry). In fact, the current use of PATH is such an example and, I suppose, one could argue that the default value for the new env_process_whitelist config entry should be PATH - although I'm not proposing that. At any rate, I retract my initial objection. Thanks @GrahamDumpleton.

[GrahamDumpleton]

Yep, Roland got it. This isn't trying to set environment variables because a specific kernel needed them, but that they were needed for underlying stuff such as Python itself, or some other reason related to the runtime.

For example, in container image I am running stuff in, I also need to ensure that LD_PRELOAD, NSS_WRAPPER_PASSWD and NS_WRAPPER_GROUP are passed through. These are used to force load a shared library into all applications which fakes out passwd/group entries for users that don't exist in system passwd and group files. If don't return valid entries, various Python packages can fail. In Jupyter docker-stacks images didn't need that NSS mess luckily as used better solution of making passwd and group files writable so could add entries from start.sh.

• https://github.com/jupyter/docker-stacks/blob/master/base-notebook/start.sh#L70

As to having PATH in env_process_whitelist I saw that as too dangerous as you have to have that. If a user overrides the configuration and doesn't include PATH in their overrides, it would break things. So PATH has to always be there no matter what.

[lresende]

LGTM, the CI build issues are unrelated and I will submit a fix for that.

[rolweber]

I merged the fix for the Travis CI builds and requested a new build for this PR.

[rolweber]

Thanks to you, Graham and Luciano!