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

Add /bin to sudo secure_path #529

Merged
merged 1 commit into from
Jan 19, 2018
Merged

Add /bin to sudo secure_path #529

merged 1 commit into from
Jan 19, 2018

Conversation

nbonnotte
Copy link
Contributor

see issue #524

cf. PR #525

@parente
Copy link
Member

parente commented Jan 10, 2018

@nbonnotte Thanks for the PR. It looks like the tests failed waiting for a notebook server to startup in many cases. I'll retry them, but it's possible the change in start.sh broke something that needs local debugging.

@@ -77,7 +77,6 @@ def test_sudo_path(container):
c = container.run(
tty=True,
user='root',
environment=['GRANT_SUDO=yes'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice to keep the GRANT_SUDO option in this test to check that jovyan can run the sudo which jupyter command, and write another test (or parameterize this one) to check the case where the container launches as root without granting the user sudo.

@parente
Copy link
Member

parente commented Jan 17, 2018

Replicating the test that is hanging:

~/p/docker-stacks ❯❯❯ docker run -it --rm --user root jupyter/base-notebook start.sh sudo which jupyter
Set username to: jovyan
usermod: no changes
Executing the command: sudo which jupyter
[sudo] password for jovyan:

The issue is that start.sh does exec sudo -E -H -u $NB_USER PATH=$PATH $cmd to switch to the $NB_USER which is jovyan by default. But then as jovyan, it's trying to execute the sudo which jupyter command given in the test or on the command line above.

When running the container as --user root with the start.sh script, the sudo test is implicit. I think we just need the new test to check which jupyter instead of sudo which jupyter. That or the test should not use start.sh and just run sudo which jupyter directly.

@nbonnotte
Copy link
Contributor Author

@parente The tests are now passing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants