Implement run-hooks as a separate script (#1979)

* Implement run-hooks as a separate script

* Add more tests

* Add more docs

docs/using/common.md

@@ -86,7 +86,7 @@ You do so by passing arguments to the `docker run` command.
 
   ```{note}
   `NB_UMASK` when set only applies to the Jupyter process itself -
-  you cannot use it to set a `umask` for additional files created during run-hooks.
+  you cannot use it to set a `umask` for additional files created during `run-hooks.sh`.
   For example, via `pip` or `conda`.
   If you need to set a `umask` for these, you **must** set the `umask` value for each command.
   ```
@@ -135,7 +135,7 @@ or executables (`chmod +x`) to be run to the paths below:
 - `/usr/local/bin/before-notebook.d/` - handled **after** all the standard options noted above are applied
   and ran right before the Server launches
 
-See the `run-hooks` function in the [`jupyter/docker-stacks-foundation start.sh`](https://github.com/jupyter/docker-stacks/blob/main/images/docker-stacks-foundation/start.sh)
+See the `run-hooks.sh` script [here](https://github.com/jupyter/docker-stacks/blob/main/images/docker-stacks-foundation/run-hooks.sh) and how it's used in the [`start.sh`](https://github.com/jupyter/docker-stacks/blob/main/images/docker-stacks-foundation/start.sh)
 script for execution details.
 
 ## SSL Certificates

docs/using/selecting.md

@@ -36,6 +36,7 @@ It contains:
   with ownership over the `/home/jovyan` and `/opt/conda` paths
 - `tini` as the container entry point
 - A `start.sh` script as the default command - useful for running alternative commands in the container as applications are added (e.g. `ipython`, `jupyter kernelgateway`, `jupyter lab`)
+- A `run-hooks.sh` script, which can source/run files in a given directory
 - Options for a passwordless sudo
 - Common system libraries like `bzip2`, `ca-certificates`, `locales`
 - `wget` to download external files

images/docker-stacks-foundation/Dockerfile

@@ -127,7 +127,7 @@ ENTRYPOINT ["tini", "-g", "--"]
 CMD ["start.sh"]
 
 # Copy local files as late as possible to avoid cache busting
-COPY start.sh /usr/local/bin/
+COPY run-hooks.sh start.sh /usr/local/bin/
 
 USER root
 

images/docker-stacks-foundation/run-hooks.sh

@@ -0,0 +1,38 @@
+#!/bin/bash
+# Copyright (c) Jupyter Development Team.
+# Distributed under the terms of the Modified BSD License.
+
+# The run-hooks.sh script looks for *.sh scripts to source
+# and executable files to run within a passed directory
+
+if [ "$#" -ne 1 ]; then
+    echo "Should pass exactly one directory"
+    return 1
+fi
+
+if [[ ! -d "${1}" ]] ; then
+    echo "Directory ${1} doesn't exist or is not a directory"
+    return 1
+fi
+
+echo "Running hooks in: ${1} as uid: $(id -u) gid: $(id -g)"
+for f in "${1}/"*; do
+    # Hadling a case when the directory is empty
+    [ -e "${f}" ] || continue
+    case "${f}" in
+        *.sh)
+            echo "Sourcing shell script: ${f}"
+            # shellcheck disable=SC1090
+            source "${f}"
+            ;;
+        *)
+            if [ -x "${f}" ] ; then
+                echo "Running executable: ${f}"
+                "${f}"
+            else
+                echo "Ignoring non-executable: ${f}"
+            fi
+            ;;
+    esac
+done
+echo "Done running hooks in: ${1}"

images/docker-stacks-foundation/start.sh

@@ -14,33 +14,6 @@ _log () {
 }
 _log "Entered start.sh with args:" "$@"
 
-# The run-hooks function looks for .sh scripts to source and executable files to
-# run within a passed directory.
-run-hooks () {
-    if [[ ! -d "${1}" ]] ; then
-        return
-    fi
-    _log "${0}: running hooks in: ${1} as uid: $(id -u) gid: $(id -g)"
-    for f in "${1}/"*; do
-        case "${f}" in
-            *.sh)
-                _log "${0}: sourcing shell script: ${f}"
-                # shellcheck disable=SC1090
-                source "${f}"
-                ;;
-            *)
-                if [[ -x "${f}" ]] ; then
-                    _log "${0}: running executable: ${f}"
-                    "${f}"
-                else
-                    _log "${0}: ignoring non-executable: ${f}"
-                fi
-                ;;
-        esac
-    done
-    _log "${0}: done running hooks in: ${1}"
-}
-
 # A helper function to unset env vars listed in the value of the env var
 # JUPYTER_ENV_VARS_TO_UNSET.
 unset_explicit_env_vars () {
@@ -62,7 +35,8 @@ else
 fi
 
 # NOTE: This hook will run as the user the container was started with!
-run-hooks /usr/local/bin/start-notebook.d
+# shellcheck disable=SC1091
+source /usr/local/bin/run-hooks.sh /usr/local/bin/start-notebook.d
 
 # If the container started as the root user, then we have permission to refit
 # the jovyan user, and ensure file permissions, grant sudo rights, and such
@@ -160,7 +134,8 @@ if [ "$(id -u)" == 0 ] ; then
     fi
 
     # NOTE: This hook is run as the root user!
-    run-hooks /usr/local/bin/before-notebook.d
+    # shellcheck disable=SC1091
+    source /usr/local/bin/run-hooks.sh /usr/local/bin/before-notebook.d
 
     unset_explicit_env_vars
     _log "Running as ${NB_USER}:" "${cmd[@]}"
@@ -255,7 +230,8 @@ else
     fi
 
     # NOTE: This hook is run as the user we started the container as!
-    run-hooks /usr/local/bin/before-notebook.d
+    # shellcheck disable=SC1091
+    source /usr/local/bin/run-hooks.sh /usr/local/bin/before-notebook.d
     unset_explicit_env_vars
     _log "Executing the command:" "${cmd[@]}"
     exec "${cmd[@]}"

tests/conftest.py

@@ -108,6 +108,7 @@ class TrackedContainer:
         timeout: int,
         no_warnings: bool = True,
         no_errors: bool = True,
+        no_failure: bool = True,
         **kwargs: Any,
     ) -> str:
         running_container = self.run_detached(**kwargs)
@@ -119,7 +120,10 @@ class TrackedContainer:
             assert not self.get_warnings(logs)
         if no_errors:
             assert not self.get_errors(logs)
-        assert rv == 0 or rv["StatusCode"] == 0
+        if no_failure:
+            assert rv == 0 or rv["StatusCode"] == 0
+        else:
+            assert rv != 0 and rv["StatusCode"] != 0
         return logs
 
     @staticmethod

tests/docker-stacks-foundation/run-hooks-data/executable.py

@@ -0,0 +1,5 @@
+#!/usr/bin/env python3
+# Copyright (c) Jupyter Development Team.
+# Distributed under the terms of the Modified BSD License.
+
+print("Executable python file was successfully run")

tests/docker-stacks-foundation/run-hooks-data/non_executable.py

@@ -0,0 +1,5 @@
+#!/usr/bin/env python3
+# Copyright (c) Jupyter Development Team.
+# Distributed under the terms of the Modified BSD License.
+
+assert False

tests/docker-stacks-foundation/run-hooks-data/run-me.sh

@@ -0,0 +1,5 @@
+#!/bin/bash
+# Copyright (c) Jupyter Development Team.
+# Distributed under the terms of the Modified BSD License.
+
+export SOME_VAR=123

tests/docker-stacks-foundation/test_run_hooks.py

@@ -0,0 +1,95 @@
+# Copyright (c) Jupyter Development Team.
+# Distributed under the terms of the Modified BSD License.
+import logging
+from pathlib import Path
+
+from tests.conftest import TrackedContainer
+
+LOGGER = logging.getLogger(__name__)
+THIS_DIR = Path(__file__).parent.resolve()
+
+
+def test_run_hooks_zero_args(container: TrackedContainer) -> None:
+    logs = container.run_and_wait(
+        timeout=5,
+        tty=True,
+        no_failure=False,
+        command=["bash", "-c", "source /usr/local/bin/run-hooks.sh"],
+    )
+    assert "Should pass exactly one directory" in logs
+
+
+def test_run_hooks_two_args(container: TrackedContainer) -> None:
+    logs = container.run_and_wait(
+        timeout=5,
+        tty=True,
+        no_failure=False,
+        command=[
+            "bash",
+            "-c",
+            "source /usr/local/bin/run-hooks.sh first-arg second-arg",
+        ],
+    )
+    assert "Should pass exactly one directory" in logs
+
+
+def test_run_hooks_missing_dir(container: TrackedContainer) -> None:
+    logs = container.run_and_wait(
+        timeout=5,
+        tty=True,
+        no_failure=False,
+        command=[
+            "bash",
+            "-c",
+            "source /usr/local/bin/run-hooks.sh /tmp/missing-dir/",
+        ],
+    )
+    assert "Directory /tmp/missing-dir/ doesn't exist or is not a directory" in logs
+
+
+def test_run_hooks_dir_is_file(container: TrackedContainer) -> None:
+    logs = container.run_and_wait(
+        timeout=5,
+        tty=True,
+        no_failure=False,
+        command=[
+            "bash",
+            "-c",
+            "touch /tmp/some-file && source /usr/local/bin/run-hooks.sh /tmp/some-file",
+        ],
+    )
+    assert "Directory /tmp/some-file doesn't exist or is not a directory" in logs
+
+
+def test_run_hooks_empty_dir(container: TrackedContainer) -> None:
+    container.run_and_wait(
+        timeout=5,
+        tty=True,
+        command=[
+            "bash",
+            "-c",
+            "mkdir /tmp/empty-dir && source /usr/local/bin/run-hooks.sh /tmp/empty-dir/",
+        ],
+    )
+
+
+def test_run_hooks_with_files(container: TrackedContainer) -> None:
+    host_data_dir = THIS_DIR / "run-hooks-data"
+    cont_data_dir = "/home/jovyan/data"
+    # https://forums.docker.com/t/all-files-appear-as-executable-in-file-paths-using-bind-mount/99921
+    # Unfortunately, Docker treats all files in mounter dir as executable files
+    # So we make a copy of mounted dir inside a container
+    command = (
+        "cp -r /home/jovyan/data/ /home/jovyan/data-copy/ &&"
+        "source /usr/local/bin/run-hooks.sh /home/jovyan/data-copy/ &&"
+        "echo SOME_VAR is ${SOME_VAR}"
+    )
+    logs = container.run_and_wait(
+        timeout=5,
+        volumes={str(host_data_dir): {"bind": cont_data_dir, "mode": "ro"}},
+        tty=True,
+        command=["bash", "-c", command],
+    )
+    assert "Executable python file was successfully" in logs
+    assert "Ignoring non-executable: /home/jovyan/data-copy//non_executable.py" in logs
+    assert "SOME_VAR is 123" in logs