Skip to content

Commit 42071c6

Browse files
consideRatioconsideRatio
authored
Merge pull request #1512 from maresb/rework-start-part-1
Rework of start.sh script, take 2
2 parents 70e51d3 + 4c47de9 commit 42071c6

File tree

2 files changed

+181
-94
lines changed

2 files changed

+181
-94
lines changed

‎base-notebook/start.sh

Lines changed: 123 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# Distributed under the terms of the Modified BSD License.
44

55
set -e
6+
echo "Running: start.sh" "$@"
67

78
# Exec the specified command or fall back on bash
89
if [ $# -eq 0 ]; then
@@ -11,140 +12,186 @@ else
1112
cmd=( "$@" )
1213
fi
1314

15+
# The run-hooks function looks for .sh scripts to source and executable files to
16+
# run within a passed directory.
1417
run-hooks () {
15-
# Source scripts or run executable files in a directory
1618
if [[ ! -d "${1}" ]] ; then
1719
return
1820
fi
19-
echo "${0}: running hooks in ${1}"
21+
echo "${0}: running hooks in ${1} as uid / gid: $(id -u) / $(id -g)"
2022
for f in "${1}/"*; do
2123
case "${f}" in
2224
*.sh)
23-
echo "${0}: running ${f}"
25+
echo "${0}: running script ${f}"
2426
# shellcheck disable=SC1090
2527
source "${f}"
2628
;;
2729
*)
2830
if [[ -x "${f}" ]] ; then
29-
echo "${0}: running ${f}"
31+
echo "${0}: running executable ${f}"
3032
"${f}"
3133
else
32-
echo "${0}: ignoring ${f}"
34+
echo "${0}: ignoring non-executable ${f}"
3335
fi
3436
;;
3537
esac
3638
done
3739
echo "${0}: done running hooks in ${1}"
3840
}
3941

42+
43+
# NOTE: This hook will run as the user the container was started with!
4044
run-hooks /usr/local/bin/start-notebook.d
4145

42-
# Handle special flags if we're root
46+
# If the container started as the root user, then we have permission to refit
47+
# the jovyan user, and ensure file permissions, grant sudo rights, and such
48+
# things before we run the command passed to start.sh as the desired user
49+
# (NB_USER).
50+
#
4351
if [ "$(id -u)" == 0 ] ; then
52+
# Environment variables:
53+
# - NB_USER: the desired username and associated home folder
54+
# - NB_UID: the desired user id
55+
# - NB_GID: a group id we want our user to belong to
56+
# - NB_GROUP: the groupname we want for the group
57+
# - GRANT_SUDO: a boolean ("1" or "yes") to grant the user sudo rights
58+
# - CHOWN_HOME: a boolean ("1" or "yes") to chown the user's home folder
59+
# - CHOWN_EXTRA: a comma separated list of paths to chown
60+
# - CHOWN_HOME_OPTS / CHOWN_EXTRA_OPTS: arguments to the chown commands
4461

45-
# Only attempt to change the jovyan username if it exists
62+
# Refit the jovyan user to the desired the user (NB_USER)
4663
if id jovyan &> /dev/null ; then
47-
echo "Set username to: ${NB_USER}"
48-
usermod -d "/home/${NB_USER}" -l "${NB_USER}" jovyan
64+
if ! usermod --home "/home/${NB_USER}" --login "${NB_USER}" jovyan 2>&1 | grep "no changes" > /dev/null; then
65+
echo "Updated the jovyan user:"
66+
echo "- username: jovyan -> ${NB_USER}"
67+
echo "- home dir: /home/jovyan -> /home/${NB_USER}"
68+
fi
69+
elif ! id -u "${NB_USER}" &> /dev/null; then
70+
echo "ERROR: Neither the jovyan user or '${NB_USER}' exists."
71+
echo " This could be the result of stopping and starting, the"
72+
echo " container with a different NB_USER environment variable."
73+
exit 1
74+
fi
75+
# Ensure the desired user (NB_USER) gets its desired user id (NB_UID) and is
76+
# a member of the desired group (NB_GROUP, NB_GID)
77+
if [ "${NB_UID}" != "$(id -u "${NB_USER}")" ] || [ "${NB_GID}" != "$(id -g "${NB_USER}")" ]; then
78+
echo "Update ${NB_USER}'s UID:GID to ${NB_UID}:${NB_GID}"
79+
# Ensure the desired group's existence
80+
if [ "${NB_GID}" != "$(id -g "${NB_USER}")" ]; then
81+
groupadd --force --gid "${NB_GID}" --non-unique "${NB_GROUP:-${NB_USER}}"
82+
fi
83+
# Recreate the desired user as we want it
84+
userdel "${NB_USER}"
85+
useradd --home "/home/${NB_USER}" --uid "${NB_UID}" --gid "${NB_GID}" --groups 100 --no-log-init "${NB_USER}"
4986
fi
5087

51-
# handle home and working directory if the username changed
88+
# Move or symlink the jovyan home directory to the desired users home
89+
# directory if it doesn't already exist, and update the current working
90+
# directory to the new location if needed.
5291
if [[ "${NB_USER}" != "jovyan" ]]; then
53-
# changing username, make sure homedir exists
54-
# (it could be mounted, and we shouldn't create it if it already exists)
5592
if [[ ! -e "/home/${NB_USER}" ]]; then
56-
echo "Copying home dir to /home/${NB_USER}"
93+
echo "Attempting to copy /home/jovyan to /home/${NB_USER}..."
5794
mkdir "/home/${NB_USER}"
58-
cp -a /home/jovyan/. "/home/${NB_USER}/" || ln -s /home/jovyan "/home/${NB_USER}"
95+
if cp -a /home/jovyan/. "/home/${NB_USER}/"; then
96+
echo "Success!"
97+
else
98+
echo "Failed!"
99+
echo "Attempting to symlink /home/jovyan to /home/${NB_USER}..."
100+
if ln -s /home/jovyan "/home/${NB_USER}"; then
101+
echo "Success!"
102+
else
103+
echo "Failed!"
104+
fi
105+
fi
59106
fi
60-
# if workdir is in /home/jovyan, cd to /home/${NB_USER}
107+
# Ensure the current working directory is updated to the new path
61108
if [[ "${PWD}/" == "/home/jovyan/"* ]]; then
62-
newcwd="/home/${NB_USER}/${PWD:13}"
63-
echo "Setting CWD to ${newcwd}"
64-
cd "${newcwd}"
109+
new_wd="/home/${NB_USER}/${PWD:13}"
110+
echo "Changing working directory to ${new_wd}"
111+
cd "${new_wd}"
65112
fi
66113
fi
67114

68-
# Handle case where provisioned storage does not have the correct permissions by default
69-
# Ex: default NFS/EFS (no auto-uid/gid)
70-
if [[ "${CHOWN_HOME}" == "1" || "${CHOWN_HOME}" == 'yes' ]]; then
71-
echo "Changing ownership of /home/${NB_USER} to ${NB_UID}:${NB_GID} with options '${CHOWN_HOME_OPTS}'"
115+
# Optionally ensure the desired user get filesystem ownership of it's home
116+
# folder and/or additional folders
117+
if [[ "${CHOWN_HOME}" == "1" || "${CHOWN_HOME}" == "yes" ]]; then
118+
echo "Ensuring /home/${NB_USER} is owned by ${NB_UID}:${NB_GID} ${CHOWN_HOME_OPTS:+(chown options: ${CHOWN_HOME_OPTS})}"
72119
# shellcheck disable=SC2086
73120
chown ${CHOWN_HOME_OPTS} "${NB_UID}:${NB_GID}" "/home/${NB_USER}"
74121
fi
75122
if [ -n "${CHOWN_EXTRA}" ]; then
76123
for extra_dir in $(echo "${CHOWN_EXTRA}" | tr ',' ' '); do
77-
echo "Changing ownership of ${extra_dir} to ${NB_UID}:${NB_GID} with options '${CHOWN_EXTRA_OPTS}'"
124+
echo "Ensuring ${extra_dir} is owned by ${NB_UID}:${NB_GID} ${CHOWN_HOME_OPTS:+(chown options: ${CHOWN_HOME_OPTS})}"
78125
# shellcheck disable=SC2086
79126
chown ${CHOWN_EXTRA_OPTS} "${NB_UID}:${NB_GID}" "${extra_dir}"
80127
done
81128
fi
82129

83-
# Change UID:GID of NB_USER to NB_UID:NB_GID if it does not match
84-
if [ "${NB_UID}" != "$(id -u "${NB_USER}")" ] || [ "${NB_GID}" != "$(id -g "${NB_USER}")" ]; then
85-
echo "Set user ${NB_USER} UID:GID to: ${NB_UID}:${NB_GID}"
86-
if [ "${NB_GID}" != "$(id -g "${NB_USER}")" ]; then
87-
groupadd -f -g "${NB_GID}" -o "${NB_GROUP:-${NB_USER}}"
88-
fi
89-
userdel "${NB_USER}"
90-
useradd --home "/home/${NB_USER}" -u "${NB_UID}" -g "${NB_GID}" -G 100 -l "${NB_USER}"
91-
fi
92-
93-
# Enable sudo if requested
94-
if [[ "${GRANT_SUDO}" == "1" || "${GRANT_SUDO}" == 'yes' ]]; then
95-
echo "Granting ${NB_USER} sudo access and appending ${CONDA_DIR}/bin to sudo PATH"
96-
echo "${NB_USER} ALL=(ALL) NOPASSWD:ALL" > /etc/sudoers.d/notebook
97-
fi
130+
# Update potentially outdated environment variables since image build
131+
export XDG_CACHE_HOME="/home/${NB_USER}/.cache"
98132

99133
# Add ${CONDA_DIR}/bin to sudo secure_path
100134
sed -r "s#Defaults\s+secure_path\s*=\s*\"?([^\"]+)\"?#Defaults secure_path=\"\1:${CONDA_DIR}/bin\"#" /etc/sudoers | grep secure_path > /etc/sudoers.d/path
101135

102-
# Exec the command as NB_USER with the PATH and the rest of
103-
# the environment preserved
136+
# Optionally grant passwordless sudo rights for the desired user
137+
if [[ "$GRANT_SUDO" == "1" || "$GRANT_SUDO" == "yes" ]]; then
138+
echo "Granting ${NB_USER} passwordless sudo rights!"
139+
echo "${NB_USER} ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers.d/added-by-start-script
140+
fi
141+
142+
# NOTE: This hook is run as the root user!
104143
run-hooks /usr/local/bin/before-notebook.d
105-
echo "Executing the command:" "${cmd[@]}"
106-
exec sudo -E -H -u "${NB_USER}" PATH="${PATH}" XDG_CACHE_HOME="/home/${NB_USER}/.cache" PYTHONPATH="${PYTHONPATH:-}" "${cmd[@]}"
144+
145+
echo "Running as ${NB_USER}:" "${cmd[@]}"
146+
exec sudo --preserve-env --set-home --user "${NB_USER}" \
147+
PATH="${PATH}" XDG_CACHE_HOME="/home/${NB_USER}/.cache" \
148+
PYTHONPATH="${PYTHONPATH:-}" \
149+
"${cmd[@]}"
150+
151+
# The container didn't start as the root user, so we will have to act as the
152+
# user we started as.
107153
else
108-
if [[ "${NB_UID}" == "$(id -u jovyan 2>/dev/null)" && "${NB_GID}" == "$(id -g jovyan 2>/dev/null)" ]]; then
109-
# User is not attempting to override user/group via environment
110-
# variables, but they could still have overridden the uid/gid that
111-
# container runs as. Check that the user has an entry in the passwd
112-
# file and if not add an entry.
113-
STATUS=0 && whoami &> /dev/null || STATUS=$? && true
114-
if [[ "${STATUS}" != "0" ]]; then
115-
if [[ -w /etc/passwd ]]; then
116-
echo "Adding passwd file entry for $(id -u)"
117-
sed -e "s/^jovyan:/nayvoj:/" /etc/passwd > /tmp/passwd
118-
echo "jovyan:x:$(id -u):$(id -g):,,,:/home/jovyan:/bin/bash" >> /tmp/passwd
119-
cat /tmp/passwd > /etc/passwd
120-
rm /tmp/passwd
121-
else
122-
echo 'Container must be run with group "root" to update passwd file'
123-
fi
124-
fi
154+
# Warn about misconfiguration of: desired username, user id, or group id
155+
if [[ -n "${NB_USER}" && "${NB_USER}" != "$(id -un)" ]]; then
156+
echo "WARNING: container must be started as root to change the desired user's name with NB_USER!"
157+
fi
158+
if [[ -n "${NB_UID}" && "${NB_UID}" != "$(id -u)" ]]; then
159+
echo "WARNING: container must be started as root to change the desired user's id with NB_UID!"
160+
fi
161+
if [[ -n "${NB_GID}" && "${NB_GID}" != "$(id -g)" ]]; then
162+
echo "WARNING: container must be started as root to change the desired user's group id with NB_GID!"
163+
fi
125164

126-
# Warn if the user isn't going to be able to write files to ${HOME}.
127-
if [[ ! -w /home/jovyan ]]; then
128-
echo 'Container must be run with group "users" to update files'
129-
fi
130-
else
131-
# Warn if looks like user want to override uid/gid but hasn't
132-
# run the container as root.
133-
if [[ -n "${NB_UID}" && "${NB_UID}" != "$(id -u)" ]]; then
134-
echo "Container must be run as root to set NB_UID to ${NB_UID}"
135-
fi
136-
if [[ -n "${NB_GID}" && "${NB_GID}" != "$(id -g)" ]]; then
137-
echo "Container must be run as root to set NB_GID to ${NB_GID}"
165+
# Warn about misconfiguration of: granting sudo rights
166+
if [[ "${GRANT_SUDO}" == "1" || "${GRANT_SUDO}" == "yes" ]]; then
167+
echo "WARNING: container must be started as root to grant sudo permissions!"
168+
fi
169+
170+
# Attempt to ensure the user uid we currently run as has a named entry in
171+
# the /etc/passwd file, as it avoids software crashing on hard assumptions
172+
# on such entry. Writing to the /etc/passwd was allowed for the root group
173+
# from the Dockerfile during build.
174+
#
175+
# ref: https://github.com/jupyter/docker-stacks/issues/552
176+
if ! whoami &> /dev/null; then
177+
echo "There is no entry in /etc/passwd for our UID. Attempting to fix..."
178+
if [[ -w /etc/passwd ]]; then
179+
echo "Renaming old jovyan user to nayvoj ($(id -u jovyan):$(id -g jovyan))"
180+
sed --in-place "s/^jovyan:/nayvoj:/" /etc/passwd
181+
182+
echo "jovyan:x:$(id -u):$(id -g):,,,:/home/jovyan:/bin/bash" >> /etc/passwd
183+
echo "Added new jovyan user ($(id -u):$(id -g)). Fixed UID!"
184+
else
185+
echo "WARNING: unable to fix missing /etc/passwd entry because we don't have write permission."
138186
fi
139187
fi
140188

141-
# Warn if looks like user want to run in sudo mode but hasn't run
142-
# the container as root.
143-
if [[ "${GRANT_SUDO}" == "1" || "${GRANT_SUDO}" == 'yes' ]]; then
144-
echo 'Container must be run as root to grant sudo permissions'
189+
# Warn if the user isn't able to write files to ${HOME}
190+
if [[ ! -w /home/jovyan ]]; then
191+
echo "WARNING: no write access to /home/jovyan. Try starting the container with group 'users' (100)."
145192
fi
146193

147-
# Execute the command
194+
# NOTE: This hook is run as the user we started the container as!
148195
run-hooks /usr/local/bin/before-notebook.d
149196
echo "Executing the command:" "${cmd[@]}"
150197
exec "${cmd[@]}"

‎base-notebook/test/test_container_options.py

Lines changed: 58 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ def test_uid_change(container):
4444
command=["start.sh", "bash", "-c", "id && touch /opt/conda/test-file"],
4545
)
4646
# usermod is slow so give it some time
47-
c.wait(timeout=120)
47+
rv = c.wait(timeout=120)
48+
assert rv == 0 or rv["StatusCode"] == 0
4849
assert "uid=1010(jovyan)" in c.logs(stdout=True).decode("utf-8")
4950

5051

@@ -56,7 +57,8 @@ def test_gid_change(container):
5657
environment=["NB_GID=110"],
5758
command=["start.sh", "id"],
5859
)
59-
c.wait(timeout=10)
60+
rv = c.wait(timeout=10)
61+
assert rv == 0 or rv["StatusCode"] == 0
6062
logs = c.logs(stdout=True).decode("utf-8")
6163
assert "gid=110(jovyan)" in logs
6264
assert "groups=110(jovyan),100(users)" in logs
@@ -77,7 +79,9 @@ def test_nb_user_change(container):
7779
time.sleep(10)
7880
LOGGER.info(f"Checking if the user is changed to {nb_user} by the start script ...")
7981
output = running_container.logs(stdout=True).decode("utf-8")
80-
assert f"Set username to: {nb_user}" in output, f"User is not changed to {nb_user}"
82+
assert (
83+
f"username: jovyan -> {nb_user}" in output
84+
), f"User is not changed to {nb_user}"
8185

8286
LOGGER.info(f"Checking {nb_user} id ...")
8387
command = "id"
@@ -108,21 +112,30 @@ def test_nb_user_change(container):
108112

109113

110114
def test_chown_extra(container):
111-
"""Container should change the UID/GID of CHOWN_EXTRA."""
115+
"""Container should change the UID/GID of a comma separated
116+
CHOWN_EXTRA list of folders."""
112117
c = container.run(
113118
tty=True,
114119
user="root",
115120
environment=[
116121
"NB_UID=1010",
117122
"NB_GID=101",
118-
"CHOWN_EXTRA=/opt/conda",
123+
"CHOWN_EXTRA=/home/jovyan,/opt/conda/bin",
119124
"CHOWN_EXTRA_OPTS=-R",
120125
],
121-
command=["start.sh", "bash", "-c", "stat -c '%n:%u:%g' /opt/conda/LICENSE.txt"],
126+
command=[
127+
"start.sh",
128+
"bash",
129+
"-c",
130+
"stat -c '%n:%u:%g' /home/jovyan/.bashrc /opt/conda/bin/jupyter",
131+
],
122132
)
123133
# chown is slow so give it some time
124-
c.wait(timeout=120)
125-
assert "/opt/conda/LICENSE.txt:1010:101" in c.logs(stdout=True).decode("utf-8")
134+
rv = c.wait(timeout=120)
135+
assert rv == 0 or rv["StatusCode"] == 0
136+
logs = c.logs(stdout=True).decode("utf-8")
137+
assert "/home/jovyan/.bashrc:1010:101" in logs
138+
assert "/opt/conda/bin/jupyter:1010:101" in logs
126139

127140

128141
def test_chown_home(container):
@@ -131,18 +144,19 @@ def test_chown_home(container):
131144
c = container.run(
132145
tty=True,
133146
user="root",
134-
environment=["CHOWN_HOME=yes", "CHOWN_HOME_OPTS=-R"],
135-
command=[
136-
"start.sh",
137-
"bash",
138-
"-c",
139-
"chown root:root /home/jovyan && ls -alsh /home",
147+
environment=[
148+
"CHOWN_HOME=yes",
149+
"CHOWN_HOME_OPTS=-R",
150+
"NB_USER=kitten",
151+
"NB_UID=1010",
152+
"NB_GID=101",
140153
],
154+
command=["start.sh", "bash", "-c", "stat -c '%n:%u:%g' /home/kitten/.bashrc"],
141155
)
142-
c.wait(timeout=120)
143-
assert "Changing ownership of /home/jovyan to 1000:100 with options '-R'" in c.logs(
144-
stdout=True
145-
).decode("utf-8")
156+
rv = c.wait(timeout=120)
157+
assert rv == 0 or rv["StatusCode"] == 0
158+
logs = c.logs(stdout=True).decode("utf-8")
159+
assert "/home/kitten/.bashrc:1010:101" in logs
146160

147161

148162
def test_sudo(container):
@@ -224,3 +238,29 @@ def test_container_not_delete_bind_mount(container, tmp_path):
224238
assert rv == 0 or rv["StatusCode"] == 0
225239
assert p.read_text() == "some-content"
226240
assert len(list(tmp_path.iterdir())) == 1
241+
242+
243+
@pytest.mark.skip(reason="not yet implemented; TODO: cherry-pick b44b7ab")
244+
def test_jupyter_env_vars_to_unset_as_root(container):
245+
"""Environment variables names listed in JUPYTER_ENV_VARS_TO_UNSET
246+
should be unset in the final environment."""
247+
c = container.run(
248+
tty=True,
249+
user="root",
250+
environment=[
251+
"JUPYTER_ENV_VARS_TO_UNSET=SECRET_ANIMAL,UNUSED_ENV,SECRET_FRUIT",
252+
"FRUIT=bananas",
253+
"SECRET_FRUIT=mango",
254+
"SECRET_ANIMAL=cats",
255+
],
256+
command=[
257+
"start.sh",
258+
"bash",
259+
"-c",
260+
"echo I like $FRUIT and ${SECRET_FRUIT:-stuff}, and love ${SECRET_LOVE:-to keep secrets}!",
261+
],
262+
)
263+
rv = c.wait(timeout=10)
264+
assert rv == 0 or rv["StatusCode"] == 0
265+
logs = c.logs(stdout=True).decode("utf-8")
266+
assert "I like bananas and stuff, and love to keep secrets!" in logs

0 commit comments

Comments
 (0)